Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the Tracer method from the Span API #1900

Merged
merged 7 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 should be used to aquire a library specific `Tracer` instead. (#1900)
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

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