Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed race condition in OnEnd and added a unit test #3951

Merged
merged 12 commits into from
Apr 14, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `TracerProvider` allows calling `Tracer()` while it's shutting down.
It used to deadlock. (#3924)
- Use the SDK version for the Telemetry SDK resource detector in `go.opentelemetry.io/otel/sdk/resource`. (#3949)
- Fix a data race in `SpanProcessor` returned by `NewSimpleSpanProcessor` in `go.opentelemetry.io/otel/sdk/trace`. (#3951)
- Automatically figure out the default aggregation with `aggregation.Default`. (#3967)

## [1.15.0-rc.2/0.38.0-rc.2] 2023-03-23
Expand Down
6 changes: 3 additions & 3 deletions sdk/trace/simple_span_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
// simpleSpanProcessor is a SpanProcessor that synchronously sends all
// completed Spans to a trace.Exporter immediately.
type simpleSpanProcessor struct {
exporterMu sync.RWMutex
exporterMu sync.Mutex
exporter SpanExporter
stopOnce sync.Once
}
Expand Down Expand Up @@ -54,8 +54,8 @@ func (ssp *simpleSpanProcessor) OnStart(context.Context, ReadWriteSpan) {}

// OnEnd immediately exports a ReadOnlySpan.
func (ssp *simpleSpanProcessor) OnEnd(s ReadOnlySpan) {
ssp.exporterMu.RLock()
defer ssp.exporterMu.RUnlock()
ssp.exporterMu.Lock()
Kaushal28 marked this conversation as resolved.
Show resolved Hide resolved
defer ssp.exporterMu.Unlock()
Kaushal28 marked this conversation as resolved.
Show resolved Hide resolved

if ssp.exporter != nil && s.SpanContext().TraceFlags().IsSampled() {
if err := ssp.exporter.ExportSpans(context.Background(), []ReadOnlySpan{s}); err != nil {
Expand Down
29 changes: 29 additions & 0 deletions sdk/trace/simple_span_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ package trace_test
import (
"context"
"errors"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"

sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/trace"
)
Expand Down Expand Up @@ -150,6 +153,32 @@ func TestSimpleSpanProcessorShutdownOnEndConcurrency(t *testing.T) {
<-done
}

func TestSimpleSpanProcessorShutdownOnEndRace(t *testing.T) {
pellared marked this conversation as resolved.
Show resolved Hide resolved
exporter := &testExporter{}
ssp := sdktrace.NewSimpleSpanProcessor(exporter)
tp := basicTracerProvider(t)
tp.RegisterSpanProcessor(ssp)

var wg sync.WaitGroup
wg.Add(2)

span := func(spanName string) {
assert.NotPanics(t, func() {
defer wg.Done()
_, span := tp.Tracer("test").Start(context.Background(), spanName)
span.End()
})
}

go span("test-span-1")
go span("test-span-2")
Kaushal28 marked this conversation as resolved.
Show resolved Hide resolved

wg.Wait()

assert.NoError(t, ssp.Shutdown(context.Background()))
assert.True(t, exporter.shutdown, "exporter shutdown")
}

func TestSimpleSpanProcessorShutdownHonorsContextDeadline(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
defer cancel()
Expand Down