Add span.TracerProvider() (open-telemetry#2009)

* Ensure that no-op tracer still progates non-recording spans Signed-off-by: Anthony J Mirabella <[email protected]> * Add `TracerProvider` to the `trace.Span` interface Signed-off-by: Anthony J Mirabella <[email protected]> * Remove config from oteltest.Tracer as it can be accessed through the provider Signed-off-by: Anthony J Mirabella <[email protected]>
hstan · Jun 17, 2021 · 39fe809 · 39fe809
1 parent d020e1a
commit 39fe809
Show file tree

Hide file tree

Showing 13 changed files with 98 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Creates package `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` implementing a gRPC `otlpmetric.Client` and offers convenience functions, `New` and `NewUnstarted`, to create an `otlpmetric.Exporter`.(#1991)
 - Added `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` exporter. (#2005)
 - Added `go.opentelemetry.io/otel/exporters/stdout/stdoutmetric` exporter. (#2005)
+- Added a `TracerProvider()` method to the `"go.opentelemetry.io/otel/trace".Span` interface. This can be used to obtain a `TracerProvider` from a given span that utilizes the same trace processing pipeline.  (#2009)
 
 ### Changed
 
@@ -108,6 +109,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Remove the `Tracer` method from the `Span` interface in the `go.opentelemetry.io/otel/trace` package.
   Using the same tracer that created a span introduces the error where an instrumentation library's `Tracer` is used by other code instead of their own.
   The `"go.opentelemetry.io/otel".Tracer` function or a `TracerProvider` should be used to acquire a library specific `Tracer` instead. (#1900)
+  - The `TracerProvider()` method on the `Span` interface may also be used to obtain a `TracerProvider` using the same trace processing pipeline. (#2009)
 - The `http.url` attribute generated by `HTTPClientAttributesFromHTTPRequest` will no longer include username or password information. (#1919)
 - The `IsEmpty` method of the `TraceState` type in the `go.opentelemetry.io/otel/trace` package is removed in favor of using the added `TraceState.Len` method. (#1931)
 - The `Set`, `Value`, `ContextWithValue`, `ContextWithoutValue`, and `ContextWithEmpty` functions in the `go.opentelemetry.io/otel/baggage` package are removed.

diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go
@@ -290,3 +290,5 @@ func (s *MockSpan) AddEvent(name string, o ...trace.EventOption) {
 func (s *MockSpan) OverrideTracer(tracer trace.Tracer) {
 	s.officialTracer = tracer
 }
+
+func (s *MockSpan) TracerProvider() trace.TracerProvider { return trace.NewNoopTracerProvider() }
diff --git a/internal/global/trace.go b/internal/global/trace.go
@@ -103,7 +103,7 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
 		return val
 	}
 
-	t := &tracer{name: name, opts: opts}
+	t := &tracer{name: name, opts: opts, provider: p}
 	p.tracers[key] = t
 	return t
 }
@@ -118,8 +118,9 @@ type il struct {
 // All Tracer functionality is forwarded to a delegate once configured.
 // Otherwise, all functionality is forwarded to a NoopTracer.
 type tracer struct {
-	name string
-	opts []trace.TracerOption
+	name     string
+	opts     []trace.TracerOption
+	provider *tracerProvider
 
 	delegate atomic.Value
 }
@@ -145,7 +146,7 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanStart
 		return delegate.(trace.Tracer).Start(ctx, name, opts...)
 	}
 
-	s := nonRecordingSpan{sc: trace.SpanContextFromContext(ctx)}
+	s := nonRecordingSpan{sc: trace.SpanContextFromContext(ctx), tracer: t}
 	ctx = trace.ContextWithSpan(ctx, s)
 	return ctx, s
 }
@@ -154,7 +155,8 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanStart
 // SpanContext. It performs no operations other than to return the wrapped
 // SpanContext.
 type nonRecordingSpan struct {
-	sc trace.SpanContext
+	sc     trace.SpanContext
+	tracer *tracer
 }
 
 var _ trace.Span = nonRecordingSpan{}
@@ -185,3 +187,5 @@ func (nonRecordingSpan) AddEvent(string, ...trace.EventOption) {}
 
 // SetName does nothing.
 func (nonRecordingSpan) SetName(string) {}
+
+func (s nonRecordingSpan) TracerProvider() trace.TracerProvider { return s.tracer.provider }
diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go
@@ -53,14 +53,18 @@ func TestTraceWithSDK(t *testing.T) {
 	_, span3 := tracer2.Start(ctx, "span3")
 	span3.End()
 
+	// The noop-span should still provide access to a usable TracerProvider.
+	_, span4 := span1.TracerProvider().Tracer("fromSpan").Start(ctx, "span4")
+	span4.End()
+
 	filterNames := func(spans []*oteltest.Span) []string {
 		names := make([]string, len(spans))
 		for i := range spans {
 			names[i] = spans[i].Name()
 		}
 		return names
 	}
-	expected := []string{"span2", "span3"}
+	expected := []string{"span2", "span3", "span4"}
 	assert.ElementsMatch(t, expected, filterNames(sr.Started()))
 	assert.ElementsMatch(t, expected, filterNames(sr.Completed()))
 }

diff --git a/oteltest/harness.go b/oteltest/harness.go
@@ -287,6 +287,12 @@ func (h *Harness) testSpan(tracerFactory func() trace.Tracer) {
 
 			return subject
 		},
+		"Span created via span.TracerProvider()": func() trace.Span {
+			ctx, spanA := tracerFactory().Start(context.Background(), "span1")
+
+			_, spanB := spanA.TracerProvider().Tracer("second").Start(ctx, "span2")
+			return spanB
+		},
 	}
 
 	for mechanismName, mechanism := range mechanisms {

diff --git a/oteltest/provider.go b/oteltest/provider.go
@@ -58,9 +58,9 @@ func (p *TracerProvider) Tracer(instName string, opts ...trace.TracerOption) tra
 	t, ok := p.tracers[inst]
 	if !ok {
 		t = &Tracer{
-			Name:    instName,
-			Version: conf.InstrumentationVersion(),
-			config:  &p.config,
+			Name:     instName,
+			Version:  conf.InstrumentationVersion(),
+			provider: p,
 		}
 		p.tracers[inst] = t
 	}

diff --git a/oteltest/span.go b/oteltest/span.go
@@ -64,8 +64,8 @@ func (s *Span) End(opts ...trace.SpanEndOption) {
 	}
 
 	s.ended = true
-	if s.tracer.config.SpanRecorder != nil {
-		s.tracer.config.SpanRecorder.OnEnd(s)
+	if s.tracer.provider.config.SpanRecorder != nil {
+		s.tracer.provider.config.SpanRecorder.OnEnd(s)
 	}
 }
 
@@ -221,3 +221,9 @@ func (s *Span) StatusMessage() string { return s.statusMessage }
 
 // SpanKind returns the span kind of s.
 func (s *Span) SpanKind() trace.SpanKind { return s.spanKind }
+
+// TracerProvider returns a trace.TracerProvider that can be used to generate
+// additional Spans on the same telemetry pipeline as the current Span.
+func (s *Span) TracerProvider() trace.TracerProvider {
+	return s.tracer.provider
+}
diff --git a/oteltest/span_test.go b/oteltest/span_test.go
@@ -569,4 +569,24 @@ func TestSpan(t *testing.T) {
 			e.Expect(subject.SpanKind()).ToEqual(trace.SpanKindConsumer)
 		})
 	})
+
+	t.Run("can provide a valid TracerProvider", func(t *testing.T) {
+		t.Parallel()
+
+		e := matchers.NewExpecter(t)
+
+		sr := new(oteltest.SpanRecorder)
+		tracerA := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr)).Tracer(t.Name())
+		ctx, spanA := tracerA.Start(context.Background(), "span1")
+
+		e.Expect(len(sr.Started())).ToEqual(1)
+
+		_, spanB := spanA.TracerProvider().Tracer("extracted").Start(ctx, "span2")
+
+		spans := sr.Started()
+
+		e.Expect(len(spans)).ToEqual(2)
+		e.Expect(spans[0]).ToEqual(spanA)
+		e.Expect(spans[1]).ToEqual(spanB)
+	})
 }
diff --git a/oteltest/tracer.go b/oteltest/tracer.go
@@ -31,7 +31,7 @@ type Tracer struct {
 	// Version is the instrumentation version.
 	Version string
 
-	config *config
+	provider *TracerProvider
 }
 
 // Start creates a span. If t is configured with a SpanRecorder its OnStart
@@ -54,7 +54,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanStart
 	if c.NewRoot() {
 		span.spanContext = trace.SpanContext{}
 	} else {
-		span.spanContext = t.config.SpanContextFunc(ctx)
+		span.spanContext = t.provider.config.SpanContextFunc(ctx)
 		if current := trace.SpanContextFromContext(ctx); current.IsValid() {
 			span.spanContext = span.spanContext.WithTraceID(current.TraceID())
 			span.parentSpanID = current.SpanID()
@@ -77,8 +77,8 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanStart
 	span.SetName(name)
 	span.SetAttributes(c.Attributes()...)
 
-	if t.config.SpanRecorder != nil {
-		t.config.SpanRecorder.OnStart(span)
+	if t.provider.config.SpanRecorder != nil {
+		t.provider.config.SpanRecorder.OnStart(span)
 	}
 	return trace.ContextWithSpan(ctx, span), span
 }
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
@@ -470,6 +470,12 @@ func (s *span) ChildSpanCount() int {
 	return s.childSpanCount
 }
 
+// TracerProvider returns a trace.TracerProvider that can be used to generate
+// additional Spans on the same telemetry pipeline as the current Span.
+func (s *span) TracerProvider() trace.TracerProvider {
+	return s.tracer.provider
+}
+
 // snapshot creates a read-only copy of the current state of the span.
 func (s *span) snapshot() ReadOnlySpan {
 	var sd snapshot

diff --git a/trace/noop.go b/trace/noop.go
@@ -42,9 +42,14 @@ type noopTracer struct{}
 
 var _ Tracer = noopTracer{}
 
-// Start starts a noop span.
+// Start carries forward a non-recording Span, if one is present in the context, otherwise it
+// creates a no-op Span.
 func (t noopTracer) Start(ctx context.Context, name string, _ ...SpanStartOption) (context.Context, Span) {
-	span := noopSpan{}
+	span := SpanFromContext(ctx)
+	if _, ok := span.(nonRecordingSpan); !ok {
+		// span is likely already a noopSpan, but let's be sure
+		span = noopSpan{}
+	}
 	return ContextWithSpan(ctx, span), span
 }
 
@@ -79,3 +84,6 @@ func (noopSpan) AddEvent(string, ...EventOption) {}
 
 // SetName does nothing.
 func (noopSpan) SetName(string) {}
+
+// TracerProvider returns a no-op TracerProvider
+func (noopSpan) TracerProvider() TracerProvider { return noopTracerProvider{} }
diff --git a/trace/noop_test.go b/trace/noop_test.go
@@ -70,3 +70,22 @@ func TestNoopSpan(t *testing.T) {
 		t.Errorf("span.IsRecording() returned %#v, want %#v", got, want)
 	}
 }
+
+func TestNonRecordingSpanTracerStart(t *testing.T) {
+	tid, err := TraceIDFromHex("01000000000000000000000000000000")
+	if err != nil {
+		t.Fatalf("failure creating TraceID: %s", err.Error())
+	}
+	sid, err := SpanIDFromHex("0200000000000000")
+	if err != nil {
+		t.Fatalf("failure creating SpanID: %s", err.Error())
+	}
+	sc := NewSpanContext(SpanContextConfig{TraceID: tid, SpanID: sid})
+
+	ctx := ContextWithSpanContext(context.Background(), sc)
+	_, span := NewNoopTracerProvider().Tracer("test instrumentation").Start(ctx, "span1")
+
+	if got, want := span.SpanContext(), sc; !assertSpanContextEqual(got, want) {
+		t.Errorf("SpanContext not carried by nonRecordingSpan. got %#v, want %#v", got, want)
+	}
+}
diff --git a/trace/trace.go b/trace/trace.go
@@ -373,6 +373,10 @@ type Span interface {
 	// already exists for an attribute of the Span it will be overwritten with
 	// the value contained in kv.
 	SetAttributes(kv ...attribute.KeyValue)
+
+	// TracerProvider returns a TracerProvider that can be used to generate
+	// additional Spans on the same telemetry pipeline as the current Span.
+	TracerProvider() TracerProvider
 }
 
 // Link is the relationship between two Spans. The relationship can be within