Skip to content

Commit

Permalink
Do not coerce baggage keys to lower case (jaegertracing#196)
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro authored Sep 4, 2017
1 parent 9174d03 commit 59da631
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 17 deletions.
14 changes: 8 additions & 6 deletions propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,22 +165,24 @@ func TestBaggagePropagationHTTP(t *testing.T) {
tracer, closer := NewTracer("DOOP", NewConstSampler(true), NewNullReporter())
defer closer.Close()

sp1 := tracer.StartSpan("s1")
sp1 := tracer.StartSpan("s1").(*Span)
sp1.SetBaggageItem("Some_Key", "12345")
assert.Equal(t, "12345", sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.(*Span).context.baggage)
sp1.SetBaggageItem("Some_Key", "98:765") // colon : should be escaped as %3A
assert.Equal(t, "98:765", sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.(*Span).context.baggage)
assert.Equal(t, "12345", sp1.BaggageItem("Some_Key"), "baggage: %+v", sp1.context.baggage)
assert.Empty(t, sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.context.baggage)
sp1.SetBaggageItem("Some_Key", "98:765")
assert.Equal(t, "98:765", sp1.BaggageItem("Some_Key"), "baggage: %+v", sp1.context.baggage)
assert.Empty(t, sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.context.baggage)

h := http.Header{}
h.Add("header1", "value1") // make sure this does not get unmarshalled as baggage
err := tracer.Inject(sp1.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(h))
require.NoError(t, err)
// check that colon : was encoded as %3A
assert.Equal(t, "98%3A765", h.Get(TraceBaggageHeaderPrefix+"some-key"))
assert.Equal(t, "98%3A765", h.Get(TraceBaggageHeaderPrefix+"Some_Key"), "headers: %+v", h)

sp2, err := tracer.Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(h))
require.NoError(t, err)
assert.Equal(t, map[string]string{"some-key": "98:765"}, sp2.(SpanContext).baggage)
assert.Equal(t, map[string]string{"some_key": "98:765"}, sp2.(SpanContext).baggage)
}

func TestJaegerBaggageHeader(t *testing.T) {
Expand Down
10 changes: 0 additions & 10 deletions span.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package jaeger

import (
"strings"
"sync"
"time"

Expand Down Expand Up @@ -167,7 +166,6 @@ func (s *Span) appendLog(lr opentracing.LogRecord) {

// SetBaggageItem implements SetBaggageItem() of opentracing.SpanContext
func (s *Span) SetBaggageItem(key, value string) opentracing.Span {
key = normalizeBaggageKey(key)
s.Lock()
defer s.Unlock()
s.tracer.setBaggage(s, key, value)
Expand All @@ -176,7 +174,6 @@ func (s *Span) SetBaggageItem(key, value string) opentracing.Span {

// BaggageItem implements BaggageItem() of opentracing.SpanContext
func (s *Span) BaggageItem(key string) string {
key = normalizeBaggageKey(key)
s.RLock()
defer s.RUnlock()
return s.context.baggage[key]
Expand Down Expand Up @@ -249,10 +246,3 @@ func setSamplingPriority(s *Span, value interface{}) bool {
}
return false
}

// Converts end-user baggage key into internal representation.
// Used for both read and write access to baggage items.
func normalizeBaggageKey(key string) string {
// TODO(yurishkuro) normalizeBaggageKey: cache the results in some bounded LRU cache
return strings.Replace(strings.ToLower(key), "_", "-", -1)
}
2 changes: 1 addition & 1 deletion span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestBaggageIterator(t *testing.T) {
sp1 := tracer.StartSpan("s1").(*Span)
sp1.SetBaggageItem("Some_Key", "12345")
sp1.SetBaggageItem("Some-other-key", "42")
expectedBaggage := map[string]string{"some-key": "12345", "some-other-key": "42"}
expectedBaggage := map[string]string{"Some_Key": "12345", "Some-other-key": "42"}
assertBaggage(t, sp1, expectedBaggage)
assertBaggageRecords(t, sp1, expectedBaggage)

Expand Down

0 comments on commit 59da631

Please sign in to comment.