diff --git a/pkg/util/tracing/context.go b/pkg/util/tracing/context.go index ab5bff7ea153..5b5b54335920 100644 --- a/pkg/util/tracing/context.go +++ b/pkg/util/tracing/context.go @@ -59,7 +59,7 @@ func maybeWrapCtx(ctx context.Context, octx *optimizedContext, sp *Span) (contex } // NB: we check sp != nil explicitly because some callers want to remove a // Span from a Context, and thus pass nil. - if sp != nil && sp.i.isNoop() { + if sp != nil && sp.IsNoop() { // If the context originally had the noop span, and we would now be wrapping // the noop span in it again, we don't have to wrap at all and can save an // allocation. @@ -68,7 +68,7 @@ func maybeWrapCtx(ctx context.Context, octx *optimizedContext, sp *Span) (contex // constitute a bug: A real, non-recording span might later start recording. // Besides, the caller expects to get their own span, and will .Finish() it, // leading to an extra, premature call to Finish(). - if ctxSp := SpanFromContext(ctx); ctxSp != nil && ctxSp.i.isNoop() { + if ctxSp := SpanFromContext(ctx); ctxSp != nil && ctxSp.IsNoop() { return ctx, sp } } diff --git a/pkg/util/tracing/grpc_interceptor.go b/pkg/util/tracing/grpc_interceptor.go index f517a1f47ff7..9676cf5c966c 100644 --- a/pkg/util/tracing/grpc_interceptor.go +++ b/pkg/util/tracing/grpc_interceptor.go @@ -197,7 +197,7 @@ func (ss *tracingServerStream) Context() context.Context { // // See #17177. func spanInclusionFuncForClient(parent *Span) bool { - return parent != nil && !parent.i.isNoop() + return parent != nil && !parent.IsNoop() } func injectSpanMeta(ctx context.Context, tracer *Tracer, clientSpan *Span) context.Context { diff --git a/pkg/util/tracing/span.go b/pkg/util/tracing/span.go index 13e7a196310f..bf575212dde4 100644 --- a/pkg/util/tracing/span.go +++ b/pkg/util/tracing/span.go @@ -55,6 +55,13 @@ type Span struct { numFinishCalled int32 // atomic } +// IsNoop returns true if this span is a black hole - it doesn't correspond to a +// CRDB span and it doesn't output either to an OpenTelemetry tracer, or to +// net.Trace. +func (sp *Span) IsNoop() bool { + return sp.i.isNoop() +} + func (sp *Span) done() bool { if sp == nil { return true @@ -78,7 +85,7 @@ func (sp *Span) SetOperationName(operationName string) { // Finish idempotently marks the Span as completed (at which point it will // silently drop any new data added to it). Finishing a nil *Span is a noop. func (sp *Span) Finish() { - if sp == nil || sp.i.isNoop() || atomic.AddInt32(&sp.numFinishCalled, 1) != 1 { + if sp == nil || sp.IsNoop() || atomic.AddInt32(&sp.numFinishCalled, 1) != 1 { return } sp.i.Finish() diff --git a/pkg/util/tracing/span_options.go b/pkg/util/tracing/span_options.go index e4742bc9d57b..a8296ea5b28a 100644 --- a/pkg/util/tracing/span_options.go +++ b/pkg/util/tracing/span_options.go @@ -53,7 +53,7 @@ type spanOptions struct { } func (opts *spanOptions) parentTraceID() uint64 { - if opts.Parent != nil && !opts.Parent.i.isNoop() { + if opts.Parent != nil && !opts.Parent.IsNoop() { return opts.Parent.i.crdb.traceID } else if !opts.RemoteParent.Empty() { return opts.RemoteParent.traceID @@ -62,7 +62,7 @@ func (opts *spanOptions) parentTraceID() uint64 { } func (opts *spanOptions) parentSpanID() uint64 { - if opts.Parent != nil && !opts.Parent.i.isNoop() { + if opts.Parent != nil && !opts.Parent.IsNoop() { return opts.Parent.i.crdb.spanID } else if !opts.RemoteParent.Empty() { return opts.RemoteParent.spanID @@ -71,7 +71,7 @@ func (opts *spanOptions) parentSpanID() uint64 { } func (opts *spanOptions) deriveRootSpan() *crdbSpan { - if opts.Parent != nil && !opts.Parent.i.isNoop() { + if opts.Parent != nil && !opts.Parent.IsNoop() { return opts.Parent.i.crdb.rootSpan } return nil @@ -79,7 +79,7 @@ func (opts *spanOptions) deriveRootSpan() *crdbSpan { func (opts *spanOptions) recordingType() RecordingType { recordingType := RecordingOff - if opts.Parent != nil && !opts.Parent.i.isNoop() { + if opts.Parent != nil && !opts.Parent.IsNoop() { recordingType = opts.Parent.i.crdb.recordingType() } else if !opts.RemoteParent.Empty() { recordingType = opts.RemoteParent.recordingType diff --git a/pkg/util/tracing/tracer.go b/pkg/util/tracing/tracer.go index 8e04f8e78b29..cad2efc90d06 100644 --- a/pkg/util/tracing/tracer.go +++ b/pkg/util/tracing/tracer.go @@ -443,7 +443,7 @@ func (t *Tracer) startSpanGeneric( opts.LogTags = logtags.FromContext(ctx) } - if opts.LogTags == nil && opts.Parent != nil && !opts.Parent.i.isNoop() { + if opts.LogTags == nil && opts.Parent != nil && !opts.Parent.IsNoop() { // If no log tags are specified in the options, use the parent // span's, if any. This behavior is the reason logTags are // fundamentally different from tags, which are strictly per span, @@ -566,7 +566,7 @@ func (t *Tracer) startSpanGeneric( // // NB: this could be optimized. if opts.Parent != nil { - if !opts.Parent.i.isNoop() { + if !opts.Parent.IsNoop() { opts.Parent.i.crdb.mu.Lock() m := opts.Parent.i.crdb.mu.baggage opts.Parent.i.crdb.mu.Unlock() diff --git a/pkg/util/tracing/tracer_test.go b/pkg/util/tracing/tracer_test.go index c5673c54a46d..73021faf2f59 100644 --- a/pkg/util/tracing/tracer_test.go +++ b/pkg/util/tracing/tracer_test.go @@ -35,17 +35,17 @@ func TestStartSpanAlwaysTrace(t *testing.T) { require.True(t, nilMeta.Empty()) sp := tr.StartSpan("foo", WithParentAndManualCollection(nilMeta)) require.False(t, sp.IsVerbose()) // parent was not verbose, so neither is sp - require.False(t, sp.i.isNoop()) + require.False(t, sp.IsNoop()) sp = tr.StartSpan("foo", WithParentAndAutoCollection(tr.noopSpan)) require.False(t, sp.IsVerbose()) // parent was not verbose - require.False(t, sp.i.isNoop()) + require.False(t, sp.IsNoop()) } func TestTracerRecording(t *testing.T) { tr := NewTracer() noop1 := tr.StartSpan("noop") - if !noop1.i.isNoop() { + if !noop1.IsNoop() { t.Error("expected noop Span") } noop1.Record("hello") @@ -54,14 +54,14 @@ func TestTracerRecording(t *testing.T) { require.Equal(t, Recording(nil), noop1.GetRecording()) noop2 := tr.StartSpan("noop2", WithParentAndManualCollection(noop1.Meta())) - if !noop2.i.isNoop() { + if !noop2.IsNoop() { t.Error("expected noop child Span") } noop2.Finish() noop1.Finish() s1 := tr.StartSpan("a", WithForceRealSpan()) - if s1.i.isNoop() { + if s1.IsNoop() { t.Error("WithForceRealSpan (but not recording) Span should not be noop") } if s1.IsVerbose() { @@ -84,7 +84,7 @@ func TestTracerRecording(t *testing.T) { // Real parent --> real child. real3 := tr.StartSpan("noop3", WithParentAndManualCollection(s1.Meta())) - if real3.i.isNoop() { + if real3.IsNoop() { t.Error("expected real child Span") } real3.Finish() @@ -229,7 +229,7 @@ func TestTracerInjectExtract(t *testing.T) { // Verify that noop spans become noop spans on the remote side. noop1 := tr.StartSpan("noop") - if !noop1.i.isNoop() { + if !noop1.IsNoop() { t.Fatalf("expected noop Span: %+v", noop1) } carrier := metadataCarrier{metadata.MD{}} @@ -248,7 +248,7 @@ func TestTracerInjectExtract(t *testing.T) { t.Errorf("expected no-op span meta: %v", wireSpanMeta) } noop2 := tr2.StartSpan("remote op", WithParentAndManualCollection(wireSpanMeta)) - if !noop2.i.isNoop() { + if !noop2.IsNoop() { t.Fatalf("expected noop Span: %+v", noop2) } noop1.Finish()