Skip to content

Commit

Permalink
apmotel: disallow closing spans twice (#1512)
Browse files Browse the repository at this point in the history
  • Loading branch information
dmathieu authored Sep 22, 2023
1 parent 5b156f2 commit 5580a9a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 0 deletions.
5 changes: 5 additions & 0 deletions module/apmotel/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type span struct {

provider *tracerProvider

ended bool
startTime time.Time
attributes []attribute.KeyValue
events []event
Expand All @@ -65,6 +66,9 @@ type span struct {
func (s *span) End(options ...trace.SpanEndOption) {
s.mu.Lock()
defer s.mu.Unlock()
if s.ended {
return
}

config := trace.NewSpanEndConfig(options...)

Expand All @@ -90,6 +94,7 @@ func (s *span) End(options ...trace.SpanEndOption) {
for iter := s.provider.resource.Iter(); iter.Next(); {
s.attributes = append(s.attributes, iter.Attribute())
}
s.ended = true

if s.span != nil {
s.setSpanAttributes()
Expand Down
50 changes: 50 additions & 0 deletions module/apmotel/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,56 @@ func TestSpanEnd(t *testing.T) {
}
}

func TestSpanEndTwice(t *testing.T) {
for _, tt := range []struct {
name string
getSpan func(context.Context, trace.Tracer) trace.Span

expectedSpansCount int
expectedTransactionsCount int
}{
{
name: "with a transaction",
getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span {
ctx, s := tracer.Start(ctx, "transaction")
return s
},

expectedTransactionsCount: 1,
},
{
name: "with a span",
getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span {
ctx, _ = tracer.Start(ctx, "transaction")
ctx, s := tracer.Start(ctx, "span")
return s
},

expectedSpansCount: 1,
},
} {
t.Run(tt.name, func(t *testing.T) {
apmTracer, recorder := transporttest.NewRecorderTracer()
tp, err := NewTracerProvider(
WithAPMTracer(apmTracer),
)
assert.NoError(t, err)
tracer := newTracer(tp.(*tracerProvider))

ctx := context.Background()
s := tt.getSpan(ctx, tracer)

s.End()
assert.NotPanics(t, func() { s.End() })

apmTracer.Flush(nil)
payloads := recorder.Payloads()
assert.Equal(t, tt.expectedSpansCount, len(payloads.Spans))
assert.Equal(t, tt.expectedTransactionsCount, len(payloads.Transactions))
})
}
}

func TestSpanAddEvent(t *testing.T) {
tp, err := NewTracerProvider()
assert.NoError(t, err)
Expand Down

0 comments on commit 5580a9a

Please sign in to comment.