Skip to content

Commit

Permalink
Remove the Tracer method from the Span API (#1900)
Browse files Browse the repository at this point in the history
* Remove the Tracer method from the Span API

* Update CHANGELOG with changes

* Update CHANGELOG.md

* Fix misspell

* Address feedback
  • Loading branch information
MrAlias authored May 12, 2021
1 parent ae85fab commit b6d5442
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 110 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
74 changes: 0 additions & 74 deletions bridge/opentracing/mix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func getMixedAPIsTestCases() []mixedAPIsTestCase {
cast := newCurrentActiveSpanTest()
coin := newContextIntactTest()
bip := newBaggageItemsPreservationTest()
tm := newTracerMessTest()
bio := newBaggageInteroperationTest()

return []mixedAPIsTestCase{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion oteltest/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})

Expand Down
5 changes: 0 additions & 5 deletions oteltest/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 0 additions & 14 deletions oteltest/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 0 additions & 5 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions trace/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
4 changes: 0 additions & 4 deletions trace/noop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
4 changes: 0 additions & 4 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b6d5442

Please sign in to comment.