Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions ddtrace/tracer/spancontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,20 @@
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
Expand Down Expand Up @@ -303,6 +317,13 @@
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()
}
Expand Down Expand Up @@ -646,7 +667,7 @@
// reasonable as span is actually way bigger, and avoids re-allocating
// over and over. Could be fine-tuned at runtime.
traceStartSize = 10
traceMaxSize = internalconfig.TraceMaxSize

Check failure on line 670 in ddtrace/tracer/spancontext.go

View workflow job for this annotation

GitHub Actions / checklocks

may require checklocks annotation for mu, used with lock held 100% of the time
)

// samplingPriorityCache holds pre-allocated pointers for the four standard
Expand Down
13 changes: 10 additions & 3 deletions ddtrace/tracer/textmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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()

Expand Down
23 changes: 16 additions & 7 deletions ddtrace/tracer/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 && !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))
}
Expand All @@ -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
Expand Down
87 changes: 87 additions & 0 deletions ddtrace/tracer/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ 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"
"github.com/DataDog/dd-trace-go/v2/internal/log"
"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"
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions internal/traceprof/traceprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading