Skip to content

Commit

Permalink
bridge/opentracing: fix baggage item key is canonicalized (#4776)
Browse files Browse the repository at this point in the history
Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
3 people authored Dec 28, 2023
1 parent 08b856f commit 27f70a3
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 2 additions & 5 deletions bridge/opentracing/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package opentracing // import "go.opentelemetry.io/otel/bridge/opentracing"
import (
"context"
"fmt"
"net/http"
"strings"
"sync"

Expand Down Expand Up @@ -74,17 +73,15 @@ 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
}
c.bag, _ = c.bag.SetMember(m)
}

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 {
Expand Down
84 changes: 84 additions & 0 deletions bridge/opentracing/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
}
4 changes: 2 additions & 2 deletions bridge/opentracing/mix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func newBaggageItemsPreservationTest() *baggageItemsPreservationTest {
value: "two",
},
{
key: "Third",
key: "third",
value: "three",
},
},
Expand Down Expand Up @@ -427,7 +427,7 @@ func newBaggageInteroperationTest() *baggageInteroperationTest {
value: "two",
},
{
key: "Third",
key: "third",
value: "three",
},
},
Expand Down

0 comments on commit 27f70a3

Please sign in to comment.