Skip to content

Commit

Permalink
Fix issue #1490, apply same logic as in the SDK (#1687)
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored Mar 10, 2021
1 parent 9d3416c commit 77aa218
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Jaeger Exporter: Ensure mapping between OTEL and Jaeger span data complies with the specification. (#1626)
- The `otel-collector` example now correctly flushes metric events prior to shutting down the exporter. (#1678)
- Synchronization issues in global trace delegate implementation. (#1686)

### Fixed
- Do not set span status message in `SpanStatusFromHTTPStatusCode` if it can be inferred from `http.status_code`. (#1681)
- Reduced excess memory usage by global `TracerProvider`. (#1687)

## [0.18.0] - 2020-03-03

Expand Down
29 changes: 27 additions & 2 deletions internal/global/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import (
// configured.
type tracerProvider struct {
mtx sync.Mutex
tracers []*tracer
tracers map[il]*tracer
delegate trace.TracerProvider
}

Expand All @@ -66,6 +66,11 @@ func (p *tracerProvider) setDelegate(provider trace.TracerProvider) {
defer p.mtx.Unlock()

p.delegate = provider

if len(p.tracers) == 0 {
return
}

for _, t := range p.tracers {
t.setDelegate(provider)
}
Expand All @@ -82,11 +87,31 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
return p.delegate.Tracer(name, opts...)
}

// At this moment it is guaranteed that no sdk is installed, save the tracer in the tracers map.

key := il{
name: name,
version: trace.NewTracerConfig(opts...).InstrumentationVersion,
}

if p.tracers == nil {
p.tracers = make(map[il]*tracer)
}

if val, ok := p.tracers[key]; ok {
return val
}

t := &tracer{name: name, opts: opts}
p.tracers = append(p.tracers, t)
p.tracers[key] = t
return t
}

type il struct {
name string
version string
}

// tracer is a placeholder for a trace.Tracer.
//
// All Tracer functionality is forwarded to a delegate once configured.
Expand Down
18 changes: 18 additions & 0 deletions internal/global/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,21 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) {

assert.LessOrEqual(t, int32(10), atomic.LoadInt32(&called), "expected configured TraceProvider to be called")
}

func TestTraceProviderDelegatesSameInstance(t *testing.T) {
global.ResetForTest()

// Retrieve the placeholder TracerProvider.
gtp := otel.GetTracerProvider()
tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))

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

assert.NotSame(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
}

0 comments on commit 77aa218

Please sign in to comment.