Skip to content

feat(memtrack): support running the memory instrument in the macro-agent sandbox#443

Closed
GuillaumeLagrange wants to merge 1 commit into
mainfrom
cod-3047-support-memory-instrument-on-macro-agent-sandbox
Closed

feat(memtrack): support running the memory instrument in the macro-agent sandbox#443
GuillaumeLagrange wants to merge 1 commit into
mainfrom
cod-3047-support-memory-instrument-on-macro-agent-sandbox

Conversation

@GuillaumeLagrange

@GuillaumeLagrange GuillaumeLagrange commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Make the memory instrument work when the runner and codspeed-memtrack run inside the macro-agent sandbox — unprivileged, in a user + PID namespace, with a delegated BPF token instead of root. Companion to the platform PR that sets up the token and the exe-gate.

  • Attach via bpf()-native links. Switch uprobes to uprobe_multi and the fork tracepoint to tp_btf, so a BPF token can authorize them (the old perf_event_open path needs CAP_PERFMON). eBPF logic unchanged.
  • Resolve PIDs in the tracker's PID namespace. eBPF sees global PIDs but memtrack registers namespace-local ones, so nothing matched inside the sandbox. In the init namespace this still resolves to global PIDs.
  • Accept a delegated BPF token as a privilege source. Treat a live LIBBPF_BPF_TOKEN_PATH as satisfied privilege instead of requiring root / file-capabilities.

Proven end-to-end on the macro-runner: codspeed run -m memory on an instrumented divan bench runs inside the sandbox and uploads to staging.

Refs COD-3047

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes memory tracking work in the macro-agent sandbox. The main changes are:

  • Adds token and perf eBPF skeleton variants.
  • Uses bpf-native links for token-based attachment.
  • Resolves tracked PIDs relative to memtrack's PID namespace.
  • Treats delegated BPF tokens as a privilege source.
  • Adds flavor-equivalence coverage for the two attach paths.

Confidence Score: 4/5

This is close, but the realloc tracking gap should be fixed before merging.

  • Realloc entry probes still compare the host PID against namespace-local tracked PIDs.
  • In the sandbox, that can drop realloc events without an error.
  • The rest of the token and namespace wiring is narrowly scoped.

crates/memtrack/src/ebpf/c/memtrack.bpf.c

Important Files Changed

Filename Overview
crates/memtrack/src/ebpf/c/memtrack.bpf.c Adds attach-mode selection and namespace-aware PID helpers, but the realloc-family entry path still checks the host PID.
crates/memtrack/src/ebpf/memtrack.rs Adds token/perf skeleton selection, offset-based uprobe attachment, and PID namespace rodata setup.
src/executor/memory/executor.rs Accepts delegated BPF tokens as an alternative to root or memtrack file capabilities.

Comments Outside Diff (1)

  1. crates/memtrack/src/ebpf/c/memtrack.bpf.c, line 319-321 (link)

    P1 Realloc uses host PID

    In the sandbox, userspace registers the namespace-local PID in tracked_pids, but this entry probe still checks tid >> 32, which is the host PID. When those values differ, this branch returns before saving realloc_args, and the return probe has no arguments to emit the realloc event. Realloc allocations are then silently missing from the memory trace.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: crates/memtrack/src/ebpf/c/memtrack.bpf.c
    Line: 319-321
    
    Comment:
    **Realloc uses host PID**
    
    In the sandbox, userspace registers the namespace-local PID in `tracked_pids`, but this entry probe still checks `tid >> 32`, which is the host PID. When those values differ, this branch returns before saving `realloc_args`, and the return probe has no arguments to emit the realloc event. Realloc allocations are then silently missing from the memory trace.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code Fix in Codex

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/memtrack/src/ebpf/c/memtrack.bpf.c:319-321
**Realloc uses host PID**

In the sandbox, userspace registers the namespace-local PID in `tracked_pids`, but this entry probe still checks `tid >> 32`, which is the host PID. When those values differ, this branch returns before saving `realloc_args`, and the return probe has no arguments to emit the realloc event. Realloc allocations are then silently missing from the memory trace.

Reviews (4): Last reviewed commit: "feat(memtrack): allow memtrack usage wit..." | Re-trigger Greptile

Comment thread crates/memtrack/src/ebpf/c/memtrack.bpf.c
Comment thread crates/memtrack/src/ebpf/memtrack.rs
Comment thread src/executor/memory/executor.rs
@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks


Comparing cod-3047-support-memory-instrument-on-macro-agent-sandbox (81c4b15) with main (3278ec8)

Open in CodSpeed

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-3047-support-memory-instrument-on-macro-agent-sandbox branch 2 times, most recently from b3e73df to b2f8ac7 Compare July 8, 2026 13:48
Comment thread crates/memtrack/src/ebpf/memtrack.rs
Comment thread src/executor/memory/executor.rs
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-3047-support-memory-instrument-on-macro-agent-sandbox branch from b2f8ac7 to 94d981a Compare July 8, 2026 14:06
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-3047-support-memory-instrument-on-macro-agent-sandbox branch from 94d981a to 81c4b15 Compare July 8, 2026 14:15
@GuillaumeLagrange GuillaumeLagrange marked this pull request as ready for review July 9, 2026 07:54
@GuillaumeLagrange GuillaumeLagrange requested a review from art049 July 9, 2026 07:55
@GuillaumeLagrange

Copy link
Copy Markdown
Contributor Author

The changes from this PR have been moved to #456, which combines the runner-side work for COD-3012 (hook-based CPU isolation) and COD-3047 (BPF-token memory instrument) into a single PR. Closing in favor of that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant