From 715239e131e13f7ef5141eadd906c52ac15f84d1 Mon Sep 17 00:00:00 2001 From: Ankur Shrivastava Date: Mon, 27 Apr 2026 12:40:27 +0800 Subject: [PATCH 1/3] refactor: remove ensureSampled, use natural OTel root spans for worker ticks The Tracing middleware previously injected a fake remote sampled span context to force ParentBased samplers to always sample worker spans. Remove this hack and let NewInternalSpan create a natural root span per tick, with sampling governed by the global TracerProvider's sampler. --- middleware/tracing.go | 31 ++--------------------------- middleware/tracing_test.go | 40 -------------------------------------- 2 files changed, 2 insertions(+), 69 deletions(-) diff --git a/middleware/tracing.go b/middleware/tracing.go index e52084c..557a88e 100644 --- a/middleware/tracing.go +++ b/middleware/tracing.go @@ -2,7 +2,6 @@ package middleware import ( "context" - "crypto/rand" "github.com/go-coldbrew/log" "github.com/go-coldbrew/tracing" @@ -10,20 +9,14 @@ import ( oteltrace "go.opentelemetry.io/otel/trace" ) -// Tracing creates an OTEL span per cycle via go-coldbrew/tracing. +// Tracing creates an OTEL root span per cycle via go-coldbrew/tracing. // The span is named "worker::cycle" and records errors. -// -// Worker spans are always sampled regardless of the global -// TracerProvider's sampler. This prevents silent span drops when -// using ParentBased(TraceIDRatioBased(ratio)), where worker root -// spans (which have no parent) would otherwise be probabilistically -// dropped. +// Sampling is determined by the global TracerProvider's sampler. // // The OTEL trace ID is injected into the log context as "trace" // for correlation with the tracing backend. func Tracing() workers.Middleware { return func(ctx context.Context, info *workers.WorkerInfo, next workers.CycleFunc) error { - ctx = ensureSampled(ctx) span, ctx := tracing.NewInternalSpan(ctx, "worker:"+info.GetName()+":cycle") defer span.Finish() @@ -38,23 +31,3 @@ func Tracing() workers.Middleware { return err } } - -// ensureSampled injects a sampled remote span context so that -// ParentBased samplers always sample the next span created from -// this context. If the context already has a sampled span, it is -// returned unchanged. -func ensureSampled(ctx context.Context) context.Context { - if oteltrace.SpanFromContext(ctx).SpanContext().IsSampled() { - return ctx - } - var traceID oteltrace.TraceID - var spanID oteltrace.SpanID - _, _ = rand.Read(traceID[:]) - _, _ = rand.Read(spanID[:]) - return oteltrace.ContextWithRemoteSpanContext(ctx, oteltrace.NewSpanContext(oteltrace.SpanContextConfig{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: oteltrace.FlagsSampled, - Remote: true, - })) -} diff --git a/middleware/tracing_test.go b/middleware/tracing_test.go index 287f048..265190f 100644 --- a/middleware/tracing_test.go +++ b/middleware/tracing_test.go @@ -6,7 +6,6 @@ import ( "github.com/go-coldbrew/workers" "github.com/stretchr/testify/assert" - oteltrace "go.opentelemetry.io/otel/trace" ) func TestTracing_NoPanic(t *testing.T) { @@ -33,42 +32,3 @@ func TestTracing_PropagatesError(t *testing.T) { assert.ErrorIs(t, err, assert.AnError) } - -func TestEnsureSampled(t *testing.T) { - // From a bare context (no span), ensureSampled should inject a - // sampled remote span context. - ctx := ensureSampled(context.Background()) - sc := oteltrace.SpanContextFromContext(ctx) - - assert.True(t, sc.IsSampled(), "should be sampled") - assert.True(t, sc.IsRemote(), "should be remote") - assert.True(t, sc.HasTraceID(), "should have a trace ID") - assert.True(t, sc.HasSpanID(), "should have a span ID") -} - -func TestEnsureSampled_AlreadySampled(t *testing.T) { - // If context already has a sampled span, ensureSampled is a no-op. - existing := oteltrace.NewSpanContext(oteltrace.SpanContextConfig{ - TraceID: oteltrace.TraceID{1, 2, 3}, - SpanID: oteltrace.SpanID{4, 5, 6}, - TraceFlags: oteltrace.FlagsSampled, - Remote: true, - }) - ctx := oteltrace.ContextWithRemoteSpanContext(context.Background(), existing) - - ctx = ensureSampled(ctx) - sc := oteltrace.SpanContextFromContext(ctx) - - assert.Equal(t, existing.TraceID(), sc.TraceID(), "should keep existing trace ID") - assert.Equal(t, existing.SpanID(), sc.SpanID(), "should keep existing span ID") -} - -func TestEnsureSampled_UniqueIDs(t *testing.T) { - ctx1 := ensureSampled(context.Background()) - ctx2 := ensureSampled(context.Background()) - - sc1 := oteltrace.SpanContextFromContext(ctx1) - sc2 := oteltrace.SpanContextFromContext(ctx2) - - assert.NotEqual(t, sc1.TraceID(), sc2.TraceID(), "each call should generate unique trace IDs") -} From 18e7cd093c0cee711838e24a613f912f0b7f0dc6 Mon Sep 17 00:00:00 2001 From: Ankur Shrivastava Date: Mon, 27 Apr 2026 12:53:58 +0800 Subject: [PATCH 2/3] docs: regenerate README.md after ensureSampled removal --- middleware/README.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/middleware/README.md b/middleware/README.md index de377c2..35ede2e 100644 --- a/middleware/README.md +++ b/middleware/README.md @@ -115,15 +115,13 @@ func Timeout(d time.Duration) workers.Middleware Timeout enforces a per\-cycle deadline. Distinct from \[workers.Worker.WithTimeout\] which controls graceful shutdown. -## func [Tracing]() +## func [Tracing]() ```go func Tracing() workers.Middleware ``` -Tracing creates an OTEL span per cycle via go\-coldbrew/tracing. The span is named "worker:\:cycle" and records errors. - -Worker spans are always sampled regardless of the global TracerProvider's sampler. This prevents silent span drops when using ParentBased\(TraceIDRatioBased\(ratio\)\), where worker root spans \(which have no parent\) would otherwise be probabilistically dropped. +Tracing creates an OTEL root span per cycle via go\-coldbrew/tracing. The span is named "worker:\:cycle" and records errors. Sampling is determined by the global TracerProvider's sampler. The OTEL trace ID is injected into the log context as "trace" for correlation with the tracing backend. From 91227d57719cf78557c3ff9cabf835fb5d2f3be3 Mon Sep 17 00:00:00 2001 From: Ankur Shrivastava Date: Mon, 27 Apr 2026 13:04:28 +0800 Subject: [PATCH 3/3] docs: clarify Tracing() span is typically a root, not guaranteed --- middleware/README.md | 4 ++-- middleware/tracing.go | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/middleware/README.md b/middleware/README.md index 35ede2e..d6c4f73 100644 --- a/middleware/README.md +++ b/middleware/README.md @@ -115,13 +115,13 @@ func Timeout(d time.Duration) workers.Middleware Timeout enforces a per\-cycle deadline. Distinct from \[workers.Worker.WithTimeout\] which controls graceful shutdown. -## func [Tracing]() +## func [Tracing]() ```go func Tracing() workers.Middleware ``` -Tracing creates an OTEL root span per cycle via go\-coldbrew/tracing. The span is named "worker:\:cycle" and records errors. Sampling is determined by the global TracerProvider's sampler. +Tracing creates an OTEL span per cycle via go\-coldbrew/tracing. The span is named "worker:\:cycle" and records errors. In typical use the span is a trace root because worker contexts carry no parent span; sampling is determined by the global TracerProvider's sampler. The OTEL trace ID is injected into the log context as "trace" for correlation with the tracing backend. diff --git a/middleware/tracing.go b/middleware/tracing.go index 557a88e..d877259 100644 --- a/middleware/tracing.go +++ b/middleware/tracing.go @@ -9,9 +9,11 @@ import ( oteltrace "go.opentelemetry.io/otel/trace" ) -// Tracing creates an OTEL root span per cycle via go-coldbrew/tracing. +// Tracing creates an OTEL span per cycle via go-coldbrew/tracing. // The span is named "worker::cycle" and records errors. -// Sampling is determined by the global TracerProvider's sampler. +// In typical use the span is a trace root because worker contexts +// carry no parent span; sampling is determined by the global +// TracerProvider's sampler. // // The OTEL trace ID is injected into the log context as "trace" // for correlation with the tracing backend.