From 7d58d1c23ba80cb4c4b7399fc403884a6fad789a Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 9 Mar 2021 19:10:23 -0800 Subject: [PATCH] Fix issue #1490, apply same logic as in the SDK Signed-off-by: Bogdan Drutu --- internal/global/trace.go | 38 +++++++++++++++++++++++------------ internal/global/trace_test.go | 19 ++++++++++++++++++ 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/internal/global/trace.go b/internal/global/trace.go index b2fdb8b66ce0..30046c512acb 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -45,8 +45,7 @@ import ( // All TracerProvider functionality is forwarded to a delegate once // configured. type tracerProvider struct { - mtx sync.Mutex - tracers []*tracer + tracers sync.Map delegate atomic.Value } @@ -63,12 +62,11 @@ var _ trace.TracerProvider = &tracerProvider{} // It is guaranteed by the caller that this happens only once. func (p *tracerProvider) setDelegate(provider trace.TracerProvider) { p.delegate.Store(provider) - p.mtx.Lock() - defer p.mtx.Unlock() - - for _, t := range p.tracers { - t.setDelegate(provider) - } + p.tracers.Range(func(key, value interface{}) bool { + value.(*tracer).setDelegate(provider) + p.tracers.Delete(key) + return true + }) } // Tracer implements TracerProvider. @@ -78,12 +76,26 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T return delegate.(trace.TracerProvider).Tracer(name, opts...) } - p.mtx.Lock() - defer p.mtx.Unlock() + key := createKey(name, opts...) + if val, ok := p.tracers.Load(key); ok { + return val.(*tracer) + } + + // Value not present, we cannot be sure that there is not any other concurrent + t, _ := p.tracers.LoadOrStore(key, &tracer{name: name, opts: opts}) + return t.(*tracer) +} + +type il struct { + name string + version string +} - t := &tracer{name: name, opts: opts} - p.tracers = append(p.tracers, t) - return t +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. diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go index f5a0c11c27f8..d834376d4d9b 100644 --- a/internal/global/trace_test.go +++ b/internal/global/trace_test.go @@ -102,6 +102,25 @@ func TestTraceProviderDelegates(t *testing.T) { 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"))) +} + func TestTraceProviderDelegates_MultiGoRoutines(t *testing.T) { global.ResetForTest()