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 bug in OnEnd and added a unit test #1

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
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()
defer ssp.exporterMu.Unlock()

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) {
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")

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