diff --git a/CHANGELOG.md b/CHANGELOG.md index 1afd5e7b250..455d29266ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Use default view if instrument does not match any registered view of a reader. (#3224, #3237) +- `sdktrace.TraceProvider.Shutdown` and `sdktrace.TraceProvider.ForceFlush` to not return error when no processor register. (#3268) ## [0.32.1] Metric SDK (Alpha) - 2022-09-22 diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 292ea5481bc..6a7fe3cb6c4 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -116,7 +116,7 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider { spanLimits: o.spanLimits, resource: o.resource, } - + tp.spanProcessors.Store(spanProcessorStates{}) global.Info("TracerProvider created", "config", o) for _, sp := range o.processors { @@ -163,14 +163,8 @@ func (p *TracerProvider) RegisterSpanProcessor(s SpanProcessor) { p.mu.Lock() defer p.mu.Unlock() newSPS := spanProcessorStates{} - if old, ok := p.spanProcessors.Load().(spanProcessorStates); ok { - newSPS = append(newSPS, old...) - } - newSpanSync := &spanProcessorState{ - sp: s, - state: &sync.Once{}, - } - newSPS = append(newSPS, newSpanSync) + newSPS = append(newSPS, p.spanProcessors.Load().(spanProcessorStates)...) + newSPS = append(newSPS, &spanProcessorState{sp: s, state: &sync.Once{}}) p.spanProcessors.Store(newSPS) } @@ -178,11 +172,11 @@ func (p *TracerProvider) RegisterSpanProcessor(s SpanProcessor) { func (p *TracerProvider) UnregisterSpanProcessor(s SpanProcessor) { p.mu.Lock() defer p.mu.Unlock() - spss := spanProcessorStates{} - old, ok := p.spanProcessors.Load().(spanProcessorStates) - if !ok || len(old) == 0 { + old := p.spanProcessors.Load().(spanProcessorStates) + if len(old) == 0 { return } + spss := spanProcessorStates{} spss = append(spss, old...) // stop the span processor if it is started and remove it from the list @@ -213,10 +207,7 @@ func (p *TracerProvider) UnregisterSpanProcessor(s SpanProcessor) { // ForceFlush immediately exports all spans that have not yet been exported for // all the registered span processors. func (p *TracerProvider) ForceFlush(ctx context.Context) error { - spss, ok := p.spanProcessors.Load().(spanProcessorStates) - if !ok { - return fmt.Errorf("failed to load span processors") - } + spss := p.spanProcessors.Load().(spanProcessorStates) if len(spss) == 0 { return nil } @@ -237,10 +228,11 @@ func (p *TracerProvider) ForceFlush(ctx context.Context) error { // Shutdown shuts down the span processors in the order they were registered. func (p *TracerProvider) Shutdown(ctx context.Context) error { - spss, ok := p.spanProcessors.Load().(spanProcessorStates) - if !ok { - return fmt.Errorf("failed to load span processors") + spss := p.spanProcessors.Load().(spanProcessorStates) + if len(spss) == 0 { + return nil } + var retErr error for _, sps := range spss { select { diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index 6ec3df6794d..e05822c1978 100644 --- a/sdk/trace/provider_test.go +++ b/sdk/trace/provider_test.go @@ -28,41 +28,45 @@ import ( "go.opentelemetry.io/otel/trace" ) -type basicSpanProcesor struct { - running bool +type basicSpanProcessor struct { + flushed bool + closed bool injectShutdownError error } -func (t *basicSpanProcesor) Shutdown(context.Context) error { - t.running = false +func (t *basicSpanProcessor) Shutdown(context.Context) error { + t.closed = true return t.injectShutdownError } -func (t *basicSpanProcesor) OnStart(context.Context, ReadWriteSpan) {} -func (t *basicSpanProcesor) OnEnd(ReadOnlySpan) {} -func (t *basicSpanProcesor) ForceFlush(context.Context) error { +func (t *basicSpanProcessor) OnStart(context.Context, ReadWriteSpan) {} +func (t *basicSpanProcessor) OnEnd(ReadOnlySpan) {} +func (t *basicSpanProcessor) ForceFlush(context.Context) error { + t.flushed = true return nil } +func TestForceFlushAndShutdownTraceProviderWithoutProcessor(t *testing.T) { + stp := NewTracerProvider() + assert.NoError(t, stp.ForceFlush(context.Background())) + assert.NoError(t, stp.Shutdown(context.Background())) +} + func TestShutdownTraceProvider(t *testing.T) { stp := NewTracerProvider() - sp := &basicSpanProcesor{} + sp := &basicSpanProcessor{} stp.RegisterSpanProcessor(sp) - sp.running = true - - _ = stp.Shutdown(context.Background()) - - if sp.running { - t.Errorf("Error shutdown basicSpanProcesor\n") - } + assert.NoError(t, stp.ForceFlush(context.Background())) + assert.True(t, sp.flushed, "error shutdown basicSpanProcessor") + assert.NoError(t, stp.Shutdown(context.Background())) + assert.True(t, sp.closed, "error shutdown basicSpanProcessor") } func TestFailedProcessorShutdown(t *testing.T) { stp := NewTracerProvider() spErr := errors.New("basic span processor shutdown failure") - sp := &basicSpanProcesor{ - running: true, + sp := &basicSpanProcessor{ injectShutdownError: spErr, } stp.RegisterSpanProcessor(sp) @@ -76,12 +80,10 @@ func TestFailedProcessorsShutdown(t *testing.T) { stp := NewTracerProvider() spErr1 := errors.New("basic span processor shutdown failure1") spErr2 := errors.New("basic span processor shutdown failure2") - sp1 := &basicSpanProcesor{ - running: true, + sp1 := &basicSpanProcessor{ injectShutdownError: spErr1, } - sp2 := &basicSpanProcesor{ - running: true, + sp2 := &basicSpanProcessor{ injectShutdownError: spErr2, } stp.RegisterSpanProcessor(sp1) @@ -90,16 +92,15 @@ func TestFailedProcessorsShutdown(t *testing.T) { err := stp.Shutdown(context.Background()) assert.Error(t, err) assert.EqualError(t, err, "basic span processor shutdown failure1; basic span processor shutdown failure2") - assert.False(t, sp1.running) - assert.False(t, sp2.running) + assert.True(t, sp1.closed) + assert.True(t, sp2.closed) } func TestFailedProcessorShutdownInUnregister(t *testing.T) { handler.Reset() stp := NewTracerProvider() spErr := errors.New("basic span processor shutdown failure") - sp := &basicSpanProcesor{ - running: true, + sp := &basicSpanProcessor{ injectShutdownError: spErr, } stp.RegisterSpanProcessor(sp)