feat: run CPU isolation and the memory instrument inside the macro-agent sandbox by GuillaumeLagrange · Pull Request #456 · CodSpeedHQ/codspeed

GuillaumeLagrange · 2026-07-15T12:13:25Z

Runner-side changes to make CPU isolation and the memory instrument work
unprivileged inside the macro-agent sandbox. Two issues, combined here because
they share the sandbox's privilege model and the same branch.

CPU isolation (COD-3012). Drop the runner's built-in systemd-run /
CGROUP: isolation for a hook-based one: an Isolation type invokes
codspeed-{pre,wrap,post}-bench and stays oblivious to how cores are attributed.
If codspeed-wrap-bench exists it takes the (unprivileged) hook path; otherwise
it falls back to systemd-run --scope --slice=codspeed.slice, so non-sandbox
hosts are unchanged.

Memory instrument (COD-3047). Let memtrack run off a delegated BPF token
instead of root: attach via bpf()-native links (uprobe_multi + tp_btf),
resolve tracked PIDs in the tracker's PID namespace, and accept
LIBBPF_BPF_TOKEN_PATH as a privilege source.

Closes COD-3012
Closes COD-3047

codspeed-hq · 2026-07-15T12:18:07Z

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.

✅ 17 untouched benchmarks

_{Comparing macro-agent-sandbox (217413d) with cod-2796-switch-to-samply-profiler-by-default (228b86f)}

greptile-apps · 2026-07-15T12:20:23Z

Greptile Summary

This PR wires CPU isolation and memtrack's memory instrument into the macro-agent sandbox by solving two orthogonal privilege problems. CPU isolation drops the runner's own systemd-run/cgroup management in favour of machine-installed hook scripts (codspeed-{pre,wrap,post}-bench) that a gen-2 image owns entirely, while keeping the existing systemd-run scope as a gen-1 fallback. Memtrack gains a second compiled BPF skeleton (uprobe_multi + tp_btf, authorized via a delegated BPF token) alongside the classic perf-based one, with automatic PID-namespace translation so both flavors produce identical tracking results when run inside a PID namespace.

CPU isolation (isolation.rs): HookScriptsGuard is replaced by an Isolation RAII enum; Isolation::Hooks runs pre-bench on construction and post-bench on Drop, eliminating the need for sudo; Isolation::Systemd preserves the old systemd-run path for gen-1 runners unchanged.
memtrack BPF flavors: A single shared main.bpf.c is compiled into two skeletons — memtrack_token (uprobe_multi/tp_btf, sandbox-delegatable) and memtrack_perf (classic, wider kernel support) — with runtime dispatch via the new Flavor enum and with_skel!/attach_one! macros.
PID namespace support: current_pid() and task_ns_pid() in flavor.h resolve PIDs relative to the tracker's namespace (via bpf_get_ns_current_pid_tgid), and the previously-flagged UPROBE_ARGS_RET pid-gating bug is fixed to use current_pid() consistently.

Confidence Score: 5/5

Safe to merge — the isolation refactoring is well-structured RAII with correct pre/post hook pairing, and both BPF skeleton flavors have been verified to produce identical tracking output by the new equivalence test.

The two previously-flagged concerns (UPROBE_ARGS_RET pid-gating bug and has_delegated_bpf_token() is_dir() consistency) are both confirmed resolved. The only remaining issues are two eprintln! calls in the new test file that have no effect on production behaviour.

Files Needing Attention: crates/memtrack/tests/flavor_equivalence_tests.rs — two eprintln! calls should be replaced with tracing::info!.

Important Files Changed

Filename	Overview
src/executor/wall_time/isolation.rs	Clean redesign: replaces the always-running HookScriptsGuard with an Isolation enum whose RAII Drop correctly pairs pre_bench/post_bench for the Hooks variant; Systemd remains the gen-1 fallback (no hooks). Logic, tests, and hook detection all look correct.
crates/memtrack/src/ebpf/memtrack/mod.rs	Splits the single skeleton into Token (uprobe_multi + tp_btf, BPF-token delegatable) and Perf (classic perf) flavors; has_delegated_bpf_token() now consistently includes the is_dir() check; PID namespace rodata is correctly propagated to both flavors before load.
crates/memtrack/src/ebpf/c/utils/flavor.h	New header defining UPROBE_SEC/URETPROBE_SEC macros, current_pid() (returns namespace-relative TGID), and task_ns_pid() for fork tracking. Level bounds check (< 4) satisfies the BPF verifier before the variable array index.
crates/memtrack/src/ebpf/c/allocator.h	All SEC("uprobe") annotations replaced with SEC(UPROBE_SEC), and all pid computations updated to use current_pid() — including the previously-flagged UPROBE_ARGS_RET macro. Change is complete and consistent.
crates/memtrack/tests/flavor_equivalence_tests.rs	New integration test that runs the deterministic workload under both flavors and asserts identical tracking summaries; two eprintln! calls violate the repo rule requiring tracing macros.
src/executor/memory/executor.rs	has_delegated_bpf_token() added with proper is_dir() guard; privilege check and grant_privileges() correctly short-circuit when a delegated token is present, avoiding an unnecessary sudo prompt.
src/executor/wall_time/executor.rs	HookScriptsGuard removed and replaced with the new Isolation lifecycle; isolate bool renamed to requires_sudo for clarity; logic is otherwise unchanged.

