From 4922f4308b7395dfbfae975d47bb783db9ce1fb8 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 9 Mar 2021 19:48:42 -0800 Subject: [PATCH] Fix issue #1490, apply same logic as in the SDK Signed-off-by: Bogdan Drutu --- CHANGELOG.md | 1 + internal/global/trace.go | 40 ++++++++++++++++++++++++++++------- internal/global/trace_test.go | 19 +++++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2890d79d946c..d64df8c66cab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `SamplingResult.TraceState` is correctly propagated to a newly created span's `SpanContext`. (#1655) - Jaeger Exporter: Ensure mapping between OTEL and Jaeger span data complies with the specification. (#1626) +- Issue #1490, apply same logic as in the SDK. (#1687) ## [0.18.0] - 2020-03-03 diff --git a/internal/global/trace.go b/internal/global/trace.go index 7b6993153fe4..ea89447f39f3 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -44,9 +44,8 @@ import ( // All TracerProvider functionality is forwarded to a delegate once // configured. type tracerProvider struct { - mtx sync.Mutex - tracers []*tracer - + mtx sync.Mutex + tracers map[il]*tracer delegate trace.TracerProvider } @@ -63,14 +62,15 @@ var _ trace.TracerProvider = &tracerProvider{} // Delegation only happens on the first call to this method. All subsequent // calls result in no delegation changes. func (p *tracerProvider) setDelegate(provider trace.TracerProvider) { - if p.delegate != nil { - return - } - p.mtx.Lock() defer p.mtx.Unlock() p.delegate = provider + + if len(p.tracers) == 0 { + return + } + for _, t := range p.tracers { t.setDelegate(provider) } @@ -87,11 +87,35 @@ 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 := createKey(name, opts...) + + 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 +} + +func createKey(name string, opts ...trace.TracerOption) il { + return il{ + name: name, + version: trace.NewTracerConfig(opts...).InstrumentationVersion, + } +} + // tracer is a placeholder for a trace.Tracer. // // All Tracer functionality is forwarded to a delegate once configured. diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go index 8eca6001f180..1ac3dee1b8cc 100644 --- a/internal/global/trace_test.go +++ b/internal/global/trace_test.go @@ -91,3 +91,22 @@ func TestTraceProviderDelegates(t *testing.T) { gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")) assert.True(t, called, "expected configured TraceProvider to be called") } + +func TestTraceProviderDelegates_SameInstance(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"))) +}