Skip to content

Commit

Permalink
fix: Create a frozen DSC when calling ToBaggage (#566)
Browse files Browse the repository at this point in the history
  • Loading branch information
cleptric committed Feb 2, 2023
1 parent fb76180 commit d0a2aa3
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 27 deletions.
7 changes: 3 additions & 4 deletions dynamic_sampling_context_test.go
Expand Up @@ -2,8 +2,9 @@ package sentry

import (
"context"
"strings"
"testing"

"github.com/getsentry/sentry-go/internal/testutils"
)

func TestDynamicSamplingContextFromHeader(t *testing.T) {
Expand Down Expand Up @@ -177,7 +178,5 @@ func TestString(t *testing.T) {
"sample_rate": "1",
},
}
assertEqual(t, strings.Contains(dsc.String(), "sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03"), true)
assertEqual(t, strings.Contains(dsc.String(), "sentry-public_key=public"), true)
assertEqual(t, strings.Contains(dsc.String(), "sentry-sample_rate=1"), true)
testutils.AssertBaggageStringsEqual(t, dsc.String(), "sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1")
}
6 changes: 1 addition & 5 deletions otel/propagator.go
Expand Up @@ -46,11 +46,7 @@ func (p sentryPropagator) Inject(ctx context.Context, carrier propagation.TextMa
// Propagate baggage header
sentryBaggageStr := ""
if sentrySpan != nil {
// Finalize/freeze the baggage
containingTransaction := sentrySpan.GetTransaction()
dynamicSamplingContext := sentry.DynamicSamplingContextFromTransaction(containingTransaction)
containingTransaction.SetDynamicSamplingContext(dynamicSamplingContext)
sentryBaggageStr = containingTransaction.ToBaggage()
sentryBaggageStr = sentrySpan.GetTransaction().ToBaggage()
}
// FIXME: We're basically reparsing the header again, because internally in sentry-go
// we currently use a vendored version of "otel/baggage" package.
Expand Down
15 changes: 13 additions & 2 deletions otel/span_processor_test.go
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/getsentry/sentry-go"
"github.com/getsentry/sentry-go/internal/testutils"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/resource"
otelSdkTrace "go.opentelemetry.io/otel/sdk/trace"
Expand Down Expand Up @@ -129,9 +130,14 @@ func TestOnStartRootSpan(t *testing.T) {
assertEqual(t, sentrySpan.TraceID.String(), otelTraceId.String())
assertEqual(t, sentrySpan.ParentSpanID, sentry.SpanID{})
assertTrue(t, sentrySpan.IsTransaction())
assertEqual(t, sentrySpan.ToBaggage(), "")
assertEqual(t, sentrySpan.Sampled, sentry.SampledTrue)
assertEqual(t, transactionName(sentrySpan), "spanName")

testutils.AssertBaggageStringsEqual(
t,
sentrySpan.ToBaggage(),
"sentry-transaction=spanName,sentry-environment=testing,sentry-public_key=abc,sentry-release=1.2.3,sentry-sample_rate=1,sentry-trace_id="+otelTraceId.String(),
)
}

func TestOnStartWithTraceParentContext(t *testing.T) {
Expand Down Expand Up @@ -178,11 +184,16 @@ func TestOnStartWithTraceParentContext(t *testing.T) {
assertEqual(t, sentrySpan.TraceID.String(), "bc6d53f15eb88f4320054569b8c553d4")
assertEqual(t, sentrySpan.ParentSpanID, SpanIDFromHex("b72fa28504b07285"))
assertTrue(t, sentrySpan.IsTransaction())
assertEqual(t, sentrySpan.ToBaggage(), "sentry-environment=dev")
assertEqual(t, sentrySpan.Sampled, sentry.SampledFalse)
assertEqual(t, transactionName(sentrySpan), "spanName")
assertEqual(t, sentrySpan.Status, sentry.SpanStatusUndefined)
assertEqual(t, sentrySpan.Source, sentry.SourceCustom)

testutils.AssertBaggageStringsEqual(
t,
sentrySpan.ToBaggage(),
"sentry-environment=dev",
)
}

func TestOnStartWithExistingParentSpan(t *testing.T) {
Expand Down
16 changes: 13 additions & 3 deletions tracing.go
Expand Up @@ -270,8 +270,9 @@ func (s *Span) GetTransaction() *Span {
// func (s *Span) TransactionName() string
// func (s *Span) SetTransactionName(name string)

// ToSentryTrace returns the trace propagation value used with the sentry-trace
// HTTP header.
// ToSentryTrace returns the seralized TraceParentContext from a transaction/sapn.
// Use this function to propagate the TraceParentContext to a downstream SDK,
// either as the value of the "sentry-trace" HTTP header, or as an html "sentry-trace" meta tag.
func (s *Span) ToSentryTrace() string {
// TODO(tracing): add instrumentation for outgoing HTTP requests using
// ToSentryTrace.
Expand All @@ -286,9 +287,18 @@ func (s *Span) ToSentryTrace() string {
return b.String()
}

// ToBaggage returns the serialized dynamic sampling context in the baggage format.
// ToBaggage returns the serialized DynamicSamplingContext from a transaction.
// Use this function to propagate the DynamicSamplingContext to a downstream SDK,
// either as the value of the "baggage" HTTP header, or as an html "baggage" meta tag.
func (s *Span) ToBaggage() string {
if containingTransaction := s.GetTransaction(); containingTransaction != nil {
// In case there is currently no frozen DynamicSamplingContext attached to the transaction,
// create one from the properties of the transaction.
if !s.dynamicSamplingContext.IsFrozen() {
// This will return a frozen DynamicSamplingContext.
s.dynamicSamplingContext = DynamicSamplingContextFromTransaction(containingTransaction)
}

return containingTransaction.dynamicSamplingContext.String()
}
return ""
Expand Down
16 changes: 3 additions & 13 deletions tracing_test.go
Expand Up @@ -811,32 +811,22 @@ func TestGetTransactionReturnsNilOnManuallyCreatedSpans(t *testing.T) {
}
}

func TestSpanToBaggage(t *testing.T) {
func TestToBaggage(t *testing.T) {
ctx := NewTestContext(ClientOptions{
EnableTracing: true,
SampleRate: 1.0,
Release: "test-release",
})

// Should work transaction
transaction := StartTransaction(ctx, "transaction-name")
transaction.TraceID = TraceIDFromHex("f1a4c5c9071eca1cdf04e4132527ed16")
// Before finalizing
assertBaggageStringsEqual(
t,
transaction.ToBaggage(),
"",
)
dsc := DynamicSamplingContextFromTransaction(transaction)
transaction.SetDynamicSamplingContext(dsc)
// After finalizing

assertBaggageStringsEqual(
t,
transaction.ToBaggage(),
"sentry-trace_id=f1a4c5c9071eca1cdf04e4132527ed16,sentry-release=test-release,sentry-transaction=transaction-name",
)

// Should work on child span
// Calling ToBaggage() on a child span should return the same result
child := transaction.StartChild("op-name")
assertBaggageStringsEqual(
t,
Expand Down

0 comments on commit d0a2aa3

Please sign in to comment.