Skip to content

Commit

Permalink
Add embedded package to trace API (#4620)
Browse files Browse the repository at this point in the history
* Add trace/embedded

* Update trace impl to use trace/embedded

* Add noop pkg to replace no-op impl in trace pkg

* Use trace/embedded in global impl

* Use trace/embedded in SDK impl

* Update opencensus bridge

* Update opentracing bridge

* Add changes to changelog

* Update trace/doc.go

Co-authored-by: David Ashpole <dashpole@google.com>

---------

Co-authored-by: David Ashpole <dashpole@google.com>
  • Loading branch information
MrAlias and dashpole committed Oct 19, 2023
1 parent da343ab commit 1e1cc90
Show file tree
Hide file tree
Showing 21 changed files with 480 additions and 35 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,32 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Add `go.opentelemetry.io/otel/bridge/opencensus.InstallTraceBridge`, which installs the OpenCensus trace bridge, and replaces `opencensus.NewTracer`. (#4567)
- Add scope version to trace and metric bridges in `go.opentelemetry.io/otel/bridge/opencensus`. (#4584)
- Add the `go.opentelemetry.io/otel/trace/embedded` package to be embedded in the exported trace API interfaces. (#4620)
- Add the `go.opentelemetry.io/otel/trace/noop` package as a default no-op implementation of the trace API. (#4620)
- Add context propagation in `go.opentelemetry.io/otel/example/dice`. (#4644)

### Deprecated

- Deprecate `go.opentelemetry.io/otel/bridge/opencensus.NewTracer` in favor of `opencensus.InstallTraceBridge`. (#4567)
- Deprecate `go.opentelemetry.io/otel/example/fib` package is in favor of `go.opentelemetry.io/otel/example/dice`. (#4618)
- Deprecate `go.opentelemetry.io/otel/trace.NewNoopTracerProvider`.
Use the added `NewTracerProvider` function in `go.opentelemetry.io/otel/trace/noop` instead. (#4620)

### Changed

- `go.opentelemetry.io/otel/bridge/opencensus.NewMetricProducer` returns a `*MetricProducer` struct instead of the metric.Producer interface. (#4583)
- The `TracerProvider` in `go.opentelemetry.io/otel/trace` now embeds the `go.opentelemetry.io/otel/trace/embedded.TracerProvider` type.
This extends the `TracerProvider` interface and is is a breaking change for any existing implementation.
Implementors need to update their implementations based on what they want the default behavior of the interface to be.
See the "API Implementations" section of the `go.opentelemetry.io/otel/trace` package documentation for more informatoin about how to accomplish this. (#4620)
- The `Tracer` in `go.opentelemetry.io/otel/trace` now embeds the `go.opentelemetry.io/otel/trace/embedded.Tracer` type.
This extends the `Tracer` interface and is is a breaking change for any existing implementation.
Implementors need to update their implementations based on what they want the default behavior of the interface to be.
See the "API Implementations" section of the `go.opentelemetry.io/otel/trace` package documentation for more informatoin about how to accomplish this. (#4620)
- The `Span` in `go.opentelemetry.io/otel/trace` now embeds the `go.opentelemetry.io/otel/trace/embedded.Span` type.
This extends the `Span` interface and is is a breaking change for any existing implementation.
Implementors need to update their implementations based on what they want the default behavior of the interface to be.
See the "API Implementations" section of the `go.opentelemetry.io/otel/trace` package documentation for more informatoin about how to accomplish this. (#4620)

## [1.19.0/0.42.0/0.0.7] 2023-09-28

Expand Down
6 changes: 3 additions & 3 deletions bridge/opencensus/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/noop"
)

func TestNewTraceConfig(t *testing.T) {
globalTP := trace.NewNoopTracerProvider()
customTP := trace.NewNoopTracerProvider()
globalTP := noop.NewTracerProvider()
customTP := noop.NewTracerProvider()
otel.SetTracerProvider(globalTP)
for _, tc := range []struct {
desc string
Expand Down
20 changes: 12 additions & 8 deletions bridge/opencensus/internal/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel"
"go.opentelemetry.io/otel/bridge/opencensus/internal/otel2oc"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
"go.opentelemetry.io/otel/trace/noop"
)

type handler struct{ err error }
Expand All @@ -38,15 +40,17 @@ func withHandler() (*handler, func()) {
}

type tracer struct {
embedded.Tracer

ctx context.Context
name string
opts []trace.SpanStartOption
}

func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
t.ctx, t.name, t.opts = ctx, name, opts
noop := trace.NewNoopTracerProvider().Tracer("testing")
return noop.Start(ctx, name, opts...)
sub := noop.NewTracerProvider().Tracer("testing")
return sub.Start(ctx, name, opts...)
}

type ctxKey string
Expand Down Expand Up @@ -110,11 +114,11 @@ func TestTracerFromContext(t *testing.T) {
})
ctx := trace.ContextWithSpanContext(context.Background(), sc)

noop := trace.NewNoopTracerProvider().Tracer("TestTracerFromContext")
tracer := noop.NewTracerProvider().Tracer("TestTracerFromContext")
// Test using the fact that the No-Op span will propagate a span context .
ctx, _ = noop.Start(ctx, "test")
ctx, _ = tracer.Start(ctx, "test")

got := internal.NewTracer(noop).FromContext(ctx).SpanContext()
got := internal.NewTracer(tracer).FromContext(ctx).SpanContext()
// Do not test the convedsion, only that the propagtion.
want := otel2oc.SpanContext(sc)
if got != want {
Expand All @@ -129,11 +133,11 @@ func TestTracerNewContext(t *testing.T) {
})
ctx := trace.ContextWithSpanContext(context.Background(), sc)

noop := trace.NewNoopTracerProvider().Tracer("TestTracerNewContext")
tracer := noop.NewTracerProvider().Tracer("TestTracerNewContext")
// Test using the fact that the No-Op span will propagate a span context .
_, s := noop.Start(ctx, "test")
_, s := tracer.Start(ctx, "test")

ocTracer := internal.NewTracer(noop)
ocTracer := internal.NewTracer(tracer)
ctx = ocTracer.NewContext(context.Background(), internal.NewSpan(s))
got := trace.SpanContextFromContext(ctx)

Expand Down
3 changes: 2 additions & 1 deletion bridge/opentracing/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ import (
iBaggage "go.opentelemetry.io/otel/internal/baggage"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/noop"
)

var (
noopTracer = trace.NewNoopTracerProvider().Tracer("")
noopTracer = noop.NewTracerProvider().Tracer("")
noopSpan = func() trace.Span {
_, s := noopTracer.Start(context.Background(), "")
return s
Expand Down
8 changes: 7 additions & 1 deletion bridge/opentracing/internal/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"go.opentelemetry.io/otel/codes"
semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
"go.opentelemetry.io/otel/trace/noop"
)

//nolint:revive // ignoring missing comments for unexported global variables in an internal package.
Expand All @@ -44,6 +46,8 @@ type MockContextKeyValue struct {
}

type MockTracer struct {
embedded.Tracer

FinishedSpans []*MockSpan
SpareTraceIDs []trace.TraceID
SpareSpanIDs []trace.SpanID
Expand Down Expand Up @@ -184,6 +188,8 @@ type MockEvent struct {
}

type MockSpan struct {
embedded.Span

mockTracer *MockTracer
officialTracer trace.Tracer
spanContext trace.SpanContext
Expand Down Expand Up @@ -295,4 +301,4 @@ func (s *MockSpan) OverrideTracer(tracer trace.Tracer) {
s.officialTracer = tracer
}

func (s *MockSpan) TracerProvider() trace.TracerProvider { return trace.NewNoopTracerProvider() }
func (s *MockSpan) TracerProvider() trace.TracerProvider { return noop.NewTracerProvider() }
3 changes: 3 additions & 0 deletions bridge/opentracing/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ import (
"sync"

"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
)

// TracerProvider is an OpenTelemetry TracerProvider that wraps an OpenTracing
// Tracer.
type TracerProvider struct {
embedded.TracerProvider

bridge *BridgeTracer
provider trace.TracerProvider

Expand Down
3 changes: 2 additions & 1 deletion bridge/opentracing/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import (

"go.opentelemetry.io/otel/bridge/opentracing/internal"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
)

type namedMockTracer struct {
name string
*internal.MockTracer
}

type namedMockTracerProvider struct{}
type namedMockTracerProvider struct{ embedded.TracerProvider }

var _ trace.TracerProvider = (*namedMockTracerProvider)(nil)

Expand Down
5 changes: 5 additions & 0 deletions bridge/opentracing/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ import (

"go.opentelemetry.io/otel/bridge/opentracing/migration"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
)

// WrapperTracerProvider is an OpenTelemetry TracerProvider that wraps an
// OpenTracing Tracer, created by the deprecated NewWrappedTracerProvider.
//
// Deprecated: Use the TracerProvider from NewTracerProvider(...) instead.
type WrapperTracerProvider struct {
embedded.TracerProvider

wTracer *WrapperTracer
}

Expand Down Expand Up @@ -56,6 +59,8 @@ func NewWrappedTracerProvider(bridge *BridgeTracer, tracer trace.Tracer) *Wrappe
// aware how to operate in environment where OpenTracing API is also
// used.
type WrapperTracer struct {
embedded.Tracer

bridge *BridgeTracer
tracer trace.Tracer
}
Expand Down
11 changes: 6 additions & 5 deletions internal/global/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/noop"
metricnoop "go.opentelemetry.io/otel/metric/noop"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
tracenoop "go.opentelemetry.io/otel/trace/noop"
)

type nonComparableTracerProvider struct {
Expand Down Expand Up @@ -55,7 +56,7 @@ func TestSetTracerProvider(t *testing.T) {
t.Run("First Set() should replace the delegate", func(t *testing.T) {
ResetForTest(t)

SetTracerProvider(trace.NewNoopTracerProvider())
SetTracerProvider(tracenoop.NewTracerProvider())

_, ok := TracerProvider().(*tracerProvider)
if ok {
Expand All @@ -67,7 +68,7 @@ func TestSetTracerProvider(t *testing.T) {
ResetForTest(t)

tp := TracerProvider()
SetTracerProvider(trace.NewNoopTracerProvider())
SetTracerProvider(tracenoop.NewTracerProvider())

ntp := tp.(*tracerProvider)

Expand Down Expand Up @@ -153,7 +154,7 @@ func TestSetMeterProvider(t *testing.T) {
t.Run("First Set() should replace the delegate", func(t *testing.T) {
ResetForTest(t)

SetMeterProvider(noop.NewMeterProvider())
SetMeterProvider(metricnoop.NewMeterProvider())

_, ok := MeterProvider().(*meterProvider)
if ok {
Expand All @@ -166,7 +167,7 @@ func TestSetMeterProvider(t *testing.T) {

mp := MeterProvider()

SetMeterProvider(noop.NewMeterProvider())
SetMeterProvider(metricnoop.NewMeterProvider())

dmp := mp.(*meterProvider)

Expand Down
7 changes: 7 additions & 0 deletions internal/global/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,16 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
)

// tracerProvider is a placeholder for a configured SDK TracerProvider.
//
// All TracerProvider functionality is forwarded to a delegate once
// configured.
type tracerProvider struct {
embedded.TracerProvider

mtx sync.Mutex
tracers map[il]*tracer
delegate trace.TracerProvider
Expand Down Expand Up @@ -119,6 +122,8 @@ type il struct {
// All Tracer functionality is forwarded to a delegate once configured.
// Otherwise, all functionality is forwarded to a NoopTracer.
type tracer struct {
embedded.Tracer

name string
opts []trace.TracerOption
provider *tracerProvider
Expand Down Expand Up @@ -156,6 +161,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 {
embedded.Span

sc trace.SpanContext
tracer *tracer
}
Expand Down
16 changes: 11 additions & 5 deletions internal/global/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
"go.opentelemetry.io/otel/trace/noop"
)

type fnTracerProvider struct {
embedded.TracerProvider

tracer func(string, ...trace.TracerOption) trace.Tracer
}

Expand All @@ -34,6 +38,8 @@ func (fn fnTracerProvider) Tracer(instrumentationName string, opts ...trace.Trac
}

type fnTracer struct {
embedded.Tracer

start func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span)
}

Expand Down Expand Up @@ -72,7 +78,7 @@ func TestTraceProviderDelegation(t *testing.T) {
assert.Equal(t, want, spanName)
}
}
return trace.NewNoopTracerProvider().Tracer(name).Start(ctx, spanName)
return noop.NewTracerProvider().Tracer(name).Start(ctx, spanName)
},
}
},
Expand Down Expand Up @@ -107,7 +113,7 @@ func TestTraceProviderDelegates(t *testing.T) {
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
called = true
assert.Equal(t, "abc", name)
return trace.NewNoopTracerProvider().Tracer("")
return noop.NewTracerProvider().Tracer("")
},
})

Expand Down Expand Up @@ -148,7 +154,7 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) {
// Signal the goroutine to finish.
close(quit)
}
return trace.NewNoopTracerProvider().Tracer("")
return noop.NewTracerProvider().Tracer("")
},
})

Expand Down Expand Up @@ -195,7 +201,7 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) {
// Signal the goroutine to finish.
close(quit)
}
return trace.NewNoopTracerProvider().Tracer("").Start(ctx, spanName)
return noop.NewTracerProvider().Tracer("").Start(ctx, spanName)
},
}
},
Expand All @@ -218,7 +224,7 @@ func TestTraceProviderDelegatesSameInstance(t *testing.T) {

SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
return trace.NewNoopTracerProvider().Tracer("")
return noop.NewTracerProvider().Tracer("")
},
})

Expand Down
8 changes: 6 additions & 2 deletions sdk/trace/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/resource"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
"go.opentelemetry.io/otel/trace/noop"
)

const (
Expand Down Expand Up @@ -73,6 +75,8 @@ func (cfg tracerProviderConfig) MarshalLog() interface{} {
// TracerProvider is an OpenTelemetry TracerProvider. It provides Tracers to
// instrumentation so it can trace operational flow through a system.
type TracerProvider struct {
embedded.TracerProvider

mu sync.Mutex
namedTracer map[instrumentation.Scope]*tracer
spanProcessors atomic.Pointer[spanProcessorStates]
Expand Down Expand Up @@ -139,7 +143,7 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider {
func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer {
// This check happens before the mutex is acquired to avoid deadlocking if Tracer() is called from within Shutdown().
if p.isShutdown.Load() {
return trace.NewNoopTracerProvider().Tracer(name, opts...)
return noop.NewTracerProvider().Tracer(name, opts...)
}
c := trace.NewTracerConfig(opts...)
if name == "" {
Expand All @@ -157,7 +161,7 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
// Must check the flag after acquiring the mutex to avoid returning a valid tracer if Shutdown() ran
// after the first check above but before we acquired the mutex.
if p.isShutdown.Load() {
return trace.NewNoopTracerProvider().Tracer(name, opts...), true
return noop.NewTracerProvider().Tracer(name, opts...), true
}
t, ok := p.namedTracer[is]
if !ok {
Expand Down
Loading

0 comments on commit 1e1cc90

Please sign in to comment.