Conversation
📝 WalkthroughWalkthroughAdds profiling-focused Rust build flags and a debug Docker image runtime stage with eBPF observability tools, plus Makefile targets to build profiling-optimized debug images. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds eBPF profiling support to the debug build infrastructure by introducing profiling-friendly RUSTFLAGS and installing eBPF tooling in the debug Docker image. It also creates a new dedicated target for building profiling-optimized debug images.
Changes:
- Added RUSTFLAGS with frame pointers and debug symbols as defaults in Dockerfile.debug for better eBPF flamegraph support
- Installed eBPF observability tools (bpftrace, bpfcc-tools, linux-tools-common) and debugging utilities (strace, procps, psmisc) in the debug runtime image
- Created new
docker-build-debug-profilingMakefile target for building profiling-specific debug images
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| Makefile | Adds BUILD_PROFILE argument to docker-build-debug target and introduces new docker-build-debug-profiling target with explicit RUSTFLAGS |
| Dockerfile.debug | Adds RUSTFLAGS ARG with profiling defaults, installs eBPF tooling packages, and sets BPFTRACE_CACHE_USER_SYMBOLS environment variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Defaults enable call-stack unwinding and symbol data for better eBPF flamegraphs. | ||
| ARG RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" |
There was a problem hiding this comment.
The RUSTFLAGS default enables frame pointers and debug info for all builds using Dockerfile.debug, including the standard docker-build-debug target. These flags increase binary size and may impact performance. If the intention is to only enable these for profiling builds, consider making RUSTFLAGS conditional via a build argument, or use empty defaults and only set these flags via --build-arg in the docker-build-debug-profiling target.
| # Defaults enable call-stack unwinding and symbol data for better eBPF flamegraphs. | |
| ARG RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" | |
| # Override via --build-arg RUSTFLAGS="..." in profiling builds (default is empty). | |
| ARG RUSTFLAGS="" |
There was a problem hiding this comment.
I think this is valid, RUSTFLAGS should be empty by default and only specified via --build-arg in the docker-build-debug-profiling make target
| . | ||
|
|
||
| .PHONY: docker-build-debug-profiling | ||
| docker-build-debug-profiling: ## Build profiling-friendly debug image (frame pointers + debuginfo) for flamegraphs. |
There was a problem hiding this comment.
The new docker-build-debug-profiling target and the eBPF tooling additions are not documented in the README or visible documentation. Consider adding documentation about when to use the profiling-enabled debug image versus the standard debug image, including the required runtime privileges mentioned in the Dockerfile comments (--privileged or specific capabilities).
| docker-build-debug-profiling: ## Build profiling-friendly debug image (frame pointers + debuginfo) for flamegraphs. | |
| docker-build-debug-profiling: ## Build profiling-oriented debug image (frame pointers + debuginfo) for CPU profiling/eBPF flamegraphs. | |
| # Use this target instead of `docker-build-debug` when you need high-fidelity profiling data, | |
| # such as when generating flamegraphs or using eBPF-based profilers from inside the container. | |
| # When running the resulting image with eBPF tooling, the container usually must be started | |
| # with `--privileged` or with the necessary capabilities (e.g., perf/eBPF and related system | |
| # capabilities) granted explicitly, depending on your Docker and host configuration. |
| ENV BPFTRACE_CACHE_USER_SYMBOLS=1 | ||
|
|
There was a problem hiding this comment.
The BPFTRACE_CACHE_USER_SYMBOLS environment variable is set to 1 for all debug builds, but the caching behavior may not be desirable in all debugging scenarios. This could cause stale symbol information if the binary is rebuilt or replaced during development. Consider documenting this setting or making it configurable through a build argument, especially since the base debug image now includes this even when not using profiling features.
| ENV BPFTRACE_CACHE_USER_SYMBOLS=1 | |
| # Control bpftrace user symbol caching. Default is enabled (1) for performance. | |
| # Override at build time with: --build-arg BPFTRACE_CACHE_USER_SYMBOLS=0 | |
| ARG BPFTRACE_CACHE_USER_SYMBOLS=1 | |
| ENV BPFTRACE_CACHE_USER_SYMBOLS=${BPFTRACE_CACHE_USER_SYMBOLS} |
Makefile
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/make -f | |||
| Makefile#!/usr/bin/make -f | |||
There was a problem hiding this comment.
The shebang line appears corrupted. The word "Makefile" has been prepended to the shebang, making it invalid. The line should be "#!/usr/bin/make -f" without any text before the shebang. This will prevent the Makefile from being executed directly as a script if that functionality was intended.
| Makefile#!/usr/bin/make -f | |
| #!/usr/bin/make -f |
| --build-arg COMMIT=$(GIT_SHA) \ | ||
| --build-arg VERSION=$(GIT_TAG) \ | ||
| --build-arg BUILD_PROFILE=$(PROFILE) \ | ||
| --build-arg RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" \ |
There was a problem hiding this comment.
The RUSTFLAGS argument in the new docker-build-debug-profiling target duplicates the same default value already set in Dockerfile.debug (line 23). This creates maintenance burden as the flags would need to be updated in two places. Consider removing the --build-arg RUSTFLAGS line here to rely on the Dockerfile default, or document why this explicit override is necessary when the values are identical.
| --build-arg RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" \ |
| apt-get install -y \ | ||
| ca-certificates \ | ||
| libssl-dev \ | ||
| linux-tools-common \ |
There was a problem hiding this comment.
The package "linux-tools-common" is a meta-package that typically requires kernel-specific versions (like linux-tools-generic or linux-tools-$(uname -r)) to actually provide the perf binary. Installing only linux-tools-common may not provide the actual perf tool needed for eBPF profiling. Consider adding "linux-tools-generic" or documenting that the image must be run with host kernel tools mounted.
| linux-tools-common \ | |
| linux-tools-common \ | |
| linux-tools-generic \ |
| linux-tools-common \ | ||
| bpftrace \ | ||
| bpfcc-tools \ | ||
| procps \ | ||
| psmisc \ | ||
| strace && \ |
There was a problem hiding this comment.
The eBPF tooling packages (bpftrace, bpfcc-tools, linux-tools-common, strace) are now installed in all debug builds, even when using the docker-build-debug target without the -profiling suffix. These tools significantly increase image size and attack surface. Consider using multi-stage builds or build arguments to conditionally install these tools only for the debug-profiling variant, or create a separate Dockerfile.debug-profiling that extends from the base debug image and adds only the eBPF tools.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Makefile (1)
56-64:docker-build-debug-profilingRUSTFLAGS are redundant with theDockerfile.debugARG default.The value on line 63 is identical to the
ARG RUSTFLAGSdefault inDockerfile.debug(line 23), so this explicit override is a no-op. The two Make targets (docker-build-debuganddocker-build-debug-profiling) only differ by image tag. The RUSTFLAGS string is now duplicated across two files, which means they can silently drift.This is a downstream symptom of the same issue flagged in
Dockerfile.debuglines 21-24. Resolving the default there (Option A: clear it) would make this explicit pass-through meaningful and eliminate the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 56 - 64, The Make target docker-build-debug-profiling currently passes a RUSTFLAGS build-arg that duplicates the ARG RUSTFLAGS default in Dockerfile.debug; remove the redundant --build-arg RUSTFLAGS="..." from the docker-build-debug-profiling target (and mirror the same change in docker-build-debug if present) so the Dockerfile.debug ARG default is the single source of truth, or alternatively clear the ARG default in Dockerfile.debug so the explicit --build-arg in docker-build-debug-profiling becomes meaningful—update the docker-build-debug-profiling target (symbol: docker-build-debug-profiling) and verify docker-build-debug consistency accordingly.Dockerfile.debug (1)
21-24: Both debug targets produce identical artifacts — clarify the intended split.The
ARG RUSTFLAGSdefault already embeds the full profiling flags. Sincedocker-build-debugin the Makefile never passes--build-arg RUSTFLAGS, it inherits this default, making it functionally identical todocker-build-debug-profiling(which explicitly passes the same values). The only difference between the two Make targets is the image tag.If the intent is for all debug images to carry profiling flags (making both tags interchangeable), the
docker-build-debug-profilingtarget is redundant. If the intent is a "lean debug" vs "profiling debug" split, theARGdefault should be cleared (ARG RUSTFLAGS=) so the standard debug build has no extra flags.♻️ Option A — clear the default so the two targets are truly distinct
-ARG RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" +ARG RUSTFLAGS="" ENV RUSTFLAGS=$RUSTFLAGS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.debug` around lines 21 - 24, The Dockerfile currently sets ARG RUSTFLAGS with profiling flags, making the Make targets docker-build-debug and docker-build-debug-profiling identical; to create a true "lean debug" vs "profiling debug" split, change the Dockerfile DEBUG arg to a blank default (ARG RUSTFLAGS=) so docker-build-debug inherits no extra flags, and ensure docker-build-debug-profiling continues to pass the profiling flags via --build-arg RUSTFLAGS=" -C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" in the Makefile target; update references to ARG RUSTFLAGS, docker-build-debug, and docker-build-debug-profiling accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Line 1: Remove the malformed leading text on line 1 of the Makefile
("Makefile#!/usr/bin/make -f"); Makefiles shouldn't include a filename prefix or
a shebang—either delete the entire first line or leave only a valid shebang
("#!/usr/bin/make -f") if you intentionally need one, ensuring the file starts
with a valid Makefile directive or nothing at all so make can parse the file
correctly.
---
Nitpick comments:
In `@Dockerfile.debug`:
- Around line 21-24: The Dockerfile currently sets ARG RUSTFLAGS with profiling
flags, making the Make targets docker-build-debug and
docker-build-debug-profiling identical; to create a true "lean debug" vs
"profiling debug" split, change the Dockerfile DEBUG arg to a blank default (ARG
RUSTFLAGS=) so docker-build-debug inherits no extra flags, and ensure
docker-build-debug-profiling continues to pass the profiling flags via
--build-arg RUSTFLAGS=" -C force-frame-pointers=yes -C debuginfo=1 -C
symbol-mangling-version=v0" in the Makefile target; update references to ARG
RUSTFLAGS, docker-build-debug, and docker-build-debug-profiling accordingly.
In `@Makefile`:
- Around line 56-64: The Make target docker-build-debug-profiling currently
passes a RUSTFLAGS build-arg that duplicates the ARG RUSTFLAGS default in
Dockerfile.debug; remove the redundant --build-arg RUSTFLAGS="..." from the
docker-build-debug-profiling target (and mirror the same change in
docker-build-debug if present) so the Dockerfile.debug ARG default is the single
source of truth, or alternatively clear the ARG default in Dockerfile.debug so
the explicit --build-arg in docker-build-debug-profiling becomes
meaningful—update the docker-build-debug-profiling target (symbol:
docker-build-debug-profiling) and verify docker-build-debug consistency
accordingly.
Makefile
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/make -f | |||
| Makefile#!/usr/bin/make -f | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -3 Makefile | cat -ARepository: berachain/bera-reth
Length of output: 173
Remove malformed content from line 1.
Line 1 contains Makefile#!/usr/bin/make -f, which is invalid make syntax. The Makefile prefix is not a valid target, rule, or variable declaration. This will cause a parse error when make attempts to process the file. Either remove this line entirely (Makefiles do not require shebangs) or replace it with valid content if intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` at line 1, Remove the malformed leading text on line 1 of the
Makefile ("Makefile#!/usr/bin/make -f"); Makefiles shouldn't include a filename
prefix or a shebang—either delete the entire first line or leave only a valid
shebang ("#!/usr/bin/make -f") if you intentionally need one, ensuring the file
starts with a valid Makefile directive or nothing at all so make can parse the
file correctly.
chuck-bear
force-pushed
the
bera-reth-debug
branch
from
0a61dee to
0b2c58d
Compare
chuck-bear
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile.debug (1)
34-34:⚠️ Potential issue | 🟠 MajorHardcoded
releasepath breaks whenBUILD_PROFILEis notreleaseThe
cpcommand always copies from/app/target/release/bera-reth, but Cargo places the binary under/app/target/$BUILD_PROFILE/. According to the AI summary, the Makefile now passesBUILD_PROFILEas a build-arg (e.g., for the newdocker-build-debug-profilingtarget) — any non-releaseprofile will make thiscpfail after a full compilation, silently breaking the image.🐛 Proposed fix
-RUN cp /app/target/release/bera-reth /app/bera-reth +RUN cp /app/target/$BUILD_PROFILE/bera-reth /app/bera-reth🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.debug` at line 34, The RUN cp line in Dockerfile.debug hardcodes /app/target/release/bera-reth which breaks when BUILD_PROFILE ≠ release; change the copy to use the build-arg BUILD_PROFILE (e.g., cp /app/target/${BUILD_PROFILE}/bera-reth /app/bera-reth) and ensure Dockerfile.debug declares ARG BUILD_PROFILE with a sensible default (release) so the binary path resolves for profiling/debug builds.
🧹 Nitpick comments (2)
Dockerfile.debug (2)
21-24:debuginfo=1provides line tables only — comment overstates coverageThe inline comment says "symbol data", but
-C debuginfo=1produces line-number tables only (not full DWARF type/variable info). For eBPF flamegraphs with frame pointers this is perfectly adequate, but if you later needaddr2lineor DWARF-based unwinding (e.g., Go-style DWARF inbpftraceprobes on inlined frames),debuginfo=2is required. Consider tightening the comment to match:📝 Suggested comment tweak
-# Defaults enable call-stack unwinding and symbol data for better eBPF flamegraphs. -ARG RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0" +# Defaults enable frame pointers and line-table debuginfo for eBPF flamegraphs. +# Use debuginfo=2 if full DWARF info (inlined frames, variable types) is needed. +ARG RUSTFLAGS="-C force-frame-pointers=yes -C debuginfo=1 -C symbol-mangling-version=v0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.debug` around lines 21 - 24, Update the inline comment above the ARG/ENV RUSTFLAGS to accurately state that -C debuginfo=1 only emits line-number tables (not full DWARF symbol/type/variable info); either change the comment to say it provides frame pointers plus line tables sufficient for eBPF flamegraphs but not full DWARF-based tools, or set -C debuginfo=2 in RUSTFLAGS if you want full DWARF for addr2line/DWARF-based unwinding; refer to the ARG RUSTFLAGS and ENV RUSTFLAGS lines when making this change.
47-47:linux-tools-commonalone is not sufficient forperfOn Ubuntu 24.04,
linux-tools-commononly ships wrapper scripts that delegate tolinux-tools-$(uname -r). Since the runtime kernel version is unknown at image build time, the kernel-specific package cannot be pre-baked in. At runtime,perfwill fail with an error like:"WARNING: perf not found for kernel X.Y — you may need to install linux-tools-$(uname -r)"
Options:
- Keep
linux-tools-commonas a convenience hint but document in the image README thatlinux-tools-$(uname -r)must be installed on the host or injected at container start.- Add an entrypoint wrapper script that installs the correct package on first run (requires network/apt access in the container).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.debug` at line 47, The Dockerfile currently installs only linux-tools-common which is insufficient for perf because ubuntu provides kernel-specific packages (linux-tools-$(uname -r)); update the image by either (A) keeping linux-tools-common but add a clear note to the README that the host must install or bind-mount linux-tools-$(uname -r) at runtime so perf works, or (B) implement an entrypoint wrapper (add an ENTRYPOINT script referenced in Dockerfile.debug) that at container start detects the running kernel (uname -r) and apt-installs or binds the matching linux-tools-$(uname -r) package if available (ensuring the container has network/apt access and handling failures gracefully); refer to linux-tools-common and perf in the Dockerfile.debug and name the wrapper script (e.g., entrypoint-perf.sh) so reviewers can find and test it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Dockerfile.debug`:
- Line 34: The RUN cp line in Dockerfile.debug hardcodes
/app/target/release/bera-reth which breaks when BUILD_PROFILE ≠ release; change
the copy to use the build-arg BUILD_PROFILE (e.g., cp
/app/target/${BUILD_PROFILE}/bera-reth /app/bera-reth) and ensure
Dockerfile.debug declares ARG BUILD_PROFILE with a sensible default (release) so
the binary path resolves for profiling/debug builds.
---
Nitpick comments:
In `@Dockerfile.debug`:
- Around line 21-24: Update the inline comment above the ARG/ENV RUSTFLAGS to
accurately state that -C debuginfo=1 only emits line-number tables (not full
DWARF symbol/type/variable info); either change the comment to say it provides
frame pointers plus line tables sufficient for eBPF flamegraphs but not full
DWARF-based tools, or set -C debuginfo=2 in RUSTFLAGS if you want full DWARF for
addr2line/DWARF-based unwinding; refer to the ARG RUSTFLAGS and ENV RUSTFLAGS
lines when making this change.
- Line 47: The Dockerfile currently installs only linux-tools-common which is
insufficient for perf because ubuntu provides kernel-specific packages
(linux-tools-$(uname -r)); update the image by either (A) keeping
linux-tools-common but add a clear note to the README that the host must install
or bind-mount linux-tools-$(uname -r) at runtime so perf works, or (B) implement
an entrypoint wrapper (add an ENTRYPOINT script referenced in Dockerfile.debug)
that at container start detects the running kernel (uname -r) and apt-installs
or binds the matching linux-tools-$(uname -r) package if available (ensuring the
container has network/apt access and handling failures gracefully); refer to
linux-tools-common and perf in the Dockerfile.debug and name the wrapper script
(e.g., entrypoint-perf.sh) so reviewers can find and test it.
Summary by CodeRabbit