forked from grafana/beyla
-
Notifications
You must be signed in to change notification settings - Fork 0
Beyla two step memory win #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ensures semaphore token is never leaked even if inspectOffsets() or other ELF parsing code panics. Zero-cost defensive programming that prevents potential deadlocks. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Use pr-X format for PR builds to avoid invalid Docker tags when branch names contain slashes or special characters. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Without this, PR builds only stay in cache and aren't available for testing. Now PR images will be pushed with pr-X tags for actual testing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Remove conditional that skipped DockerHub login for PRs. Now that we push PR images, we need to authenticate. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Switch from old fix-memory-leak-minimal.patch to fix-memory-spikes.patch containing semaphore and panic recovery fixes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Memory spike fixes are already applied directly to vendor files, so patch step is no longer needed and was causing conflicts. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Fails fast if our memory spike fixes are missing from the source code before building, preventing 20+ min builds that produce wrong images. Checks for: - BEYLA_MAX_CONCURRENT_ELF environment variable - elfParseSem semaphore variable - Semaphore implementation comments 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This prevents the Dockerfile from running 'make copy-obi-vendor' which was overwriting our memory spike fixes with the original OBI code.
- Run full vendor update to get latest dependencies - Apply semaphore patches to prevent ELF parsing memory spikes - Copy generated eBPF files and Java agent jar from .obi-src - Update Dockerfile to verify patches are preserved during build - Add build-preserve-patches target to Makefile for CI - Set DEV_OBI=1 in Docker build to skip vendor regeneration This ensures we have both: 1. All required dependencies and generated files 2. Our memory spike fixes for concurrent ELF parsing
This ensures we catch any unexpected patch overwrites right before the compile step, providing maximum confidence that our memory fixes are actually being compiled into the binary.
The previous commit was missing: - grafana-opentelemetry-java.jar - All *bpfel.go and *bpfel.o eBPF generated files These files are required for compilation and must be committed since we're using DEV_OBI=1 to skip regeneration.
Since we've already committed all vendor dependencies and generated files, we don't need to regenerate anything. The CI should just compile directly. This avoids: 1. Running docker-generate which was creating new files in .obi-src 2. The broken copy-generated-files-only which was copying to wrong directories 3. Any possibility of overwriting our vendored patches
The test target needs setup-envtest and other tools installed. Also creates the testoutput directory needed for coverage reports.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.