Skip to content

Commit

Permalink
TracerProvider allows calling Tracer() while it's shutting down
Browse files Browse the repository at this point in the history
  • Loading branch information
ash2k committed Mar 22, 2023
1 parent 795ad97 commit aeac359
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed

- `TracerProvider` consistently doesn't allow to register a `SpanProcessor` after shutdown. (#3845)
- `TracerProvider` allows calling `Tracer()` while it's shutting down. It used to deadlock.

### Removed

Expand Down
27 changes: 17 additions & 10 deletions sdk/trace/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ 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 {
tracerMu sync.Mutex // separate mutex because Tracer() may be called while holding mu via Shutdown().
namedTracer map[instrumentation.Scope]*tracer

mu sync.Mutex
namedTracer map[instrumentation.Scope]*tracer
spanProcessors atomic.Value
isShutdown bool

Expand Down Expand Up @@ -138,8 +140,6 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider {
func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer {
c := trace.NewTracerConfig(opts...)

p.mu.Lock()
defer p.mu.Unlock()
if name == "" {
name = defaultTracerName
}
Expand All @@ -148,14 +148,21 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
}
t, ok := p.namedTracer[is]
if !ok {
t = &tracer{
provider: p,
instrumentationScope: is,
t, ok := func() (*tracer, bool) {
p.tracerMu.Lock()
defer p.tracerMu.Unlock()
t, ok := p.namedTracer[is]
if !ok {
t = &tracer{
provider: p,
instrumentationScope: is,
}
p.namedTracer[is] = t
}
p.namedTracer[is] = t
global.Info("Tracer created", "name", name, "version", c.InstrumentationVersion(), "schemaURL", c.SchemaURL())
return t, ok
}()
if !ok {
global.Info("Tracer created", "name", name, "version", is.Version, "schemaURL", is.SchemaURL)
}
return t
}
Expand Down
27 changes: 27 additions & 0 deletions sdk/trace/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,33 @@ func (t *basicSpanProcessor) ForceFlush(context.Context) error {
return nil
}

type shutdownSpanProcessor struct {
shutdown func(context.Context) error
}

func (t *shutdownSpanProcessor) Shutdown(ctx context.Context) error {
return t.shutdown(ctx)
}

func (t *shutdownSpanProcessor) OnStart(context.Context, ReadWriteSpan) {}
func (t *shutdownSpanProcessor) OnEnd(ReadOnlySpan) {}
func (t *shutdownSpanProcessor) ForceFlush(context.Context) error {
return nil
}

func TestShutdownCallsTracerMethod(t *testing.T) {
stp := NewTracerProvider()
sp := &shutdownSpanProcessor{
shutdown: func(ctx context.Context) error {
_ = stp.Tracer("abc") // must not deadlock
return nil
},
}
stp.RegisterSpanProcessor(sp)
assert.NoError(t, stp.Shutdown(context.Background()))
assert.True(t, stp.isShutdown)
}

func TestForceFlushAndShutdownTraceProviderWithoutProcessor(t *testing.T) {
stp := NewTracerProvider()
assert.NoError(t, stp.ForceFlush(context.Background()))
Expand Down

0 comments on commit aeac359

Please sign in to comment.