fix(dynamic-sampling): Do not freeze DSC if no Sentry values are pres…

…ent (#532)
getsentry · Jan 18, 2023 · 808b6d3 · 808b6d3
1 parent fe08ff2
commit 808b6d3
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 9 deletions.
diff --git a/dynamic_sampling_context.go b/dynamic_sampling_context.go
@@ -33,7 +33,8 @@ func DynamicSamplingContextFromHeader(header []byte) (DynamicSamplingContext, er
 
 	return DynamicSamplingContext{
 		Entries: entries,
-		Frozen:  true,
+		// If there's at least one Sentry value, we consider the DSC frozen
+		Frozen: len(entries) > 0,
 	}, nil
 }
 

diff --git a/dynamic_sampling_context_test.go b/dynamic_sampling_context_test.go
@@ -8,16 +8,27 @@ import (
 
 func TestDynamicSamplingContextFromHeader(t *testing.T) {
 	tests := []struct {
-		input []byte
-		want  DynamicSamplingContext
+		input  []byte
+		want   DynamicSamplingContext
+		errMsg string
 	}{
+		// Empty baggage header
 		{
 			input: []byte(""),
 			want: DynamicSamplingContext{
-				Frozen:  true,
+				Frozen:  false,
 				Entries: map[string]string{},
 			},
 		},
+		// Third-party baggage
+		{
+			input: []byte("other-vendor-key1=value1;value2, other-vendor-key2=value3"),
+			want: DynamicSamplingContext{
+				Frozen:  false,
+				Entries: map[string]string{},
+			},
+		},
+		// Sentry-only baggage
 		{
 			input: []byte("sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1"),
 			want: DynamicSamplingContext{
@@ -29,6 +40,7 @@ func TestDynamicSamplingContextFromHeader(t *testing.T) {
 				},
 			},
 		},
+		// Mixed baggage
 		{
 			input: []byte("sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1,foo=bar;foo;bar;bar=baz"),
 			want: DynamicSamplingContext{
@@ -40,14 +52,22 @@ func TestDynamicSamplingContextFromHeader(t *testing.T) {
 				},
 			},
 		},
+		// Invalid baggage value
+		{
+			input: []byte(","),
+			want: DynamicSamplingContext{
+				Frozen: false,
+			},
+			errMsg: "invalid baggage list-member: \"\"",
+		},
 	}
 
 	for _, tc := range tests {
 		got, err := DynamicSamplingContextFromHeader(tc.input)
+		assertEqual(t, got, tc.want, "Context mismatch")
 		if err != nil {
-			t.Fatal(err)
+			assertEqual(t, err.Error(), tc.errMsg, "Error mismatch")
 		}
-		assertEqual(t, got, tc.want)
 	}
 }
 

diff --git a/tracing.go b/tracing.go
@@ -699,9 +699,10 @@ func ContinueFromHeaders(trace, baggage string) SpanOption {
 		if baggage != "" {
 			s.updateFromBaggage([]byte(baggage))
 		}
-		// In case a sentry-trace header is present but no baggage header,
-		// create an empty, frozen DynamicSamplingContext.
-		if trace != "" && baggage == "" {
+
+		// In case a sentry-trace header is present but there are no sentry-related
+		// values in the baggage, create an empty, frozen DynamicSamplingContext.
+		if trace != "" && !s.dynamicSamplingContext.HasEntries() {
 			s.dynamicSamplingContext = DynamicSamplingContext{
 				Frozen: true,
 			}

diff --git a/tracing_test.go b/tracing_test.go
@@ -429,6 +429,84 @@ func TestContinueSpanFromRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestContinueTransactionFromHeaders(t *testing.T) {
+	tests := []struct {
+		traceStr   string
+		baggageStr string
+		wantSpan   Span
+	}{
+		{
+			// No sentry-trace or baggage => nothing to do, unfrozen DSC
+			traceStr:   "",
+			baggageStr: "",
+			wantSpan: Span{
+				isTransaction: true,
+				Sampled:       0,
+				dynamicSamplingContext: DynamicSamplingContext{
+					Frozen:  false,
+					Entries: nil,
+				},
+			},
+		},
+		{
+			// Third-party baggage => nothing to do, unfrozen DSC
+			traceStr:   "",
+			baggageStr: "other-vendor-key1=value1;value2, other-vendor-key2=value3",
+			wantSpan: Span{
+				isTransaction: true,
+				Sampled:       0,
+				dynamicSamplingContext: DynamicSamplingContext{
+					Frozen:  false,
+					Entries: map[string]string{},
+				},
+			},
+		},
+		{
+			// sentry-trace and no baggage => we should create a new DSC and freeze it
+			// immediately.
+			traceStr:   "bc6d53f15eb88f4320054569b8c553d4-b72fa28504b07285-1",
+			baggageStr: "",
+			wantSpan: Span{
+				isTransaction: true,
+				TraceID:       TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"),
+				ParentSpanID:  SpanIDFromHex("b72fa28504b07285"),
+				Sampled:       1,
+				dynamicSamplingContext: DynamicSamplingContext{
+					Frozen: true,
+				},
+			},
+		},
+		{
+			// 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{
+				isTransaction: true,
+				TraceID:       TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"),
+				ParentSpanID:  SpanIDFromHex("b72fa28504b07285"),
+				Sampled:       1,
+				dynamicSamplingContext: DynamicSamplingContext{
+					Frozen: true,
+					Entries: map[string]string{
+						"public_key":  "public",
+						"sample_rate": "1",
+						"trace_id":    "d49d9bf66f13450b81f65bc51cf49c03",
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		s := Span{isTransaction: true}
+		spanOption := ContinueFromHeaders(tt.traceStr, tt.baggageStr)
+		spanOption(&s)
+
+		assertEqual(t, s, tt.wantSpan)
+	}
+}
+
 func TestContinueSpanFromTrace(t *testing.T) {
 	traceID := TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4")
 	spanID := SpanIDFromHex("b72fa28504b07285")