diff --git a/CHANGELOG.md b/CHANGELOG.md index 4db1d233028..79334e486df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- OpenTracing Bridge: allow any OpenTracing propagation carrier. (#2141) + ### Fixed - Delegated instruments are unwrapped before delegating Callbacks. (#2784) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 3e931b00e29..a101e711dbf 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -620,6 +620,45 @@ func (s fakeSpan) SpanContext() trace.SpanContext { return s.sc } +// textMapAdapter adapts an OpenTracing TextMapReader or TextMapWriter to an OpenTelemetry propagation.TextMapCarrier. +type textMapAdapter struct { + r ot.TextMapReader + w ot.TextMapWriter +} + +func (a textMapAdapter) Get(key string) string { + if a.r == nil { + return "" // not implemented on writer + } + var ret string + _ = a.r.ForeachKey(func(k, v string) error { + if key == k { + ret = v + } + return nil + }) + return ret +} + +func (a textMapAdapter) Set(key string, value string) { + if a.w == nil { + return // not implemented on reader + } + a.w.Set(key, value) +} + +func (a textMapAdapter) Keys() []string { + if a.r == nil { + return nil // not implemented on writer + } + var ret []string + _ = a.r.ForeachKey(func(k, v string) error { + ret = append(ret, k) + return nil + }) + return ret +} + // Inject is a part of the implementation of the OpenTracing Tracer // interface. // @@ -635,18 +674,22 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int if builtinFormat, ok := format.(ot.BuiltinFormat); !ok || builtinFormat != ot.HTTPHeaders { return ot.ErrUnsupportedFormat } - hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) + // If carrier implements the required interface directly, use that + tmCarrier, ok := carrier.(propagation.TextMapCarrier) if !ok { - return ot.ErrInvalidCarrier + tmWriter, ok := carrier.(ot.TextMapWriter) // otherwise see if we can wrap it + if !ok { + return ot.ErrInvalidCarrier + } + tmCarrier = textMapAdapter{w: tmWriter} } - header := http.Header(hhcarrier) fs := fakeSpan{ Span: noop.Span, sc: bridgeSC.otelSpanContext, } ctx := trace.ContextWithSpan(context.Background(), fs) ctx = baggage.ContextWithBaggage(ctx, bridgeSC.bag) - t.getPropagator().Inject(ctx, propagation.HeaderCarrier(header)) + t.getPropagator().Inject(ctx, tmCarrier) return nil } @@ -658,12 +701,22 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span if builtinFormat, ok := format.(ot.BuiltinFormat); !ok || builtinFormat != ot.HTTPHeaders { return nil, ot.ErrUnsupportedFormat } - hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) - if !ok { + var tmCarrier propagation.TextMapCarrier + switch typedCarrier := carrier.(type) { + case propagation.TextMapCarrier: + // If carrier implements the required interface directly, use that. + tmCarrier = typedCarrier + case ot.HTTPHeadersCarrier: + // HTTPHeadersCarrier requires special wrapping, because Set() title-cases the key. + header := http.Header(typedCarrier) + tmCarrier = propagation.HeaderCarrier(header) + case ot.TextMapReader: + // We can wrap anything corresponding to TextMapReader. + tmCarrier = textMapAdapter{r: typedCarrier} + default: return nil, ot.ErrInvalidCarrier } - header := http.Header(hhcarrier) - ctx := t.getPropagator().Extract(context.Background(), propagation.HeaderCarrier(header)) + ctx := t.getPropagator().Extract(context.Background(), tmCarrier) baggage := baggage.FromContext(ctx) bridgeSC := &bridgeSpanContext{ bag: baggage, diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go new file mode 100644 index 00000000000..97390162ad1 --- /dev/null +++ b/bridge/opentracing/bridge_test.go @@ -0,0 +1,94 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package opentracing + +import ( + "reflect" + "sort" + "testing" + + ot "github.com/opentracing/opentracing-go" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/bridge/opentracing/internal" + "go.opentelemetry.io/otel/propagation" +) + +func TestTextMapAdapterKeys(t *testing.T) { + carrier := ot.TextMapCarrier{ + "foo": "bar", + "baz": "qux", + } + + keys := textMapAdapter{r: carrier}.Keys() + sort.Strings(keys) + expected := []string{"baz", "foo"} + if !reflect.DeepEqual(expected, keys) { + t.Errorf("Keys do not match: %#v, %#v", expected, keys) + } + // Check what happens if we read from a write-capable adaptor. + keys = textMapAdapter{w: carrier}.Keys() + if keys != nil { + t.Errorf("Keys should be nil: %#v", keys) + } +} + +func TestMapCarrier(t *testing.T) { + carrier := propagation.MapCarrier{} + testBridgeWithCarrier(t, carrier) +} + +func TestHeaderCarrier(t *testing.T) { + carrier := propagation.HeaderCarrier{} + testBridgeWithCarrier(t, carrier) +} + +func TestOTHTTPHeadersCarrier(t *testing.T) { + carrier := ot.HTTPHeadersCarrier{} + testBridgeWithCarrier(t, carrier) +} + +func TestOTTextMapCarrier(t *testing.T) { + carrier := ot.TextMapCarrier{} + testBridgeWithCarrier(t, carrier) +} + +func testBridgeWithCarrier(t *testing.T, carrier interface{}) { + mockOtelTracer := internal.NewMockTracer() + bridgeTracer, _ := NewTracerPair(mockOtelTracer) + otel.SetTextMapPropagator(propagation.TraceContext{}) + + span := bridgeTracer.StartSpan("testSpan1") + defer span.Finish() + + err := bridgeTracer.Inject(span.Context(), ot.HTTPHeaders, carrier) + if err != nil { + t.Errorf("Inject error: %s", err) + } + + extractedSpanContext, err := bridgeTracer.Extract(ot.HTTPHeaders, carrier) + if err != nil { + t.Errorf("Extract error: %s", err) + } + + // Make a copy of the SpanContext with remote set to true + originalSpanContext := span.Context().(*bridgeSpanContext) + copySpanContext := *originalSpanContext + copySpanContext.otelSpanContext = copySpanContext.otelSpanContext.WithRemote(true) + // Now the copy should be equal to the original we passed in. + if !reflect.DeepEqual(©SpanContext, extractedSpanContext) { + t.Errorf("Extracted span context does not match: %#v, %#v", span.Context(), extractedSpanContext) + } +}