feat(tracer): add whole trace id to pprof labels and enable them with appsec#5064
feat(tracer): add whole trace id to pprof labels and enable them with appsec#5064eliottness wants to merge 2 commits into
Conversation
Emit the full 128-bit trace id (32-char lowercase hex) as a new "trace id" pprof label so profiler samples can be correlated with the whole trace, not just the local root span. The "trace id" and "local root span id" labels are now emitted for code hotspots and whenever AppSec is enabled, so AppSec can correlate security events with traces/profiles. "span id" stays gated behind code hotspots and "trace endpoint" behind endpoint profiling.
Config Audit |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 679beae | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-24 11:35:06 Comparing candidate commit 679beae in PR branch Found 2 performance improvements and 18 performance regressions! Performance is the same for 306 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
|
Ouch!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae25f6e5ca
ℹ️ 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".
| func (t *tracer) applyPPROFLabels(ctx gocontext.Context, span *Span, snap internalconfig.SpanStartSnapshot) { | ||
| // "local root span id" and "trace id" are emitted for code hotspots and when | ||
| // AppSec is enabled, so it can correlate security events with traces/profiles. | ||
| correlate := snap.ProfilerHotspotsEnabled || appsec.Enabled() |
There was a problem hiding this comment.
Keep AppSec correlation from enabling endpoint labels
When AppSec is enabled while endpoint profiling is explicitly off, this makes correlate true and applyPPROFLabels now stores a non-nil pprofCtxActive. Later, Span.SetTag(ext.ResourceName, ...) only checks s.pprofCtxActive != nil before adding traceprof.TraceEndpoint (span.go:490-498), so web/custom spans that update their resource after start begin emitting trace endpoint labels even with WithProfilerEndpoints(false). Please keep an explicit endpoints-enabled state for that update path or avoid creating endpoint labels unless endpoint profiling was enabled.
Useful? React with 👍 / 👎.
| func (t *tracer) applyPPROFLabels(ctx gocontext.Context, span *Span, snap internalconfig.SpanStartSnapshot) { | ||
| // "local root span id" and "trace id" are emitted for code hotspots and when | ||
| // AppSec is enabled, so it can correlate security events with traces/profiles. | ||
| correlate := snap.ProfilerHotspotsEnabled || appsec.Enabled() |
There was a problem hiding this comment.
Don't clear user pprof labels in AppSec-only spans
When AppSec is enabled with both profiler label features disabled, correlate becomes true for root spans created via StartSpan without a context; those roots use context.Background() as their restore context, so starting the span replaces any existing goroutine pprof labels and Finish restores the background labels, clearing user labels that previously survived when hotspots/endpoints were off. Please avoid applying AppSec correlation labels without a real caller context, or preserve/restore the existing labels.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| if correlate { | ||
| labels = append(labels, traceprof.TraceID, span.context.TraceID()) |
There was a problem hiding this comment.
Avoid per-span trace ID encoding on the hot path
With code hotspots defaulting on, this calls span.context.TraceID() for every started span. For locally-created spans, the trace ID cache is intentionally empty and HexEncoded recomputes/allocates on each call (spancontext.go:53-68, spancontext.go:323-331), so a trace with many child spans now pays a duplicate 128-bit hex encoding allocation for every span even though they share the same trace ID. Please cache/reuse the trace label value per trace or otherwise avoid adding this allocation to every StartSpan.
Useful? React with 👍 / 👎.
The new pprof "trace id" label added a per-span hex.EncodeToString allocation (+1 alloc/span on BenchmarkStartSpan and other span-creation benchmarks). Every span in a trace shares the same trace id, so compute the 32-char hex once and let child spans reuse the parent's cached string: hexEncodedCached memoizes the hex on the not-yet-shared SpanContext during StartSpan, and newSpanContext copies the parent's cached hex into the child (guarded by a full trace-id equality check). The read path (HexEncoded, used by Inject) still never writes, so the v2.8.0-rc.2 concurrent-Inject data race cannot reoccur. benchstat (count=10): allocs/op 16 -> 15, B/op -1.84% on BenchmarkStartSpan / BenchmarkStartSpanConcurrent.
|
@kakkoyun the +1 alloc was the per-span trace-id hex encode — it's now computed once per trace and reused by child spans ( |
What does this PR do?
Adds the full 128-bit trace ID as a new
trace idpprof label so the continuousprofiler can correlate samples with the whole trace, not just the local root span.
traceprof.TraceID = "trace id", encoded as the whole128-bit trace ID in 32-character lowercase hex (unlike the existing 64-bit
decimal
span id/local root span id).trace idandlocal root span idare now emitted for code hotspots andwhenever AppSec is enabled, so AppSec can correlate security events with
traces/profiles.
span idstays gated behind code hotspots andtrace endpointbehind endpoint profiling.
applyPPROFLabelssoappsec.Enabled()is evaluated atmost once per span start (and not at all in the default hotspots-on path, thanks
to short-circuiting). Label keys remain in alphabetical order for the go1.24
label optimization.
Motivation
Today the pprof labels emitted by the tracer only carry the 64-bit
span idandlocal root span id. With 128-bit trace IDs, profiler samples cannot be tied tothe whole trace ID, and AppSec has no trace-correlation labels unless the
profiler's code-hotspots feature happens to be enabled. This change makes the full
trace ID available and ensures the correlation labels are present when AppSec is
on.
Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Note
No new/proven backend pprof contract exists yet for a
trace idlabel acrossDatadog tracers; the profiler backend must be set up to read
trace idbeforerelying on it for correlation. Value is high-cardinality (unique per trace),
emitted only under the same gates as the existing correlation labels.