From b6d5442ff66268e1149909f60a1bf69da103a2b7 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 12 May 2021 15:19:50 +0000 Subject: [PATCH] Remove the Tracer method from the Span API (#1900) * Remove the Tracer method from the Span API * Update CHANGELOG with changes * Update CHANGELOG.md * Fix misspell * Address feedback --- CHANGELOG.md | 3 ++ bridge/opentracing/mix_test.go | 74 ---------------------------------- oteltest/harness.go | 1 - oteltest/span.go | 5 --- oteltest/span_test.go | 14 ------- sdk/trace/span.go | 5 --- trace/noop.go | 3 -- trace/noop_test.go | 4 -- trace/trace.go | 4 -- 9 files changed, 3 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1329e8cff..fa47f4852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,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 diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index 7bd65ce03..8dc9445b6 100644 --- a/bridge/opentracing/mix_test.go +++ b/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 { diff --git a/oteltest/harness.go b/oteltest/harness.go index 6f3190c15..88af8c232 100644 --- a/oteltest/harness.go +++ b/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() }) diff --git a/oteltest/span.go b/oteltest/span.go index 0f2436c2b..2d7edd15c 100644 --- a/oteltest/span.go +++ b/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. diff --git a/oteltest/span_test.go b/oteltest/span_test.go index 11372df09..c997539a8 100644 --- a/oteltest/span_test.go +++ b/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) { diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 374975d5f..ea61e9c6b 100644 --- a/sdk/trace/span.go +++ b/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) { diff --git a/trace/noop.go b/trace/noop.go index 4a20f20cb..d3691d1c6 100644 --- a/trace/noop.go +++ b/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) {} diff --git a/trace/noop_test.go b/trace/noop_test.go index 64a4d95ae..897dcf469 100644 --- a/trace/noop_test.go +++ b/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) - } } diff --git a/trace/trace.go b/trace/trace.go index 3ddbc2e78..41a2e2797 100644 --- a/trace/trace.go +++ b/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