From 27f70a335008ed7cdb398013eedf6812cc1c44e0 Mon Sep 17 00:00:00 2001 From: CZ Date: Fri, 29 Dec 2023 06:46:17 +1300 Subject: [PATCH] bridge/opentracing: fix baggage item key is canonicalized (#4776) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert PajÄ…k Co-authored-by: Yuri Shkuro --- CHANGELOG.md | 1 + bridge/opentracing/bridge.go | 7 +-- bridge/opentracing/bridge_test.go | 84 +++++++++++++++++++++++++++++++ bridge/opentracing/mix_test.go | 4 +- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b94281d2ff5..717a990bc7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix `Parse` in `go.opentelemetry.io/otel/baggage` to validate member value before percent-decoding. (#4755) - Fix whitespace encoding of `Member.String` in `go.opentelemetry.io/otel/baggage`. (#4756) +- Fix baggage item key so that it is not canonicalized in `go.opentelemetry.io/otel/bridge/opentracing`. (#4776) ## [1.21.0/0.44.0] 2023-11-16 diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index cc3d7d19c7f..f61dc9eee00 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -17,7 +17,6 @@ package opentracing // import "go.opentelemetry.io/otel/bridge/opentracing" import ( "context" "fmt" - "net/http" "strings" "sync" @@ -74,8 +73,7 @@ func (c *bridgeSpanContext) ForeachBaggageItem(handler func(k, v string) bool) { } func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) { - crk := http.CanonicalHeaderKey(restrictedKey) - m, err := baggage.NewMember(crk, value) + m, err := baggage.NewMember(restrictedKey, value) if err != nil { return } @@ -83,8 +81,7 @@ func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) { } func (c *bridgeSpanContext) baggageItem(restrictedKey string) baggage.Member { - crk := http.CanonicalHeaderKey(restrictedKey) - return c.bag.Member(crk) + return c.bag.Member(restrictedKey) } type bridgeSpan struct { diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index bbeb3f9fbfe..7a3e384424b 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -574,3 +574,87 @@ func TestBridgeSpanContextPromotedMethods(t *testing.T) { assert.True(t, spanContext.(spanContextProvider).HasTraceID()) }) } + +func TestBridgeCarrierBaggagePropagation(t *testing.T) { + carriers := []struct { + name string + carrier interface{} + format ot.BuiltinFormat + }{ + { + name: "TextMapCarrier", + carrier: ot.TextMapCarrier{}, + format: ot.TextMap, + }, + { + name: "HTTPHeadersCarrier", + carrier: ot.HTTPHeadersCarrier{}, + format: ot.HTTPHeaders, + }, + } + + testCases := []struct { + name string + baggageItems []bipBaggage + }{ + { + name: "single baggage item", + baggageItems: []bipBaggage{ + { + key: "foo", + value: "bar", + }, + }, + }, + { + name: "multiple baggage items", + baggageItems: []bipBaggage{ + { + key: "foo", + value: "bar", + }, + { + key: "foo2", + value: "bar2", + }, + }, + }, + } + + for _, c := range carriers { + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s %s", c.name, tc.name), func(t *testing.T) { + mockOtelTracer := internal.NewMockTracer() + b, _ := NewTracerPair(mockOtelTracer) + b.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator( + propagation.TraceContext{}, + propagation.Baggage{}), // Required for baggage propagation. + ) + + // Set baggage items. + span := b.StartSpan("test") + for _, bi := range tc.baggageItems { + span.SetBaggageItem(bi.key, bi.value) + } + defer span.Finish() + + err := b.Inject(span.Context(), c.format, c.carrier) + assert.NoError(t, err) + + spanContext, err := b.Extract(c.format, c.carrier) + assert.NoError(t, err) + + // Check baggage items. + bsc, ok := spanContext.(*bridgeSpanContext) + assert.True(t, ok) + + var got []bipBaggage + for _, m := range bsc.bag.Members() { + got = append(got, bipBaggage{m.Key(), m.Value()}) + } + + assert.ElementsMatch(t, tc.baggageItems, got) + }) + } + } +} diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index 0cd5a667ca5..b57f37e8ea7 100644 --- a/bridge/opentracing/mix_test.go +++ b/bridge/opentracing/mix_test.go @@ -334,7 +334,7 @@ func newBaggageItemsPreservationTest() *baggageItemsPreservationTest { value: "two", }, { - key: "Third", + key: "third", value: "three", }, }, @@ -427,7 +427,7 @@ func newBaggageInteroperationTest() *baggageInteroperationTest { value: "two", }, { - key: "Third", + key: "third", value: "three", }, },