fix(otel): Use vendored bagggage implementation in propagator (#573)

otel/context.go

@@ -6,3 +6,4 @@ package sentryotel
 type dynamicSamplingContextKey struct{}
 type sentryTraceHeaderContextKey struct{}
 type sentryTraceParentContextKey struct{}
+type baggageContextKey struct{}

otel/helpers_test.go

@@ -10,9 +10,9 @@ import (
 	"time"
 
 	"github.com/getsentry/sentry-go"
+	"github.com/getsentry/sentry-go/internal/otel/baggage"
 	"github.com/getsentry/sentry-go/internal/testutils"
 	"github.com/google/go-cmp/cmp"
-	"go.opentelemetry.io/otel/baggage"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -28,6 +28,8 @@ var assertFalse = testutils.AssertFalse
 // It is needed because some headers (e.g. "baggage") might contain the same set of values/attributes,
 // (and therefore be semantically equal), but serialized in different order.
 func assertMapCarrierEqual(t *testing.T, got, want propagation.MapCarrier, userMessage ...interface{}) {
+	t.Helper()
+
 	// Make sure that keys are the same
 	gotKeysSorted := got.Keys()
 	sort.Strings(gotKeysSorted)

otel/propagator.go

@@ -6,7 +6,7 @@ import (
 	"context"
 
 	"github.com/getsentry/sentry-go"
-	"go.opentelemetry.io/otel/baggage"
+	"github.com/getsentry/sentry-go/internal/otel/baggage"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -48,13 +48,16 @@ func (p sentryPropagator) Inject(ctx context.Context, carrier propagation.TextMa
 	if sentrySpan != nil {
 		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.
+	// FIXME(anton): We're basically reparsing the header again, because in sentry-go
+	// we currently don't expose a method to get only DSC or its baggage (only a string).
 	// This is not optimal and we should consider other approaches.
 	sentryBaggage, _ := baggage.Parse(sentryBaggageStr)
 
 	// Merge the baggage values
-	finalBaggage := baggage.FromContext(ctx)
+	finalBaggage, baggageOk := ctx.Value(baggageContextKey{}).(baggage.Baggage)
+	if !baggageOk {
+		finalBaggage = baggage.Baggage{}
+	}
 	for _, member := range sentryBaggage.Members() {
 		var err error
 		finalBaggage, err = finalBaggage.SetMember(member)
@@ -96,7 +99,7 @@ func (p sentryPropagator) Extract(ctx context.Context, carrier propagation.TextM
 		// Preserve the original baggage
 		parsedBaggage, err := baggage.Parse(baggageHeader)
 		if err == nil {
-			ctx = baggage.ContextWithBaggage(ctx, parsedBaggage)
+			ctx = context.WithValue(ctx, baggageContextKey{}, parsedBaggage)
 		}
 	}
 

otel/propagator_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 
 	"github.com/getsentry/sentry-go"
-	"go.opentelemetry.io/otel/baggage"
+	"github.com/getsentry/sentry-go/internal/otel/baggage"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -106,7 +106,7 @@ func TestInjectUsesSentryTraceOnEmptySpan(t *testing.T) {
 func TestInjectUsesBaggageOnEmptySpan(t *testing.T) {
 	propagator, carrier := setupPropagatorTest()
 	bag, _ := baggage.Parse("key1=value1;value2, key2=value2")
-	ctx := baggage.ContextWithBaggage(context.Background(), bag)
+	ctx := context.WithValue(context.Background(), baggageContextKey{}, bag)
 
 	propagator.Inject(ctx, carrier)
 
@@ -292,47 +292,54 @@ func TestExtractSetsDefinedDynamicSamplingContext(t *testing.T) {
 
 /// Integration tests
 
-// Valid baggage and sentry-trace headers
-func TestExtractAndInjectValidSentryTraceAndBaggage(t *testing.T) {
-	propagator, incomingCarrier := setupPropagatorTest()
-	outgoingCarrier := propagation.MapCarrier{}
-	incomingCarrier.Set(
-		sentry.SentryBaggageHeader,
-		"sentry-environment=production,sentry-release=1.0.0,othervendor=bla,sentry-transaction=dsc-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b",
-	)
-	incomingCarrier.Set(
-		sentry.SentryTraceHeader,
-		"d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1",
-	)
-
-	ctx := propagator.Extract(context.Background(), incomingCarrier)
-	propagator.Inject(ctx, outgoingCarrier)
-
-	assertMapCarrierEqual(t,
-		outgoingCarrier,
-		propagation.MapCarrier{
-			"baggage":      "sentry-environment=production,sentry-release=1.0.0,othervendor=bla,sentry-transaction=dsc-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b",
-			"sentry-trace": "d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1",
+func TestExtractAndInjectIntegration(t *testing.T) {
+	tests := []struct {
+		name          string
+		inSentryTrace *string
+		inBaggage     *string
+	}{
+		{
+			name:          "valid sentry-trace and baggage",
+			inSentryTrace: stringPtr("d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1"),
+			inBaggage:     stringPtr("sentry-environment=production,sentry-release=1.0.0,othervendor=bla,sentry-transaction=dsc-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b"),
 		},
-	)
-}
+		{
+			name:          "only sentry-trace, no baggage",
+			inSentryTrace: stringPtr("d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1"),
+		},
+		{
+			name:          "valid sentry-trace and mixed baggage with special characters",
+			inSentryTrace: stringPtr("d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1"),
+			inBaggage:     stringPtr("sentry-transaction=GET%20POST,userId=Am%C3%A9lie, key1 = +++ , key2=%253B"),
+		},
+	}
 
-// No sentry-trace header, and baggage without sentry values
-func TestExtractAndInjectNoSentryTraceAndExistingBaggage(t *testing.T) {
-	propagator, incomingCarrier := setupPropagatorTest()
-	outgoingCarrier := propagation.MapCarrier{}
-	incomingCarrier.Set(
-		sentry.SentryBaggageHeader,
-		"othervendor=bla",
-	)
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			propagator, incomingCarrier := setupPropagatorTest()
 
-	ctx := propagator.Extract(context.Background(), incomingCarrier)
-	propagator.Inject(ctx, outgoingCarrier)
+			if tt.inBaggage != nil {
+				incomingCarrier.Set(
+					"baggage",
+					*tt.inBaggage,
+				)
+			}
+			if tt.inSentryTrace != nil {
+				incomingCarrier.Set(
+					"sentry-trace",
+					*tt.inSentryTrace,
+				)
+			}
+			outgoingCarrier := propagation.MapCarrier{}
 
-	assertMapCarrierEqual(t,
-		outgoingCarrier,
-		propagation.MapCarrier{
-			"baggage": "othervendor=bla",
-		},
-	)
+			ctx := propagator.Extract(context.Background(), incomingCarrier)
+			propagator.Inject(ctx, outgoingCarrier)
+
+			assertMapCarrierEqual(t,
+				outgoingCarrier,
+				incomingCarrier,
+			)
+		})
+	}
 }