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

fix(dynamic-sampling): Do not freeze DSC if no Sentry values are present #532

Merged
merged 5 commits into from Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion dynamic_sampling_context.go
Expand Up @@ -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
}

Expand Down
30 changes: 25 additions & 5 deletions dynamic_sampling_context_test.go
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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)
}
}

Expand Down
7 changes: 4 additions & 3 deletions tracing.go
Expand Up @@ -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,
}
Expand Down
78 changes: 78 additions & 0 deletions tracing_test.go
Expand Up @@ -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")
Expand Down