util/tracing: introduce helper Span.IsNoop() method

This was hidden in an internal helper, but it's such a common concern
that it deserves better ergonomics, and it even deserves to be public.

Release note: None

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
 		}
 	}

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 {

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()

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,15 +71,15 @@ 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
 }
 
 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

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()

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()