fix(tracing): Protect from concurrent reads/writes to Span Tags, Cont…

…exts, Data (#609)

Span.SetTag(), Span.SetData(), Span.SetContext(), and Span.toEvent() read and
write to underlying Span.Tags, Span.Data, and Span.contexts maps,
which might read to panics if the operations are not synchronized.

This PR adds a mutex that makes sure that those methods are not executed
in parallel.

tracing.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"regexp"
 	"strings"
+	"sync"
 	"time"
 )
 
@@ -37,6 +38,8 @@ type Span struct { //nolint: maligned // prefer readability over optimal memory
 	Sampled      Sampled                `json:"-"`
 	Source       TransactionSource      `json:"-"`
 
+	// mu protects concurrent writes to map fields
+	mu sync.RWMutex
 	// sample rate the span was sampled with.
 	sampleRate float64
 	// ctx is the context where the span was started. Always non-nil.
@@ -221,6 +224,9 @@ func (s *Span) StartChild(operation string, options ...SpanOption) *Span {
 // accessing the tags map directly as SetTag takes care of initializing the map
 // when necessary.
 func (s *Span) SetTag(name, value string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if s.Tags == nil {
 		s.Tags = make(map[string]string)
 	}
@@ -231,6 +237,9 @@ func (s *Span) SetTag(name, value string) {
 // accessing the data map directly as SetData takes care of initializing the map
 // when necessary.
 func (s *Span) SetData(name, value string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if s.Data == nil {
 		s.Data = make(map[string]interface{})
 	}
@@ -241,6 +250,9 @@ func (s *Span) SetData(name, value string) {
 // accessing the contexts map directly as SetContext takes care of initializing the map
 // when necessary.
 func (s *Span) SetContext(key string, value Context) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if s.contexts == nil {
 		s.contexts = make(map[string]Context)
 	}
@@ -478,6 +490,9 @@ func (s *Span) sample() Sampled {
 }
 
 func (s *Span) toEvent() *Event {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if !s.isTransaction {
 		return nil // only transactions can be transformed into events
 	}

tracing_test.go

@@ -476,13 +476,14 @@ func TestContinueTransactionFromHeaders(t *testing.T) {
 	tests := []struct {
 		traceStr   string
 		baggageStr string
-		wantSpan   Span
+		// Using a pointer to Span so we don't implicitly copy Span.mu mutex
+		wantSpan *Span
 	}{
 		{
 			// No sentry-trace or baggage => nothing to do, unfrozen DSC
 			traceStr:   "",
 			baggageStr: "",
-			wantSpan: Span{
+			wantSpan: &Span{
 				isTransaction: true,
 				Sampled:       0,
 				dynamicSamplingContext: DynamicSamplingContext{
@@ -495,7 +496,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) {
 			// Third-party baggage => nothing to do, unfrozen DSC
 			traceStr:   "",
 			baggageStr: "other-vendor-key1=value1;value2, other-vendor-key2=value3",
-			wantSpan: Span{
+			wantSpan: &Span{
 				isTransaction: true,
 				Sampled:       0,
 				dynamicSamplingContext: DynamicSamplingContext{
@@ -509,7 +510,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) {
 			// immediately.
 			traceStr:   "bc6d53f15eb88f4320054569b8c553d4-b72fa28504b07285-1",
 			baggageStr: "",
-			wantSpan: Span{
+			wantSpan: &Span{
 				isTransaction: true,
 				TraceID:       TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"),
 				ParentSpanID:  SpanIDFromHex("b72fa28504b07285"),
@@ -523,7 +524,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) {
 			// sentry-trace and baggage with Sentry values => we freeze immediately.
 			traceStr:   "bc6d53f15eb88f4320054569b8c553d4-b72fa28504b07285-1",
 			baggageStr: "sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1",
-			wantSpan: Span{
+			wantSpan: &Span{
 				isTransaction: true,
 				TraceID:       TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"),
 				ParentSpanID:  SpanIDFromHex("b72fa28504b07285"),
@@ -541,9 +542,9 @@ func TestContinueTransactionFromHeaders(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		s := Span{isTransaction: true}
+		s := &Span{isTransaction: true}
 		spanOption := ContinueFromHeaders(tt.traceStr, tt.baggageStr)
-		spanOption(&s)
+		spanOption(s)
 
 		assertEqual(t, s, tt.wantSpan)
 	}