fix(orchestrion): allow the experimental span pool together with orchestrion GLS

Final PR in the GLS span-pool stack. With the reclaim signal decoupled from the
recyclable span (cell-based, captured at push time) and Peek skipping finished
entries, the two reasons #4891 gated span pool and orchestrion GLS as mutually
exclusive no longer hold: a recycled span can neither leak a stale GLS entry nor
resurface as the active span. Remove the gate (the shouldDisableSpanPool helper,
its newConfig call, and its unit test) so WithSpanPool(true) /
DD_TRACER_EXPERIMENTAL_SPAN_POOL_ENABLED is honored under orchestrion.

Add TestGLSNoHeapLeakWithSpanPool as the coexistence regression gate. It enables
the pool via WithSpanPool(true) and runs glsleak.MeasureLeakLiveInject: the
realistic franz-go shape that injects each span while live and finishes it after
the worker pushes it, respecting the span pool's "do not touch a span after
Finish" contract. Run under -race in CI it guards both the leak and the
span-pool-vs-GLS data races. The existing finish-then-inject TestGLSNoHeapLeak
stays as the no-pool gate (its finish-before-inject order is a deliberate
use-after-Finish that the pool legitimately recycles, so it is not run pooled).

Refs: DataDog/orchestrion#782

Environment: Datadog workspace

Co-authored-by: Kemal Akkoyun <kakkoyun@users.noreply.github.com>

Author: kakkoyun

ddtrace/tracer/option.go

@@ -234,18 +234,6 @@ func newConfig(opts ...StartOption) (*config, error) {
 		}
 		fn(c)
 	}