Sequence Diagram

sequenceDiagram
    participant E as WallTimeExecutor
    participant I as Isolation::resolve()
    participant H as Machine Hooks
    participant P as Profiler
    participant B as Benchmark

    E->>I: Isolation::resolve()
    alt codspeed-wrap-bench exists
        I->>H: run_pre_bench(runner_pid)
        H-->>I: (evacuates bench cores)
        I-->>E: Isolation::Hooks
        E->>P: "wrap_command(requires_sudo=false)"
        H->>B: exec benchmark (bench cores)
        P->>B: record (descendant, no sudo)
        Note over I: Drop -> run_hook(post-bench)
    else no wrap-bench, can elevate
        I-->>E: Isolation::Systemd
        E->>P: "wrap_command(requires_sudo=true)"
        P->>B: record (system-wide, sudo)
    else no hook, no elevation
        I-->>E: Isolation::None
        E->>P: "wrap_command(requires_sudo=false)"
        P->>B: record (descendant, no sudo)
    end

_{Reviews (3): Last reviewed commit: "feat(memtrack): allow memtrack usage wit..." | Re-trigger Greptile}

Replace the runner's built-in CPU-isolation mechanisms with a single, machine-driven one. The runner previously hard-coded `systemd-run --scope` and a `CODSPEED_ISOLATION=CGROUP:<dir>` mode with per-spawn cgroup-dir plumbing, split across a `HookScriptsGuard` (which ran the pre/post-bench hooks) and `isolation.rs` (which did the wrapping). Now a single `Isolation` type owns the whole lifecycle: `resolve()` runs the pre-bench hook, `wrap_bench()` pins the benchmark leaf, and `Drop` runs post-bench. Cpuset logic lives on the machine behind three hooks (`codspeed-{pre,wrap,post}-bench`); the runner only invokes them and is otherwise oblivious to how cores are attributed. Discovery is by hook presence: - an executable `codspeed-wrap-bench` selects the hook path — unprivileged, and the benchmark stays a descendant of the profiler so it records without sudo; - its absence falls back to `systemd-run --scope --slice=codspeed.slice`, so hosts without the hook keep working unchanged. The pre-bench hook is invoked with the runner's PID so the machine places the runner (and the profiler it spawns) onto the system cores; the runner makes no cgroup writes of its own. The profiler's `wrap_command` flag is renamed `isolate` -> `requires_sudo`, now true only for the systemd fallback. Refs COD-3012 Co-Authored-By: Claude <noreply@anthropic.com>

Refs COD-3047

GuillaumeLagrange · 2026-07-27T16:23:13Z

@greptileai review

not-matthias

Overall, already pretty good. I think we can improve the naming a bit and let's check if we can keep the tests unified.

Lmk if you have some other thoughts on some comments.

not-matthias · 2026-07-27T18:00:21Z

+
+/// Which set of attach mechanisms a loaded skeleton uses.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Flavor {


nit: When I read flavor I think of 🧁

Wdyt of MemtrackVariant or BpfVariant?

not-matthias · 2026-07-27T18:00:21Z

+    /// Load the skeleton, selecting the attach flavor from the environment: a
+    /// delegated BPF token means we are the sandboxed workload and must use the
+    /// token path; otherwise the perf path, which also supports kernels
+    /// predating uprobe_multi.


this comment can be simplified/deslopped

not-matthias · 2026-07-27T18:00:22Z

+    /// Load the skeleton for a specific attach flavor, bypassing environment
+    /// detection. Used by tests to exercise either path directly; both attach
+    /// fine on a privileged kernel without a token (the token only authorizes
+    /// `bpf()` from inside the unprivileged sandbox).


this comment is also a bit too verbose imo

the token only authorizes /// `bpf()` from inside the unprivileged sandbox

would it make sense to also test this behavior here to ensure it works? wdyt?

not-matthias · 2026-07-27T18:00:22Z

+/// Set the PID-namespace rodata on an open skeleton, so both flavors resolve
+/// PIDs the same way. `$open` is the open skeleton, of either flavor.
+macro_rules! set_pidns_rodata {
+    ($open:expr, $ids:expr) => {
+        if let (Some((dev, ino)), Some(rodata)) = ($ids, $open.maps.rodata_data.as_deref_mut()) {
+            rodata.target_pidns_dev = dev;
+            rodata.target_pidns_ino = ino;
+        }
+    };
+}


nit: since this macro is only used inside the with_flavor func, can we just move it there to not have it in the global scope (even if it's not exported)

not-matthias · 2026-07-27T18:00:23Z

+/* Attach mechanism, selected at build time. With a delegated BPF token
+ * (MEMTRACK_BPF_LINKS) the uprobes attach as uprobe_multi links and the fork
+ * hook as tp_btf, both authorized through bpf(). Without it they use the classic
+ * perf-based uprobe/tracepoint paths, which work on kernels predating
+ * uprobe_multi (< 6.6) but cannot be delegated into the sandbox. */
+#ifdef MEMTRACK_BPF_LINKS
+#define UPROBE_SEC "uprobe.multi"
+#define URETPROBE_SEC "uretprobe.multi"
+#else
+#define UPROBE_SEC "uprobe"
+#define URETPROBE_SEC "uretprobe"
+#endif


As discussed, let's check if we need uprobe.multi

not-matthias · 2026-07-27T18:00:24Z

@@ -0,0 +1,5 @@
+/* Classic perf attach variant: perf-based uprobe/uretprobe + a perf


Does "perf" make sense here for the name of the variant/file? I associate it more with the perf tool.

not-matthias · 2026-07-27T18:00:24Z

@@ -0,0 +1,123 @@
+//! Verifies the two eBPF attach flavors produce equivalent tracking results.


Wouldn't it make more sense to simply run the existing tests both with flavor::token and flavor::perf? because then we can reuse the existing test fixtures rather than causing a split and having to test everything separately.

it should result in the same snapshots, right?

not-matthias · 2026-07-27T18:00:25Z

    }
+}

+#ifdef MEMTRACK_BPF_LINKS


no idea if possible, but could we use the tp_btf solution for both?

not-matthias · 2026-07-27T18:00:26Z


-    __u64 pid_tgid = bpf_get_current_pid_tgid();
-    __u32 tgid = pid_tgid >> 32;
+    __u32 tgid = current_pid();


tgid is actually the wrong name here, this should be named pid

not-matthias · 2026-07-27T18:00:26Z

+const volatile __u64 target_pidns_ino = 0;
+
+/* Current task's PID in the configured namespace (or global when unset). */
+static __always_inline __u32 current_pid(void) {


don't we need to return both the tid and pid for the target namespace?

we could mimic the style of bpf_get_current_pid_tgid and have memtrack_current_pid_tgid or something like that

This was referenced Jul 15, 2026

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

Closed

refactor(walltime): hook-based CPU isolation for the walltime runner #427

Closed

greptile-apps Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread crates/memtrack/src/ebpf/memtrack.rs Outdated

GuillaumeLagrange changed the title ~~[COD-3012] feat: run CPU isolation and the memory instrument inside the macro-agent sandbox~~ feat: run CPU isolation and the memory instrument inside the macro-agent sandbox Jul 15, 2026

GuillaumeLagrange requested review from not-matthias and removed request for not-matthias July 27, 2026 13:55

GuillaumeLagrange force-pushed the macro-agent-sandbox branch 3 times, most recently from cd83132 to 1142235 Compare July 27, 2026 15:29

GuillaumeLagrange changed the base branch from main to cod-2796-switch-to-samply-profiler-by-default July 27, 2026 15:30

GuillaumeLagrange requested a review from not-matthias July 27, 2026 15:30

GuillaumeLagrange and others added 2 commits July 27, 2026 17:34

feat(memtrack): allow memtrack usage with a bpf token

217413d

Refs COD-3047

GuillaumeLagrange force-pushed the macro-agent-sandbox branch from 1142235 to 217413d Compare July 27, 2026 15:34

GuillaumeLagrange force-pushed the cod-2796-switch-to-samply-profiler-by-default branch from 228b86f to 266f658 Compare July 27, 2026 16:20

Base automatically changed from cod-2796-switch-to-samply-profiler-by-default to main July 27, 2026 16:55

not-matthias requested changes Jul 27, 2026

View reviewed changes

		@@ -0,0 +1,5 @@
		/* Classic perf attach variant: perf-based uprobe/uretprobe + a perf

		@@ -0,0 +1,123 @@
		//! Verifies the two eBPF attach flavors produce equivalent tracking results.

Uh oh!

Conversation

GuillaumeLagrange commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

greptile-apps Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

GuillaumeLagrange commented Jul 27, 2026

Uh oh!

not-matthias left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GuillaumeLagrange commented Jul 15, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 15, 2026 •

edited

Loading

greptile-apps Bot commented Jul 15, 2026 •

edited

Loading