diff --git a/CHANGELOG.md b/CHANGELOG.md index c3eb300b28c..2bba4d3f40d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Rename `Float64ObserverOption` to `Float64ObservableOption` in `go.opentelemetry.io/otel/metric/instrument`. (#3895) - The internal logging changes the verbosity level of info to `V(4)`, the verbosity level of debug to `V(8)`. (#3900) +### Fixed + +- `TracerProvider` consistently doesn't allow to register a `SpanProcessor` after shutdown. (#3845) + ### Removed - The deprecated `go.opentelemetry.io/otel/metric/global` package is removed. (#3829) diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 201c1781700..0636c780736 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -237,14 +237,13 @@ func (p *TracerProvider) ForceFlush(ctx context.Context) error { // Shutdown shuts down TracerProvider. All registered span processors are shut down // in the order they were registered and any held computational resources are released. func (p *TracerProvider) Shutdown(ctx context.Context) error { - spss := p.spanProcessors.Load().(spanProcessorStates) - if len(spss) == 0 { - return nil - } - p.mu.Lock() defer p.mu.Unlock() + if p.isShutdown { + return nil + } p.isShutdown = true + spss := p.spanProcessors.Load().(spanProcessorStates) var retErr error for _, sps := range spss { diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index 2cf19aaa029..5d984ff3b86 100644 --- a/sdk/trace/provider_test.go +++ b/sdk/trace/provider_test.go @@ -50,6 +50,7 @@ func TestForceFlushAndShutdownTraceProviderWithoutProcessor(t *testing.T) { stp := NewTracerProvider() assert.NoError(t, stp.ForceFlush(context.Background())) assert.NoError(t, stp.Shutdown(context.Background())) + assert.True(t, stp.isShutdown) } func TestShutdownTraceProvider(t *testing.T) { @@ -60,6 +61,7 @@ func TestShutdownTraceProvider(t *testing.T) { assert.NoError(t, stp.ForceFlush(context.Background())) assert.True(t, sp.flushed, "error ForceFlush basicSpanProcessor") assert.NoError(t, stp.Shutdown(context.Background())) + assert.True(t, stp.isShutdown) assert.True(t, sp.closed, "error Shutdown basicSpanProcessor") } @@ -74,6 +76,7 @@ func TestFailedProcessorShutdown(t *testing.T) { err := stp.Shutdown(context.Background()) assert.Error(t, err) assert.Equal(t, err, spErr) + assert.True(t, stp.isShutdown) } func TestFailedProcessorsShutdown(t *testing.T) { @@ -94,6 +97,7 @@ func TestFailedProcessorsShutdown(t *testing.T) { assert.EqualError(t, err, "basic span processor shutdown failure1; basic span processor shutdown failure2") assert.True(t, sp1.closed) assert.True(t, sp2.closed) + assert.True(t, stp.isShutdown) } func TestFailedProcessorShutdownInUnregister(t *testing.T) { @@ -110,6 +114,7 @@ func TestFailedProcessorShutdownInUnregister(t *testing.T) { err := stp.Shutdown(context.Background()) assert.NoError(t, err) + assert.True(t, stp.isShutdown) } func TestSchemaURL(t *testing.T) { @@ -122,6 +127,32 @@ func TestSchemaURL(t *testing.T) { assert.EqualValues(t, schemaURL, tracerStruct.instrumentationScope.SchemaURL) } +func TestRegisterAfterShutdownWithoutProcessors(t *testing.T) { + stp := NewTracerProvider() + err := stp.Shutdown(context.Background()) + assert.NoError(t, err) + assert.True(t, stp.isShutdown) + + sp := &basicSpanProcessor{} + stp.RegisterSpanProcessor(sp) // no-op + assert.Empty(t, stp.spanProcessors.Load().(spanProcessorStates)) +} + +func TestRegisterAfterShutdownWithProcessors(t *testing.T) { + stp := NewTracerProvider() + sp1 := &basicSpanProcessor{} + + stp.RegisterSpanProcessor(sp1) + err := stp.Shutdown(context.Background()) + assert.NoError(t, err) + assert.True(t, stp.isShutdown) + assert.Empty(t, stp.spanProcessors.Load().(spanProcessorStates)) + + sp2 := &basicSpanProcessor{} + stp.RegisterSpanProcessor(sp2) // no-op + assert.Empty(t, stp.spanProcessors.Load().(spanProcessorStates)) +} + func TestTracerProviderSamplerConfigFromEnv(t *testing.T) { type testCase struct { sampler string