-	// The experimental span pool and Orchestrion's GLS weave are mutually
-	// exclusive. Span pooling recycles a finished span via sync.Pool; under
-	// Orchestrion that span may still be referenced from a goroutine-local
-	// storage (GLS) stack whose stale entry has not been drained, so reusing it
-	// can resurface a recycled span or leak the entry (see orchestrion#782).
-	// Until the reclaim signal is decoupled from the pooled span, disable
-	// pooling when Orchestrion is active and warn once. Checked after the option
-	// loop so an explicit WithSpanPool(true) is gated too.
-	if shouldDisableSpanPool(c.spanPoolEnabled, orchestrion.Enabled()) {
-		c.spanPoolEnabled = false
-		log.Warn("the experimental span pool (DD_TRACER_EXPERIMENTAL_SPAN_POOL_ENABLED / WithSpanPool) is incompatible with Orchestrion and has been disabled")
-	}
 	rawAgentURL := c.internalConfig.RawAgentURL()
 	if c.httpClient == nil || orchestrion.Enabled() {
 		if orchestrion.Enabled() && c.httpClient != nil {
@@ -377,16 +365,6 @@ func newConfig(opts ...StartOption) (*config, error) {
 	return c, nil
 }
 
-// shouldDisableSpanPool reports whether the experimental span pool must be
-// turned off because Orchestrion's GLS weave is active. The two are mutually
-// exclusive: pooling can recycle a finished span whose stale GLS entry has not
-// yet been drained, which the GLS reclaim path does not yet tolerate
-// (orchestrion#782). It is a pure helper so the gate is unit-testable without an
-// Orchestrion build (orchestrion.Enabled() is a build-time constant).
-func shouldDisableSpanPool(spanPoolEnabled, orchestrionEnabled bool) bool {
-	return spanPoolEnabled && orchestrionEnabled
-}
-
 func llmobsAgentlessEnabledFromEnv() *bool {
 	v, ok := internal.BoolEnvNoDefault(envLLMObsAgentlessEnabled)
 	if !ok {

ddtrace/tracer/span_pool_gate_test.go

@@ -51,3 +51,28 @@ func TestGLSNoHeapLeak(t *testing.T) {
 			"one span per record) — the contextStack.Push reclaim in ddtrace/tracer/orchestrion.yml regressed",
 		r.PerRecord)
 }
+
+// TestGLSNoHeapLeakWithSpanPool is the regression gate for running the
+// experimental span pool together with orchestrion GLS (the combination #4891
+// gated off and this stack re-enables). It uses the live-inject workload, which
+// respects the span pool's "do not use a span after Finish" contract, and
+// enables pooling via WithSpanPool(true). With the decoupled, cell-based reclaim
+// the worker's GLS stack stays flat even though every finished span is recycled
+// by the pool; without it, recycled spans would either leak (stale entries never
+// drained) or resurface as the wrong active span. Run under -race in CI, it also
+// guards against the span-pool-vs-GLS data races from the review.
+func TestGLSNoHeapLeakWithSpanPool(t *testing.T) {
+	if !orchestrionEnabled {
+		t.Skip("GLS only exists in orchestrion builds")
+	}
+	require.True(t, built.WithOrchestrion)
+
+	require.NoError(t, tracer.Start(tracer.WithLogStartup(false), tracer.WithSpanPool(true)))
+	defer tracer.Stop()
+
+	r := glsleak.MeasureLeakLiveInject(200_000)
+	require.Lessf(t, r.PerRecord, glsleak.MaxRetainedObjectsPerRecord,
+		"GLS span leak with span pool: %.3f retained heap objects/record (want flat ~0) — "+
+			"the decoupled reclaim regressed, or span pool + orchestrion GLS no longer coexist safely",
+		r.PerRecord)
+}

internal/orchestrion/_integration/gls-leak/leak_test.go

@@ -42,6 +42,11 @@ import (
 //
 // Without the injected fix this goroutine's GLS grows by one entry per record
 // (an unbounded leak proportional to the record count); with it, depth stays ~1.
+//
+// This test deliberately finishes the span BEFORE re-injecting it, so it is not
+// run with the experimental span pool, which legitimately recycles a finished
+// span (touching it afterward is out of the pool's contract). The span-pool +
+// GLS coexistence is covered by the live-inject TestGLSNoHeapLeakWithSpanPool.
 func TestSpanGLSNoLeakCrossGoroutine(t *testing.T) {
 	if !orchestrionEnabled {
 		t.Skip("GLS only exists in orchestrion builds")
@@ -96,6 +101,10 @@ func TestSpanGLSNoLeakCrossGoroutine(t *testing.T) {
 // additionally catches a regression that still pushes but stops reclaiming in a
 // way the bounded-depth check might not. The runnable gls-leak command exercises
 // the same helper.
+//
+// Like TestSpanGLSNoLeakCrossGoroutine, this uses the finish-then-inject order
+// and is therefore not run under the experimental span pool (which recycles the
+// finished span); TestGLSNoHeapLeakWithSpanPool covers the pooled, live-inject path.
 func TestSpanGLSNoHeapLeakCrossGoroutine(t *testing.T) {
 	if !orchestrionEnabled {
 		t.Skip("GLS only exists in orchestrion builds")

internal/orchestrion/_integration/gls/span_gls_test.go

@@ -93,3 +93,70 @@ func MeasureLeak(n int) Result {
 		PerRecord: float64(objects) / float64(n),
 	}
 }
+
+// MeasureLeakLiveInject runs the same cross-goroutine GLS-leak measurement as
+// MeasureLeak, but injects each span while it is still LIVE and finishes it only
+// after the worker has pushed it. This matches the real franz-go/Kafka shape
+// (a span is put into the context while in flight, then finished) and, unlike
+// MeasureLeak's finish-then-inject order, respects the span pool's "do not touch
+// a span after Finish" contract. It is therefore the workload used to assert
+// that the experimental span pool and orchestrion GLS coexist without leaking
+// (orchestrion#782): the reclaim cell is captured at push time while the span is
+// live, finish marks it, and the worker's next push drains the stale entry even
+// after the span object has been recycled by the pool.
+//
+// A per-item handshake orders the worker's push ahead of the owner's Finish, so
+// the worker never reads a span's fields while the owner finishes (or the pool
+// recycles) it: the workload stays data-race-free under -race while still
+// exercising pool reuse and cross-goroutine reclaim.
+//
+// The tracer must already be started by the caller (with the span pool enabled,
+// e.g. tracer.WithSpanPool(true)). n <= 0 returns a zero Result.
+func MeasureLeakLiveInject(n int) Result {
+	if n <= 0 {
+		return Result{Records: n}
+	}
+	base := context.Background()
+
+	run := func() {
+		spanCh := make(chan *tracer.Span)
+		pushedCh := make(chan struct{})
+		var wg sync.WaitGroup
+		wg.Go(func() { // worker: pushes the live span onto its own GLS stack
+			for s := range spanCh {
+				_ = tracer.ContextWithSpan(base, s)
+				pushedCh <- struct{}{} // signal: done reading s, owner may finish it
+			}
+		})
+		for range n { // owner: creates and (after the worker pushed) finishes the span
+			s := tracer.StartSpan("kafka.consume")
+			spanCh <- s // hand the LIVE span to the worker
+			<-pushedCh  // wait until the worker pushed it (orders push before finish)
+			s.Finish()  // finish on the owner; the worker's popper is a no-op here
+		}
+		close(spanCh)
+		wg.Wait()
+	}
+
+	run() // warm up so first-run/lazy allocations don't count toward the delta
+
+	runtime.GC()
+	var before runtime.MemStats
+	runtime.ReadMemStats(&before)
+
+	run()
+
+	tracer.Flush() // drop buffered spans so only a GLS leak can retain them
+	runtime.GC()
+	runtime.GC()
+	var after runtime.MemStats
+	runtime.ReadMemStats(&after)
+
+	objects := int64(after.HeapObjects) - int64(before.HeapObjects)
+	return Result{
+		Records:   n,
+		Objects:   objects,
+		Bytes:     int64(after.HeapInuse) - int64(before.HeapInuse),
+		PerRecord: float64(objects) / float64(n),
+	}
+}