feat: run memtrack memory profiling without sudo via file capabilities#407
feat: run memtrack memory profiling without sudo via file capabilities#407not-matthias wants to merge 9 commits into
Conversation
Raising RLIMIT_MEMLOCK requires CAP_SYS_RESOURCE, which unprivileged agents and many containers lack. On kernels >= 5.11 BPF memory is accounted against the cgroup, so the limit is irrelevant; treat a failed setrlimit as a warning instead of bailing.
Without an IPC server nothing toggles the tracking_enabled map, so the eBPF is_enabled() check drops every event. Enable tracking up front in that path.
Merging this PR will not alter performance
|
codspeed setup --mode memory now grants codspeed-memtrack the capabilities it needs (cap_bpf, cap_perfmon, cap_dac_read_search, cap_sys_admin, cap_sys_resource) via setcap, idempotently and with a single sudo prompt. At run time the memory executor no longer wraps memtrack in sudo: it requires root or those capabilities, otherwise failing with guidance to run codspeed setup. Lets agents run memory benchmarks unattended. Related: COD-1801
setup status now prints a privileges line under the memory executor: satisfied when running as root or the codspeed-memtrack binary carries the required file capabilities, missing (with remediation) otherwise. Lets agents confirm sudo-less memory profiling is ready before a run. Related: COD-1801
memtrack-based memory profiling is Linux-only; the caps crate fails to build on macOS. Move caps/memtrack/ipc-channel to Linux-only dependencies and cfg-gate the memory module and its wiring so macOS builds compile.
6b0e488 to
d729c92
Compare
Greptile SummaryThis PR replaces the previous
Confidence Score: 5/5Safe to merge. The capability-granting path is idempotent and best-effort; the hard privilege gate in run() ensures memtrack never silently runs without the needed caps. The xattr parsing is correct for all VFS_CAP revisions, the two-layer design (best-effort grant at setup, hard-fail at run) is sound, all tracing-macro and git-pin rules are followed, and the no-IPC tracking fix is clearly correct. The only open points are non-blocking UX refinements. src/executor/memory/setup.rs — ensure_memtrack_capabilities silently returns Ok(()) when setcap or the post-grant xattr verification fails, so codspeed setup exits 0 even when caps were not actually written. Important Files Changed
|
| /// `setcap` text form of [`MEMTRACK_REQUIRED_CAPS`]. Kept as an explicit string | ||
| /// because it must match libcap's grammar exactly. | ||
| const MEMTRACK_SETCAP_SPEC: &str = | ||
| "cap_bpf,cap_perfmon,cap_dac_read_search,cap_sys_admin,cap_sys_resource+ep"; |
There was a problem hiding this comment.
MEMTRACK_SETCAP_SPEC and MEMTRACK_REQUIRED_CAPS must be kept in sync manually
These two constants encode the same set of capabilities in two different representations — one as a &[Capability] array (used by memtrack_required_caps_mask() to build the xattr verification mask) and one as a libcap grammar string (passed verbatim to setcap). If a future contributor adds a new capability to MEMTRACK_REQUIRED_CAPS without updating MEMTRACK_SETCAP_SPEC, setcap will grant the old set, the post-grant binary_has_capabilities check will fail (because the required mask is now larger), the "capabilities did not stick" warning fires, and every subsequent codspeed run will hit the ensure_privileges bail with "run codspeed setup" — even though setup was just run successfully. A compile-time or unit-test assertion that MEMTRACK_SETCAP_SPEC names exactly the caps in MEMTRACK_REQUIRED_CAPS would catch this class of drift.
No description provided.