feat: Add eBPF prctl monitor to detect late OTEL_CTX publication#89
feat: Add eBPF prctl monitor to detect late OTEL_CTX publication#89nsavoire wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d57978d1ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| goto exit; | ||
| } | ||
|
|
||
| if (report_pid(ctx, pid_tgid, RATELIMIT_ACTION_DEFAULT)) { |
There was a problem hiding this comment.
Bypass rate limiting for OTEL_CTX publication
When an already-tracked process has a recent reported_pids token from another PID event (for example an unknown-PC resync), RATELIMIT_ACTION_DEFAULT can suppress this one-shot prctl notification even though the earlier synchronization may have run before the mapping was named. Since SynchronizeProcess only removes reported_pids for new processes (processmanager/processinfo.go notes non-new syncs remain rate-limited), that leaves no guaranteed later resync and the newly published process context can remain stale/missing; the prctl publication event should bypass/reset the PID-event rate limit.
Useful? React with 👍 / 👎.
| if err := trc.AttachPrctlMonitor(); err != nil { | ||
| return fmt.Errorf("failed to attach prctl monitor: %w", err) |
There was a problem hiding this comment.
Do not fail startup when the prctl tracepoint is absent
On kernels where the syscalls/sys_exit_prctl tracepoint is unavailable (for example syscall tracepoints are not enabled), this new hard error causes Controller.Start to abort the whole profiler even though late OTEL_CTX discovery is an optional enhancement. Treating the prctl monitor attach as best-effort would preserve existing profiling behavior on those kernels instead of turning this feature into a startup dependency.
Useful? React with 👍 / 👎.
Summary
Adds an eBPF tracepoint on the
prctlsyscall that detects when a tracked process names an anonymous VMAOTEL_CTXviaprctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ..., "OTEL_CTX"), and triggers a PID resynchronization so the newly-published process context mapping is observed without waiting for the nextSynchronizeProcess.Without this hook, a process that publishes its OTEL_CTX mapping after the profiler has already synchronized might never have its resource attributes attached to profiles, since
SynchronizeProcessonly re-runs when the unwinder hits an unknown executable mapping, and OTEL_CTX is neither executable nor reachable from a stack.Implementation notes
sys_exit_prctl), not entry. The resync must run after the kernel has applied the rename; otherwise user space could re-read/proc/<pid>/mapsbefore[anon:OTEL_CTX]is visible and miss the freshly published context. Since the exit tracepoint only carries the return value, the prctl arguments (option,arg2, name pointer) are recovered from the task's entrypt_regs(x86-64 and arm64).option == PR_SET_VMAandarg2 == PR_SET_VMA_ANON_NAMEand checks that the passed string is"OTEL_CTX".reported_pidsorpid_information_exists) are reported.bpf.num_prctl_set_vma_otel_ctxcounts how often the resync hook fires.PR_SET_VMA_ANON_NAMErequires Linux 5.17+ withCONFIG_ANON_VMA_NAME, but the tracepoint fires regardless of whether the kernel honors the call, so the resync path works on older kernels too.SynchronizeProcessand re-parse/proc/PID/mapsto locate the OTEL_CTX mapping. Since the tracepoint already has the mapping's address and length, a future optimization could introduce a dedicated event type that hands those values directly to the process context resolver, skipping the full/proc/PID/mapsre-parse.Note: this PR is stacked on #66 (process context propagation).
🤖 Generated with Claude Code