span.Finish() should be a noop when span is already finished #587
Description
At the moment we allow to call Finish() on spans (and, specifically, transactions) multiple times, and run the full Finish() logic every time.
It's not optimal, because finalizing the same span/transaction is not something that we officially support or recommend, and it also might cause a data race, as in the following example:
transaction := StartTransaction(ctx, "op")
go func() {
for {
transaction.Finish()
}
}()
go func() {
for {
transaction.Finish()
}
}()
time.Sleep(100 * time.Second)Data race log
==================
WARNING: DATA RACE
Read at 0x00c00013c9a8 by goroutine 9:
github.com/getsentry/sentry-go.(*environmentIntegration).processor()
/Users/anton/reps/sentry-go/integrations.go:91 +0x3ad
github.com/getsentry/sentry-go.(*environmentIntegration).processor-fm()
<autogenerated>:1 +0x4d
github.com/getsentry/sentry-go.(*Client).prepareEvent()
/Users/anton/reps/sentry-go/client.go:651 +0xb69
github.com/getsentry/sentry-go.(*Client).processEvent()
/Users/anton/reps/sentry-go/client.go:571 +0x4a4
github.com/getsentry/sentry-go.(*Client).CaptureEvent()
/Users/anton/reps/sentry-go/client.go:397 +0xa8
github.com/getsentry/sentry-go.(*Hub).CaptureEvent()
/Users/anton/reps/sentry-go/hub.go:224 +0x92
github.com/getsentry/sentry-go.(*Span).Finish()
/Users/anton/reps/sentry-go/tracing.go:205 +0x2e4
github.com/getsentry/sentry-go.TestSetContextAndFinishTransaction.func1()
/Users/anton/reps/sentry-go/tracing_test.go:850 +0x30
Previous write at 0x00c00013c9a8 by goroutine 10:
github.com/getsentry/sentry-go.(*environmentIntegration).processor()
/Users/anton/reps/sentry-go/integrations.go:92 +0x3f8
github.com/getsentry/sentry-go.(*environmentIntegration).processor-fm()
<autogenerated>:1 +0x4d
github.com/getsentry/sentry-go.(*Client).prepareEvent()
/Users/anton/reps/sentry-go/client.go:651 +0xb69
github.com/getsentry/sentry-go.(*Client).processEvent()
/Users/anton/reps/sentry-go/client.go:571 +0x4a4
github.com/getsentry/sentry-go.(*Client).CaptureEvent()
/Users/anton/reps/sentry-go/client.go:397 +0xa8
github.com/getsentry/sentry-go.(*Hub).CaptureEvent()
/Users/anton/reps/sentry-go/hub.go:224 +0x92
github.com/getsentry/sentry-go.(*Span).Finish()
/Users/anton/reps/sentry-go/tracing.go:205 +0x2e4
github.com/getsentry/sentry-go.TestSetContextAndFinishTransaction.func2()
/Users/anton/reps/sentry-go/tracing_test.go:861 +0x30
Goroutine 9 (running) created at:
github.com/getsentry/sentry-go.TestSetContextAndFinishTransaction()
/Users/anton/reps/sentry-go/tracing_test.go:848 +0x199
testing.tRunner()
/Users/anton/.asdf/installs/golang/1.20/go/src/testing/testing.go:1576 +0x216
testing.(*T).Run.func1()
/Users/anton/.asdf/installs/golang/1.20/go/src/testing/testing.go:1629 +0x47
Goroutine 10 (running) created at:
github.com/getsentry/sentry-go.TestSetContextAndFinishTransaction()
/Users/anton/reps/sentry-go/tracing_test.go:859 +0x2b1
testing.tRunner()
/Users/anton/.asdf/installs/golang/1.20/go/src/testing/testing.go:1576 +0x216
testing.(*T).Run.func1()
/Users/anton/.asdf/installs/golang/1.20/go/src/testing/testing.go:1629 +0x47
==================
The proposal is to make Span.Finish() a no-op after Finish() has already been called once.
Extracted from #570
Metadata
Metadata
Assignees
Labels
Fields
Give feedbackNo fields configured for issues without a type.