diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go index 9367ed850b..37b01d6db1 100644 --- a/ddtrace/tracer/spancontext.go +++ b/ddtrace/tracer/spancontext.go @@ -123,6 +123,20 @@ func (t *traceID) cacheHex() { t.hexEncoded = hex.EncodeToString(t.value[:]) } +// hexEncodedCached returns HexEncoded and memoizes the result on the traceID so +// subsequent reads (and child spans, via newSpanContext) reuse it instead of +// re-encoding. Because it writes t.hexEncoded, it MUST only be called while the +// enclosing SpanContext is not yet shared with other goroutines (e.g. from +// tracer.StartSpan, before the span is returned). This lets a whole trace pay a +// single hex allocation for the profiler's "trace id" label instead of one per +// span. +func (t *traceID) hexEncodedCached() string { + if t.hexEncoded == "" { + t.cacheHex() + } + return t.hexEncoded +} + // SpanContext represents a span state that can propagate to descendant spans // and across process boundaries. It contains all the information needed to // spawn a direct descendant of the span that it belongs to. It can be used @@ -303,6 +317,13 @@ func newSpanContext(span *Span, parent *SpanContext) *SpanContext { tUp := uint64(uint32(id128)) << 32 // We need the time at the upper 32 bits of the uint context.traceID.SetUpper(tUp) } + // A child span shares its parent's trace ID, so reuse the parent's cached hex + // string (lazily populated on the pprof "trace id" label path) rather than + // re-encoding it. This keeps the whole trace at a single hex allocation. The + // value equality check guards against ever copying a mismatched hex. + if parent != nil && parent.traceID.hexEncoded != "" && context.traceID.value == parent.traceID.value { + context.traceID.hexEncoded = parent.traceID.hexEncoded + } if context.trace == nil { context.trace = newTrace() } diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index 9ccdcb9927..e4e3aad628 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -3640,8 +3640,11 @@ func TestSpanContextDebugLoggingSecurity(t *testing.T) { // It covers both hex-cache states a shared SpanContext can be in when injected // concurrently: // -// - cold cache: a locally started span. newSpanContext does not populate -// hexEncoded, so every UpperHex() takes the non-caching fallback. +// - cold cache: a locally started span with the pprof label path disabled. +// newSpanContext does not populate hexEncoded, so every UpperHex() takes the +// non-caching fallback. Code hotspots must be off here: when it (or AppSec) +// is enabled, applyPPROFLabels warms the hex cache at StartSpan (safely, +// before the span is shared) via hexEncodedCached. // - hot cache: an extracted context. extractTextMap finalizes the traceID via // cacheHex, so UpperHex() returns the cached string. // @@ -3653,7 +3656,11 @@ func TestConcurrentInjectTraceIDHex(t *testing.T) { t.Setenv(envPropagationStyleExtract, "datadog") t.Setenv("DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED", "true") - tracer, _, _, stop, err := startTestTracer(t) + // Disable code hotspots so locally started spans keep a cold hex cache; with + // it (or AppSec) enabled, applyPPROFLabels warms the cache at StartSpan and + // the "cold cache" subtest below could no longer exercise the non-caching + // HexEncoded read path. + tracer, _, _, stop, err := startTestTracer(t, WithProfilerCodeHotspots(false)) require.NoError(t, err) defer stop() diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 10c6d38903..261db8ee63 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -1018,11 +1018,7 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp log.Debug("Started Span: %v, Operation: %s, Resource: %s, Tags: %v, %v", //nolint:gocritic // Debug logging needs full span representation span, span.name, span.resource, &span.meta, span.metrics) } - if cSnap.ProfilerHotspotsEnabled || cSnap.ProfilerEndpoints { - t.applyPPROFLabels(span.pprofCtxRestore, span, cSnap) - } else { - span.pprofCtxRestore = nil - } + t.applyPPROFLabels(span.pprofCtxRestore, span, cSnap) if cSnap.DebugAbandonedSpans { select { case t.abandonedSpansDebugger.In <- newAbandonedSpanCandidate(span, false): @@ -1052,13 +1048,21 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp // many times each endpoint is called. // +checklocksignore — Initialization time, called from StartSpan before span is shared. 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() + if !correlate && !snap.ProfilerEndpoints { + // No feature needs pprof labels; nothing to restore when the span finishes. + span.pprofCtxRestore = nil + return + } // Important: The label keys are ordered alphabetically to take advantage of // an upstream optimization that landed in go1.24. This results in ~10% // better performance on BenchmarkStartSpan. See // https://go-review.googlesource.com/c/go/+/574516 for more information. - labels := make([]string, 0, 3*2 /* 3 key value pairs */) + labels := make([]string, 0, 4*2 /* up to 4 key value pairs */) localRootSpan := span.Root() - if snap.ProfilerHotspotsEnabled && localRootSpan != nil { + if correlate && localRootSpan != nil { spanID := localRootSpan.getSpanID() labels = append(labels, traceprof.LocalRootSpanID, strconv.FormatUint(spanID, 10)) } @@ -1077,6 +1081,11 @@ func (t *tracer) applyPPROFLabels(ctx gocontext.Context, span *Span, snap intern } } } + if correlate { + // hexEncodedCached memoizes the hex on the (not-yet-shared) context so + // child spans reuse it: one hex allocation per trace, not per span. + labels = append(labels, traceprof.TraceID, span.context.traceID.hexEncodedCached()) + } if len(labels) > 0 { pprofActive := pprof.WithLabels(ctx, pprof.Labels(labels...)) span.pprofCtxRestore = ctx diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index a52eadac72..3dff9aa099 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -38,6 +38,7 @@ import ( traceinternal "github.com/DataDog/dd-trace-go/v2/ddtrace/tracer/internal" tracertest "github.com/DataDog/dd-trace-go/v2/ddtrace/x/agenttest" "github.com/DataDog/dd-trace-go/v2/internal" + "github.com/DataDog/dd-trace-go/v2/internal/appsec" internalconfig "github.com/DataDog/dd-trace-go/v2/internal/config" "github.com/DataDog/dd-trace-go/v2/internal/globalconfig" "github.com/DataDog/dd-trace-go/v2/internal/locking" @@ -45,6 +46,7 @@ import ( "github.com/DataDog/dd-trace-go/v2/internal/processtags" "github.com/DataDog/dd-trace-go/v2/internal/remoteconfig" "github.com/DataDog/dd-trace-go/v2/internal/statsdtest" + "github.com/DataDog/dd-trace-go/v2/internal/traceprof" "github.com/DataDog/datadog-go/v5/statsd" "github.com/stretchr/testify/assert" @@ -3222,6 +3224,91 @@ func TestPprofLabels(t *testing.T) { }) } +// TestApplyPPROFLabelsTraceID verifies the pprof label gating matrix: the whole +// 128-bit "trace id" (32-char lowercase hex) and "local root span id" are +// emitted for code hotspots; "span id" stays hotspots-only; "trace endpoint" +// stays endpoints-only; and nothing is emitted when no feature is enabled. +func TestApplyPPROFLabelsTraceID(t *testing.T) { + // WithAppSecEnabled(false) disables AppSec so the code-hotspots and endpoints + // gates can be tested deterministically; assert the global state to be safe. + tr, err := newTracer(WithAppSecEnabled(false)) + require.NoError(t, err) + defer tr.Stop() + require.False(t, appsec.Enabled(), "appsec must be off for the deterministic gating cases") + + span := tr.StartSpan("web.request", ResourceName("/things"), SpanType(ext.SpanTypeWeb)) + defer span.Finish() + + traceID := span.context.TraceID() + require.Regexp(t, "^[0-9a-f]{32}$", traceID, "trace id must be the whole 128-bit id as lowercase hex") + localRoot := strconv.FormatUint(span.Root().getSpanID(), 10) + spanID := strconv.FormatUint(span.spanID, 10) + + // apply resets the label context and (re)applies the labels for snap. + apply := func(snap internalconfig.SpanStartSnapshot) context.Context { + span.pprofCtxActive = nil + tr.applyPPROFLabels(context.Background(), span, snap) + return span.pprofCtxActive + } + present := func(t *testing.T, ctx context.Context, key, want string) { + t.Helper() + got, ok := pprof.Label(ctx, key) + require.Truef(t, ok, "label %q should be present", key) + require.Equalf(t, want, got, "value of label %q", key) + } + absent := func(t *testing.T, ctx context.Context, keys ...string) { + t.Helper() + for _, key := range keys { + _, ok := pprof.Label(ctx, key) + require.Falsef(t, ok, "label %q should be absent", key) + } + } + + t.Run("hotspots", func(t *testing.T) { + ctx := apply(internalconfig.SpanStartSnapshot{ProfilerHotspotsEnabled: true}) + require.NotNil(t, ctx) + present(t, ctx, traceprof.TraceID, traceID) + present(t, ctx, traceprof.LocalRootSpanID, localRoot) + present(t, ctx, traceprof.SpanID, spanID) + absent(t, ctx, traceprof.TraceEndpoint) + }) + t.Run("endpoints-only", func(t *testing.T) { + ctx := apply(internalconfig.SpanStartSnapshot{ProfilerEndpoints: true}) + require.NotNil(t, ctx) + present(t, ctx, traceprof.TraceEndpoint, "/things") + absent(t, ctx, traceprof.TraceID, traceprof.LocalRootSpanID, traceprof.SpanID) + }) + t.Run("none", func(t *testing.T) { + require.Nil(t, apply(internalconfig.SpanStartSnapshot{})) + }) +} + +// TestApplyPPROFLabelsTraceIDAppSec verifies that enabling AppSec (with the +// profiler features off) still emits "trace id" and "local root span id" for +// trace/security correlation, but not the hotspots-only "span id". +func TestApplyPPROFLabelsTraceIDAppSec(t *testing.T) { + if err := Start(WithAppSecEnabled(true), WithProfilerCodeHotspots(false), WithProfilerEndpoints(false)); err != nil { + t.Fatal(err) + } + defer Stop() + if !appsec.Enabled() { + t.Skip("appsec is not enabled on this platform; skipping appsec-only pprof label test") + } + + span := StartSpan("web.request") + defer span.Finish() + + ctx := span.pprofCtxActive + require.NotNil(t, ctx) + got, ok := pprof.Label(ctx, traceprof.TraceID) + require.True(t, ok, "trace id label should be present under appsec") + require.Equal(t, span.context.TraceID(), got) + _, ok = pprof.Label(ctx, traceprof.LocalRootSpanID) + require.True(t, ok, "local root span id label should be present under appsec") + _, ok = pprof.Label(ctx, traceprof.SpanID) + require.False(t, ok, "span id is hotspots-only and must be absent under appsec-only") +} + func TestNoopTracerStartSpan(t *testing.T) { r, w, err := os.Pipe() if err != nil { diff --git a/internal/traceprof/traceprof.go b/internal/traceprof/traceprof.go index dc55bc889f..e57a7d5499 100644 --- a/internal/traceprof/traceprof.go +++ b/internal/traceprof/traceprof.go @@ -11,6 +11,11 @@ const ( SpanID = "span id" LocalRootSpanID = "local root span id" TraceEndpoint = "trace endpoint" + // TraceID is the full 128-bit trace ID encoded as a 32-character lowercase + // hex string (unlike SpanID/LocalRootSpanID, which are 64-bit decimal). It + // lets consumers correlate a sample with the whole trace, not just the local + // root span. + TraceID = "trace id" ) // env variables used to control cross-cutting tracer/profiling features.