From 59da631654802751f545c186354f41d765c30f97 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 4 Sep 2017 12:01:35 -0400 Subject: [PATCH] Do not coerce baggage keys to lower case (#196) --- propagation_test.go | 14 ++++++++------ span.go | 10 ---------- span_test.go | 2 +- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/propagation_test.go b/propagation_test.go index 74d30c08..f8365b83 100644 --- a/propagation_test.go +++ b/propagation_test.go @@ -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) { diff --git a/span.go b/span.go index 26dcbaf3..04b5d6b2 100644 --- a/span.go +++ b/span.go @@ -21,7 +21,6 @@ package jaeger import ( - "strings" "sync" "time" @@ -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) @@ -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] @@ -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) -} diff --git a/span_test.go b/span_test.go index a0e732e1..aab13c7a 100644 --- a/span_test.go +++ b/span_test.go @@ -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)