Implement global default non-recording span

CHANGELOG.md

@@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `ExportSpans` method of the`SpanExporter` interface type was updated to accept `ReadOnlySpan`s instead of the removed `SpanSnapshot`.
   This brings the export interface into compliance with the specification in that it now accepts an explicitly immutable type instead of just an implied one. (#1873)
 - Unembed `SpanContext` in `Link`. (#1877)
+- Spans created by the global `Tracer` obtained from `go.opentelemetry.io/otel`, prior to a functioning `TracerProvider` being set, now propagate the span context from their parent if one exists. (#1901)
 
 ### Deprecated
 
@@ -55,6 +56,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Removed the `SpanSnapshot` type from the `go.opentelemetry.io/otel/sdk/trace` package.
   The use of this type has been replaced with the use of the explicitly immutable `ReadOnlySpan` type.
   When a concrete representation of a read-only span is needed for testing, the newly added `SpanStub` in the `go.opentelemetry.io/otel/sdk/trace/tracetest` package should be used. (#1873)
+- 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)
 
 ### Fixed
 

bridge/opentracing/mix_test.go

@@ -42,7 +42,6 @@ func getMixedAPIsTestCases() []mixedAPIsTestCase {
 	cast := newCurrentActiveSpanTest()
 	coin := newContextIntactTest()
 	bip := newBaggageItemsPreservationTest()
-	tm := newTracerMessTest()
 	bio := newBaggageInteroperationTest()
 
 	return []mixedAPIsTestCase{
@@ -94,18 +93,6 @@ func getMixedAPIsTestCases() []mixedAPIsTestCase {
 			run:   bip.runOTOtelOT,
 			check: bip.check,
 		},
-		{
-			desc:  "consistent tracers otel -> ot -> otel",
-			setup: tm.setup,
-			run:   tm.runOtelOTOtel,
-			check: tm.check,
-		},
-		{
-			desc:  "consistent tracers ot -> otel -> ot",
-			setup: tm.setup,
-			run:   tm.runOTOtelOT,
-			check: tm.check,
-		},
 		{
 			desc:  "baggage items interoperation across layers ot -> otel -> ot",
 			setup: bio.setup,
@@ -419,67 +406,6 @@ func (bip *baggageItemsPreservationTest) addAndRecordBaggage(t *testing.T, ctx c
 	return ctx
 }
 
-// tracer mess test
-
-type tracerMessTest struct {
-	recordedOTSpanTracers   []ot.Tracer
-	recordedOtelSpanTracers []trace.Tracer
-}
-
-func newTracerMessTest() *tracerMessTest {
-	return &tracerMessTest{
-		recordedOTSpanTracers:   nil,
-		recordedOtelSpanTracers: nil,
-	}
-}
-
-func (tm *tracerMessTest) setup(t *testing.T, tracer *internal.MockTracer) {
-	tm.recordedOTSpanTracers = nil
-	tm.recordedOtelSpanTracers = nil
-}
-
-func (tm *tracerMessTest) check(t *testing.T, tracer *internal.MockTracer) {
-	globalOtTracer := ot.GlobalTracer()
-	globalOtelTracer := otel.Tracer("")
-	if len(tm.recordedOTSpanTracers) != 3 {
-		t.Errorf("Expected 3 recorded OpenTracing tracers from spans, got %d", len(tm.recordedOTSpanTracers))
-	}
-	if len(tm.recordedOtelSpanTracers) != 3 {
-		t.Errorf("Expected 3 recorded OpenTelemetry tracers from spans, got %d", len(tm.recordedOtelSpanTracers))
-	}
-	for idx, tracer := range tm.recordedOTSpanTracers {
-		if tracer != globalOtTracer {
-			t.Errorf("Expected OpenTracing tracer %d to be the same as global tracer (%#v), but got %#v", idx, globalOtTracer, tracer)
-		}
-	}
-	for idx, tracer := range tm.recordedOtelSpanTracers {
-		if tracer != globalOtelTracer {
-			t.Errorf("Expected OpenTelemetry tracer %d to be the same as global tracer (%#v), but got %#v", idx, globalOtelTracer, tracer)
-		}
-	}
-}
-
-func (tm *tracerMessTest) runOtelOTOtel(t *testing.T, ctx context.Context) {
-	runOtelOTOtel(t, ctx, "tm", tm.recordTracers)
-}
-
-func (tm *tracerMessTest) runOTOtelOT(t *testing.T, ctx context.Context) {
-	runOTOtelOT(t, ctx, "tm", tm.recordTracers)
-}
-
-func (tm *tracerMessTest) recordTracers(t *testing.T, ctx context.Context) context.Context {
-	otSpan := ot.SpanFromContext(ctx)
-	if otSpan == nil {
-		t.Errorf("No current OpenTracing span?")
-	} else {
-		tm.recordedOTSpanTracers = append(tm.recordedOTSpanTracers, otSpan.Tracer())
-	}
-
-	otelSpan := trace.SpanFromContext(ctx)
-	tm.recordedOtelSpanTracers = append(tm.recordedOtelSpanTracers, otelSpan.Tracer())
-	return ctx
-}
-
 // baggage interoperation test
 
 type baggageInteroperationTest struct {

internal/global/trace.go

@@ -36,7 +36,8 @@ import (
 	"sync"
 	"sync/atomic"
 
-	"go.opentelemetry.io/otel/internal/trace/noop"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -143,5 +144,44 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio
 	if delegate != nil {
 		return delegate.(trace.Tracer).Start(ctx, name, opts...)
 	}
-	return noop.Tracer.Start(ctx, name, opts...)
+
+	s := nonRecordingSpan{sc: trace.SpanContextFromContext(ctx)}
+	ctx = trace.ContextWithSpan(ctx, s)
+	return ctx, s
+}
+
+// nonRecordingSpan is a minimal implementation of a Span that wraps a
+// SpanContext. It performs no operations other than to return the wrapped
+// SpanContext.
+type nonRecordingSpan struct {
+	sc trace.SpanContext
 }
+
+var _ trace.Span = nonRecordingSpan{}
+
+// SpanContext returns the wrapped SpanContext.
+func (s nonRecordingSpan) SpanContext() trace.SpanContext { return s.sc }
+
+// IsRecording always returns false.
+func (nonRecordingSpan) IsRecording() bool { return false }
+
+// SetStatus does nothing.
+func (nonRecordingSpan) SetStatus(codes.Code, string) {}
+
+// SetError does nothing.
+func (nonRecordingSpan) SetError(bool) {}
+
+// SetAttributes does nothing.
+func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {}
+
+// End does nothing.
+func (nonRecordingSpan) End(...trace.SpanOption) {}
+
+// RecordError does nothing.
+func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {}
+
+// AddEvent does nothing.
+func (nonRecordingSpan) AddEvent(string, ...trace.EventOption) {}
+
+// SetName does nothing.
+func (nonRecordingSpan) SetName(string) {}

internal/global/trace_test.go

@@ -213,3 +213,19 @@ func TestTraceProviderDelegatesSameInstance(t *testing.T) {
 
 	assert.NotSame(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
 }
+
+func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) {
+	global.ResetForTest()
+
+	sc := trace.NewSpanContext(trace.SpanContextConfig{
+		TraceID:    [16]byte{0x01},
+		SpanID:     [8]byte{0x01},
+		TraceFlags: trace.FlagsSampled,
+		Remote:     true,
+	})
+	ctx := trace.ContextWithSpanContext(context.Background(), sc)
+	_, span := otel.Tracer("test").Start(ctx, "test")
+
+	assert.Equal(t, sc, span.SpanContext())
+	assert.False(t, span.IsRecording())
+}

oteltest/harness.go

@@ -119,7 +119,6 @@ func (h *Harness) TestTracer(subjectFactory func() trace.Tracer) {
 
 			e.Expect(span).NotToBeNil()
 
-			e.Expect(span.Tracer()).ToEqual(subject)
 			e.Expect(span.SpanContext().IsValid()).ToBeTrue()
 		})
 

oteltest/span.go

@@ -46,11 +46,6 @@ type Span struct {
 	spanKind      trace.SpanKind
 }
 
-// Tracer returns the Tracer that created s.
-func (s *Span) Tracer() trace.Tracer {
-	return s.tracer
-}
-
 // End ends s. If the Tracer that created s was configured with a
 // SpanRecorder, that recorder's OnEnd method is called as the final part of
 // this method.

oteltest/span_test.go

@@ -32,20 +32,6 @@ import (
 )
 
 func TestSpan(t *testing.T) {
-	t.Run("#Tracer", func(t *testing.T) {
-		tp := oteltest.NewTracerProvider()
-		t.Run("returns the tracer used to start the span", func(t *testing.T) {
-			t.Parallel()
-
-			e := matchers.NewExpecter(t)
-
-			tracer := tp.Tracer(t.Name())
-			_, subject := tracer.Start(context.Background(), "test")
-
-			e.Expect(subject.Tracer()).ToEqual(tracer)
-		})
-	})
-
 	t.Run("#End", func(t *testing.T) {
 		tp := oteltest.NewTracerProvider()
 		t.Run("ends the span", func(t *testing.T) {

sdk/trace/span.go

@@ -292,11 +292,6 @@ func typeStr(i interface{}) string {
 	return fmt.Sprintf("%s.%s", t.PkgPath(), t.Name())
 }
 
-// Tracer returns the Tracer that created this span.
-func (s *span) Tracer() trace.Tracer {
-	return s.tracer
-}
-
 // AddEvent adds an event with the provided name and options. If this span is
 // not being recorded than this method does nothing.
 func (s *span) AddEvent(name string, o ...trace.EventOption) {

trace/noop.go

@@ -74,9 +74,6 @@ func (noopSpan) End(...SpanOption) {}
 // RecordError does nothing.
 func (noopSpan) RecordError(error, ...EventOption) {}
 
-// Tracer returns the Tracer that created this Span.
-func (noopSpan) Tracer() Tracer { return noopTracer{} }
-
 // AddEvent does nothing.
 func (noopSpan) AddEvent(string, ...EventOption) {}
 

trace/noop_test.go

@@ -69,8 +69,4 @@ func TestNoopSpan(t *testing.T) {
 	if got, want := span.IsRecording(), false; got != want {
 		t.Errorf("span.IsRecording() returned %#v, want %#v", got, want)
 	}
-
-	if got, want := span.Tracer(), tracer; got != want {
-		t.Errorf("span.Tracer() returned %#v, want %#v", got, want)
-	}
 }

trace/trace.go

@@ -497,10 +497,6 @@ func (sc SpanContext) MarshalJSON() ([]byte, error) {
 // create a Span and it is then up to the operation the Span represents to
 // properly end the Span when the operation itself ends.
 type Span interface {
-	// Tracer returns the Tracer that created the Span. Tracer MUST NOT be
-	// nil.
-	Tracer() Tracer
-
 	// End completes the Span. The Span is considered complete and ready to be
 	// delivered through the rest of the telemetry pipeline after this method
 	// is called. Therefore, updates to the Span are not allowed after this