From 3bce9c97f800c12a791e78a1611991818f9b4db6 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 17 Feb 2021 11:04:49 -0500 Subject: [PATCH] Add Keys() method to propagation.TextMapCarrier (#1544) ...and propagation.HeaderCarrier to adapt http.Header to this interface. Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + bridge/opentracing/bridge.go | 4 +-- oteltest/text_map_propagator.go | 12 +++++++++ oteltest/text_map_propagator_test.go | 9 +++++++ propagation/baggage_test.go | 6 ++--- propagation/propagation.go | 29 ++++++++++++++++++++- propagation/propagation_test.go | 2 ++ propagation/propagators_test.go | 4 +++ propagation/trace_context_benchmark_test.go | 4 +-- propagation/trace_context_test.go | 10 +++---- 10 files changed, 68 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f858615c20b..e0b56d1c05d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Added `resource.Default()` for use with meter and tracer providers. (#1507) +- Added `Keys()` method to `propagation.TextMapCarrier` and `propagation.HeaderCarrier` to adapt `http.Header` to this interface. (#1544) ## [0.17.0] - 2020-02-12 diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 04005f52139..7dd2d688c2c 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -637,7 +637,7 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int } ctx := trace.ContextWithSpan(context.Background(), fs) ctx = baggage.ContextWithMap(ctx, bridgeSC.baggageItems) - t.getPropagator().Inject(ctx, header) + t.getPropagator().Inject(ctx, propagation.HeaderCarrier(header)) return nil } @@ -654,7 +654,7 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span return nil, ot.ErrInvalidCarrier } header := http.Header(hhcarrier) - ctx := t.getPropagator().Extract(context.Background(), header) + ctx := t.getPropagator().Extract(context.Background(), propagation.HeaderCarrier(header)) baggage := baggage.MapFromContext(ctx) otelSC, _, _ := otelparent.GetSpanContextAndLinks(ctx, false) bridgeSC := &bridgeSpanContext{ diff --git a/oteltest/text_map_propagator.go b/oteltest/text_map_propagator.go index 6a765d0d3ea..4803ad18f73 100644 --- a/oteltest/text_map_propagator.go +++ b/oteltest/text_map_propagator.go @@ -46,6 +46,18 @@ func NewTextMapCarrier(data map[string]string) *TextMapCarrier { return &TextMapCarrier{data: copied} } +// Keys returns the keys for which this carrier has a value. +func (c *TextMapCarrier) Keys() []string { + c.mtx.Lock() + defer c.mtx.Unlock() + + result := make([]string, 0, len(c.data)) + for k := range c.data { + result = append(result, k) + } + return result +} + // Get returns the value associated with the passed key. func (c *TextMapCarrier) Get(key string) string { c.mtx.Lock() diff --git a/oteltest/text_map_propagator_test.go b/oteltest/text_map_propagator_test.go index 48debc55f00..02c7a5437fa 100644 --- a/oteltest/text_map_propagator_test.go +++ b/oteltest/text_map_propagator_test.go @@ -16,6 +16,7 @@ package oteltest import ( "context" + "reflect" "testing" ) @@ -23,6 +24,14 @@ var ( key, value = "test", "true" ) +func TestTextMapCarrierKeys(t *testing.T) { + tmc := NewTextMapCarrier(map[string]string{key: value}) + expected, actual := []string{key}, tmc.Keys() + if !reflect.DeepEqual(actual, expected) { + t.Errorf("expected tmc.Keys() to be %v but it was %v", expected, actual) + } +} + func TestTextMapCarrierGet(t *testing.T) { tmc := NewTextMapCarrier(map[string]string{key: value}) tmc.GotN(t, 0) diff --git a/propagation/baggage_test.go b/propagation/baggage_test.go index abd4792944c..94685068112 100644 --- a/propagation/baggage_test.go +++ b/propagation/baggage_test.go @@ -90,7 +90,7 @@ func TestExtractValidBaggageFromHTTPReq(t *testing.T) { req.Header.Set("baggage", tt.header) ctx := context.Background() - ctx = prop.Extract(ctx, req.Header) + ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header)) gotBaggage := baggage.MapFromContext(ctx) wantBaggage := baggage.NewMap(baggage.MapUpdate{MultiKV: tt.wantKVs}) if gotBaggage.Len() != wantBaggage.Len() { @@ -152,7 +152,7 @@ func TestExtractInvalidDistributedContextFromHTTPReq(t *testing.T) { ctx := baggage.NewContext(context.Background(), tt.hasKVs...) wantBaggage := baggage.MapFromContext(ctx) - ctx = prop.Extract(ctx, req.Header) + ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header)) gotBaggage := baggage.MapFromContext(ctx) if gotBaggage.Len() != wantBaggage.Len() { t.Errorf( @@ -218,7 +218,7 @@ func TestInjectBaggageToHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := baggage.ContextWithMap(context.Background(), baggage.NewMap(baggage.MapUpdate{MultiKV: tt.kvs})) - propagator.Inject(ctx, req.Header) + propagator.Inject(ctx, propagation.HeaderCarrier(req.Header)) gotHeader := req.Header.Get("baggage") wantedLen := len(strings.Join(tt.wantInHeader, ",")) diff --git a/propagation/propagation.go b/propagation/propagation.go index c867f86b8f9..9cfeb347a37 100644 --- a/propagation/propagation.go +++ b/propagation/propagation.go @@ -14,7 +14,10 @@ package propagation // import "go.opentelemetry.io/otel/propagation" -import "context" +import ( + "context" + "net/http" +) // TextMapCarrier is the storage medium used by a TextMapPropagator. type TextMapCarrier interface { @@ -22,6 +25,30 @@ type TextMapCarrier interface { Get(key string) string // Set stores the key-value pair. Set(key string, value string) + // Keys lists the keys stored in this carrier. + Keys() []string +} + +// HeaderCarrier adapts http.Header to satisfy the TextMapCarrier interface. +type HeaderCarrier http.Header + +// Get returns the value associated with the passed key. +func (hc HeaderCarrier) Get(key string) string { + return http.Header(hc).Get(key) +} + +// Set stores the key-value pair. +func (hc HeaderCarrier) Set(key string, value string) { + http.Header(hc).Set(key, value) +} + +// Keys lists the keys stored in this carrier. +func (hc HeaderCarrier) Keys() []string { + keys := make([]string, 0, len(hc)) + for k := range hc { + keys = append(keys, k) + } + return keys } // TextMapPropagator propagates cross-cutting concerns as key-value text diff --git a/propagation/propagation_test.go b/propagation/propagation_test.go index 477938dec3b..69d7502f09b 100644 --- a/propagation/propagation_test.go +++ b/propagation/propagation_test.go @@ -30,6 +30,8 @@ var ( type carrier []string +func (c *carrier) Keys() []string { return nil } + func (c *carrier) Get(string) string { return "" } func (c *carrier) Set(setter, _ string) { diff --git a/propagation/propagators_test.go b/propagation/propagators_test.go index df392b51495..4cf72e67955 100644 --- a/propagation/propagators_test.go +++ b/propagation/propagators_test.go @@ -79,6 +79,10 @@ type nilCarrier struct{} var _ propagation.TextMapCarrier = nilCarrier{} +func (nilCarrier) Keys() []string { + return nil +} + func (nilCarrier) Get(key string) string { return "" } diff --git a/propagation/trace_context_benchmark_test.go b/propagation/trace_context_benchmark_test.go index 0f2d5fcebb2..9bb66f2fb3f 100644 --- a/propagation/trace_context_benchmark_test.go +++ b/propagation/trace_context_benchmark_test.go @@ -31,7 +31,7 @@ func BenchmarkInject(b *testing.B) { req, _ := http.NewRequest("GET", "http://example.com", nil) b.ResetTimer() for i := 0; i < b.N; i++ { - t.Inject(ctx, req.Header) + t.Inject(ctx, propagation.HeaderCarrier(req.Header)) } }) } @@ -66,7 +66,7 @@ func BenchmarkExtract(b *testing.B) { ctx := context.Background() b.ResetTimer() for i := 0; i < b.N; i++ { - propagator.Extract(ctx, req.Header) + propagator.Extract(ctx, propagation.HeaderCarrier(req.Header)) } }) } diff --git a/propagation/trace_context_test.go b/propagation/trace_context_test.go index 54260f0fd85..092bf3d2ba7 100644 --- a/propagation/trace_context_test.go +++ b/propagation/trace_context_test.go @@ -112,7 +112,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { req.Header.Set("traceparent", tt.header) ctx := context.Background() - ctx = prop.Extract(ctx, req.Header) + ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header)) gotSc := trace.RemoteSpanContextFromContext(ctx) if diff := cmp.Diff(gotSc, tt.wantSc, cmp.AllowUnexported(trace.TraceState{})); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) @@ -200,7 +200,7 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { req.Header.Set("traceparent", tt.header) ctx := context.Background() - ctx = prop.Extract(ctx, req.Header) + ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header)) gotSc := trace.RemoteSpanContextFromContext(ctx) if diff := cmp.Diff(gotSc, wantSc, cmp.AllowUnexported(trace.TraceState{})); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) @@ -257,7 +257,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { ctx = trace.ContextWithRemoteSpanContext(ctx, tt.sc) ctx, _ = mockTracer.Start(ctx, "inject") } - prop.Inject(ctx, req.Header) + prop.Inject(ctx, propagation.HeaderCarrier(req.Header)) gotHeader := req.Header.Get("traceparent") if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { @@ -337,7 +337,7 @@ func TestTraceStatePropagation(t *testing.T) { inReq.Header.Add(hk, hv) } - ctx := prop.Extract(context.Background(), inReq.Header) + ctx := prop.Extract(context.Background(), propagation.HeaderCarrier(inReq.Header)) if diff := cmp.Diff( trace.RemoteSpanContextFromContext(ctx), tt.wantSc, @@ -351,7 +351,7 @@ func TestTraceStatePropagation(t *testing.T) { mockTracer := oteltest.DefaultTracer() ctx, _ = mockTracer.Start(ctx, "inject") outReq, _ := http.NewRequest(http.MethodGet, "http://www.example.com", nil) - prop.Inject(ctx, outReq.Header) + prop.Inject(ctx, propagation.HeaderCarrier(outReq.Header)) if diff := cmp.Diff(outReq.Header.Get(stateHeader), tt.headers[stateHeader]); diff != "" { t.Errorf("Propagated tracestate: -got +want %s", diff)