From 0c3c0dab82167a1283ee7ef7216a61d1c943626d Mon Sep 17 00:00:00 2001 From: ZHENK Date: Fri, 22 Dec 2023 03:53:36 +1300 Subject: [PATCH 1/9] bridge/opentracing: add example test to propagate baggage item with carrier --- bridge/opentracing/bridge_test.go | 86 +++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index bbeb3f9fbfe..1359f930932 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -574,3 +574,89 @@ func TestBridgeSpanContextPromotedMethods(t *testing.T) { assert.True(t, spanContext.(spanContextProvider).HasTraceID()) }) } + +func TestBridgeCarrierBaggagePropagaton(t *testing.T) { + testCases := []struct { + name string + format ot.BuiltinFormat + carrier interface{} + baggageItems []bipBaggage + wantBaggageItems []bipBaggage + }{ + { + name: "HTTPHeadersCarrier", + format: ot.HTTPHeaders, + carrier: ot.HTTPHeadersCarrier(http.Header{}), + baggageItems: []bipBaggage{ + { + key: "foo", + value: "bar", + }, + }, + wantBaggageItems: []bipBaggage{ + { + key: "Foo", + value: "bar", + }, + }, + }, + { + name: "TextMapCarrier", + format: ot.TextMap, + carrier: ot.TextMapCarrier(map[string]string{}), + baggageItems: []bipBaggage{ + { + key: "foo", + value: "bar", + }, + { + key: "foo2", + value: "bar2", + }, + }, + wantBaggageItems: []bipBaggage{ + { + key: "Foo", + value: "bar", + }, + { + key: "Foo2", + value: "bar2", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(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(), tc.format, tc.carrier) + assert.NoError(t, err) + + spanContext, err := b.Extract(tc.format, tc.carrier) + assert.NoError(t, err) + + // Check baggage items. + bsc, ok := spanContext.(*bridgeSpanContext) + assert.True(t, ok) + assert.Equal(t, len(tc.wantBaggageItems), bsc.bag.Len()) + for i, m := range bsc.bag.Members() { + assert.Equal(t, tc.wantBaggageItems[i].key, m.Key()) + assert.Equal(t, tc.wantBaggageItems[i].value, m.Value()) + } + }) + } +} From 285ad6d0af69ff8c3e32a4b641404ba2fbe9dcdb Mon Sep 17 00:00:00 2001 From: CZ Date: Fri, 22 Dec 2023 04:04:32 +1300 Subject: [PATCH 2/9] Update bridge/opentracing/bridge_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- bridge/opentracing/bridge_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index 1359f930932..e8c1c7bcbc1 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -575,7 +575,7 @@ func TestBridgeSpanContextPromotedMethods(t *testing.T) { }) } -func TestBridgeCarrierBaggagePropagaton(t *testing.T) { +func TestBridgeCarrierBaggagePropagation(t *testing.T) { testCases := []struct { name string format ot.BuiltinFormat From a112d3f67926fe27a2948a8b15380d5c5ccdcbe1 Mon Sep 17 00:00:00 2001 From: ZHENK Date: Fri, 22 Dec 2023 13:33:33 +1300 Subject: [PATCH 3/9] Add pre-defined loop for different carriers + fix order --- bridge/opentracing/bridge_test.go | 84 ++++++++++++++++++------------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index e8c1c7bcbc1..a2e3e98fef9 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -576,17 +576,30 @@ func TestBridgeSpanContextPromotedMethods(t *testing.T) { } 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 - format ot.BuiltinFormat - carrier interface{} baggageItems []bipBaggage wantBaggageItems []bipBaggage }{ { - name: "HTTPHeadersCarrier", - format: ot.HTTPHeaders, - carrier: ot.HTTPHeadersCarrier(http.Header{}), + name: "single baggage item", baggageItems: []bipBaggage{ { key: "foo", @@ -601,9 +614,7 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { }, }, { - name: "TextMapCarrier", - format: ot.TextMap, - carrier: ot.TextMapCarrier(map[string]string{}), + name: "multiple baggage items", baggageItems: []bipBaggage{ { key: "foo", @@ -627,36 +638,37 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { }, } - for _, tc := range testCases { - t.Run(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() + 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(), tc.format, tc.carrier) - assert.NoError(t, err) + err := b.Inject(span.Context(), c.format, c.carrier) + assert.NoError(t, err) - spanContext, err := b.Extract(tc.format, tc.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) - assert.Equal(t, len(tc.wantBaggageItems), bsc.bag.Len()) - for i, m := range bsc.bag.Members() { - assert.Equal(t, tc.wantBaggageItems[i].key, m.Key()) - assert.Equal(t, tc.wantBaggageItems[i].value, m.Value()) - } - }) + // Check baggage items. + bsc, ok := spanContext.(*bridgeSpanContext) + assert.True(t, ok) + assert.Equal(t, len(tc.wantBaggageItems), bsc.bag.Len()) + for _, bi := range tc.wantBaggageItems { + assert.Equal(t, bi.value, bsc.bag.Member(bi.key).Value()) + } + }) + } } } From 25a2930e891aded915787a1d2b3317f45e52375c Mon Sep 17 00:00:00 2001 From: ZHENK Date: Thu, 28 Dec 2023 01:23:24 +1300 Subject: [PATCH 4/9] Fix propagated baggage item key canonicalized --- bridge/opentracing/bridge.go | 7 ++----- bridge/opentracing/bridge_test.go | 6 +++--- bridge/opentracing/mix_test.go | 4 ++-- 3 files changed, 7 insertions(+), 10 deletions(-) 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 a2e3e98fef9..77078e54e8d 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -608,7 +608,7 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { }, wantBaggageItems: []bipBaggage{ { - key: "Foo", + key: "foo", value: "bar", }, }, @@ -627,11 +627,11 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { }, wantBaggageItems: []bipBaggage{ { - key: "Foo", + key: "foo", value: "bar", }, { - key: "Foo2", + key: "foo2", value: "bar2", }, }, 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", }, }, From d357c8e12e852b1fb66ff4d296fdac8019f02f49 Mon Sep 17 00:00:00 2001 From: ZHENK Date: Thu, 28 Dec 2023 01:27:57 +1300 Subject: [PATCH 5/9] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c283b9cbebb..fc26d8edadd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,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 is canonicalized in `go.opentelemetry.io/otel/bridge/opentracing`. (#4799) ## [1.21.0/0.44.0] 2023-11-16 From e689a1f23aa805b32b87f16f801f757b8910b070 Mon Sep 17 00:00:00 2001 From: CZ Date: Thu, 28 Dec 2023 16:51:02 +1300 Subject: [PATCH 6/9] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc26d8edadd..ba156485ca2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,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 is canonicalized in `go.opentelemetry.io/otel/bridge/opentracing`. (#4799) +- Fix baggage item key so that is not canonicalized in `go.opentelemetry.io/otel/bridge/opentracing`. (#4776) ## [1.21.0/0.44.0] 2023-11-16 From e22a135395fdbfcfbf410d7caab243891d962eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 28 Dec 2023 18:19:22 +0100 Subject: [PATCH 7/9] Update CHANGELOG.md Co-authored-by: Yuri Shkuro --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba156485ca2..92362b7ae02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,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 is not canonicalized in `go.opentelemetry.io/otel/bridge/opentracing`. (#4776) +- 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 From b4c11fea9b9e2e00df2c4e80dfd7c3a3c89021f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 28 Dec 2023 18:38:04 +0100 Subject: [PATCH 8/9] Refactor TestBridgeCarrierBaggagePropagation --- bridge/opentracing/bridge_test.go | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index 77078e54e8d..ef5a36dd79f 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -594,9 +594,8 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { } testCases := []struct { - name string - baggageItems []bipBaggage - wantBaggageItems []bipBaggage + name string + baggageItems []bipBaggage }{ { name: "single baggage item", @@ -606,12 +605,6 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { value: "bar", }, }, - wantBaggageItems: []bipBaggage{ - { - key: "foo", - value: "bar", - }, - }, }, { name: "multiple baggage items", @@ -625,16 +618,6 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { value: "bar2", }, }, - wantBaggageItems: []bipBaggage{ - { - key: "foo", - value: "bar", - }, - { - key: "foo2", - value: "bar2", - }, - }, }, } @@ -664,10 +647,13 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { // Check baggage items. bsc, ok := spanContext.(*bridgeSpanContext) assert.True(t, ok) - assert.Equal(t, len(tc.wantBaggageItems), bsc.bag.Len()) - for _, bi := range tc.wantBaggageItems { - assert.Equal(t, bi.value, bsc.bag.Member(bi.key).Value()) + + var got []bipBaggage + for _, m := range bsc.bag.Members() { + got = append(got, bipBaggage{m.Key(), m.Value()}) } + + assert.Equal(t, tc.baggageItems, got) }) } } From 8776d8b5c383724a556b6015e820bf5a90393ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 28 Dec 2023 18:40:23 +0100 Subject: [PATCH 9/9] Stablize TestBridgeCarrierBaggagePropagation --- bridge/opentracing/bridge_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index ef5a36dd79f..7a3e384424b 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -653,7 +653,7 @@ func TestBridgeCarrierBaggagePropagation(t *testing.T) { got = append(got, bipBaggage{m.Key(), m.Value()}) } - assert.Equal(t, tc.baggageItems, got) + assert.ElementsMatch(t, tc.baggageItems, got) }) } }