diff --git a/CHANGELOG.md b/CHANGELOG.md index 713eed7fd90..d94994675e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add support for `opentracing.TextMap` format in the `Inject` and `Extract` methods +of the `"go.opentelemetry.io/otel/bridge/opentracing".BridgeTracer` type. (#2911) + ### Changed - The `crosslink` make target has been updated to use the `go.opentelemetry.io/build-tools/crosslink` package. (#2886) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 947321debc0..2927e86535a 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -634,7 +634,7 @@ func (s fakeSpan) SpanContext() trace.SpanContext { // Inject is a part of the implementation of the OpenTracing Tracer // interface. // -// Currently only the HTTPHeaders format is supported. +// Currently only the HTTPHeaders and TextMap formats are supported. func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier interface{}) error { bridgeSC, ok := sm.(*bridgeSpanContext) if !ok { @@ -643,38 +643,75 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int if !bridgeSC.otelSpanContext.IsValid() { return ot.ErrInvalidSpanContext } - if builtinFormat, ok := format.(ot.BuiltinFormat); !ok || builtinFormat != ot.HTTPHeaders { + + builtinFormat, ok := format.(ot.BuiltinFormat) + if !ok { return ot.ErrUnsupportedFormat } - hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) - if !ok { - return ot.ErrInvalidCarrier + + var textCarrier propagation.TextMapCarrier + + switch builtinFormat { + case ot.HTTPHeaders: + hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) + if !ok { + return ot.ErrInvalidCarrier + } + + textCarrier = propagation.HeaderCarrier(hhcarrier) + case ot.TextMap: + if textCarrier, ok = carrier.(propagation.TextMapCarrier); !ok { + var err error + if textCarrier, err = newTextMapWrapperForInject(carrier); err != nil { + return err + } + } + default: + return ot.ErrUnsupportedFormat } - header := http.Header(hhcarrier) + fs := fakeSpan{ Span: noopSpan, 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, textCarrier) return nil } // Extract is a part of the implementation of the OpenTracing Tracer // interface. // -// Currently only the HTTPHeaders format is supported. +// Currently only the HTTPHeaders and TextMap formats are supported. func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.SpanContext, error) { - if builtinFormat, ok := format.(ot.BuiltinFormat); !ok || builtinFormat != ot.HTTPHeaders { + builtinFormat, ok := format.(ot.BuiltinFormat) + if !ok { return nil, ot.ErrUnsupportedFormat } - hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) - if !ok { - return nil, ot.ErrInvalidCarrier + + var textCarrier propagation.TextMapCarrier + + switch builtinFormat { + case ot.HTTPHeaders: + hhcarrier, ok := carrier.(ot.HTTPHeadersCarrier) + if !ok { + return nil, ot.ErrInvalidCarrier + } + + textCarrier = propagation.HeaderCarrier(hhcarrier) + case ot.TextMap: + if textCarrier, ok = carrier.(propagation.TextMapCarrier); !ok { + var err error + if textCarrier, err = newTextMapWrapperForExtract(carrier); err != nil { + return nil, err + } + } + default: + return nil, ot.ErrUnsupportedFormat } - header := http.Header(hhcarrier) - ctx := t.getPropagator().Extract(context.Background(), propagation.HeaderCarrier(header)) + + ctx := t.getPropagator().Extract(context.Background(), textCarrier) bag := baggage.FromContext(ctx) bridgeSC := &bridgeSpanContext{ bag: bag, @@ -692,3 +729,105 @@ func (t *BridgeTracer) getPropagator() propagation.TextMapPropagator { } return otel.GetTextMapPropagator() } + +// textMapWrapper Provides operating.TextMapWriter and operating.TextMapReader to +// propagation.TextMapCarrier compatibility. +// Usually, Inject method will only use the write-related interface. +// Extract method will only use the reade-related interface. +// To avoid panic, +// when the carrier implements only one of the interfaces, +// it provides a default implementation of the other interface (textMapWriter and textMapReader). +type textMapWrapper struct { + ot.TextMapWriter + ot.TextMapReader + readerMap map[string]string +} + +func (t *textMapWrapper) Get(key string) string { + if t.readerMap == nil { + t.loadMap() + } + + return t.readerMap[key] +} + +func (t *textMapWrapper) Set(key string, value string) { + t.TextMapWriter.Set(key, value) +} + +func (t *textMapWrapper) Keys() []string { + if t.readerMap == nil { + t.loadMap() + } + + str := make([]string, 0, len(t.readerMap)) + for key := range t.readerMap { + str = append(str, key) + } + + return str +} + +func (t *textMapWrapper) loadMap() { + t.readerMap = make(map[string]string) + + _ = t.ForeachKey(func(key, val string) error { + t.readerMap[key] = val + + return nil + }) +} + +func newTextMapWrapperForExtract(carrier interface{}) (*textMapWrapper, error) { + t := &textMapWrapper{} + + reader, ok := carrier.(ot.TextMapReader) + if !ok { + return nil, ot.ErrInvalidCarrier + } + + t.TextMapReader = reader + + writer, ok := carrier.(ot.TextMapWriter) + if ok { + t.TextMapWriter = writer + } else { + t.TextMapWriter = &textMapWriter{} + } + + return t, nil +} + +func newTextMapWrapperForInject(carrier interface{}) (*textMapWrapper, error) { + t := &textMapWrapper{} + + writer, ok := carrier.(ot.TextMapWriter) + if !ok { + return nil, ot.ErrInvalidCarrier + } + + t.TextMapWriter = writer + + reader, ok := carrier.(ot.TextMapReader) + if ok { + t.TextMapReader = reader + } else { + t.TextMapReader = &textMapReader{} + } + + return t, nil +} + +type textMapWriter struct { +} + +func (t *textMapWriter) Set(key string, value string) { + // maybe print a warning log. +} + +type textMapReader struct { +} + +func (t *textMapReader) ForeachKey(handler func(key, val string) error) error { + return nil // maybe print a warning log. +} diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go new file mode 100644 index 00000000000..eea3091f265 --- /dev/null +++ b/bridge/opentracing/bridge_test.go @@ -0,0 +1,362 @@ +// 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 ( + "context" + "errors" + "net/http" + "strings" + "testing" + + ot "github.com/opentracing/opentracing-go" + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/trace" +) + +type testOnlyTextMapReader struct { +} + +func newTestOnlyTextMapReader() *testOnlyTextMapReader { + return &testOnlyTextMapReader{} +} + +func (t *testOnlyTextMapReader) ForeachKey(handler func(key string, val string) error) error { + _ = handler("key1", "val1") + _ = handler("key2", "val2") + + return nil +} + +type testOnlyTextMapWriter struct { + m map[string]string +} + +func newTestOnlyTextMapWriter() *testOnlyTextMapWriter { + return &testOnlyTextMapWriter{m: map[string]string{}} +} + +func (t *testOnlyTextMapWriter) Set(key, val string) { + t.m[key] = val +} + +type testTextMapReaderAndWriter struct { + *testOnlyTextMapReader + *testOnlyTextMapWriter +} + +func newTestTextMapReaderAndWriter() *testTextMapReaderAndWriter { + return &testTextMapReaderAndWriter{ + testOnlyTextMapReader: newTestOnlyTextMapReader(), + testOnlyTextMapWriter: newTestOnlyTextMapWriter(), + } +} + +func TestTextMapWrapper_New(t *testing.T) { + _, err := newTextMapWrapperForExtract(newTestOnlyTextMapReader()) + assert.NoError(t, err) + + _, err = newTextMapWrapperForExtract(newTestOnlyTextMapWriter()) + assert.True(t, errors.Is(err, ot.ErrInvalidCarrier)) + + _, err = newTextMapWrapperForExtract(newTestTextMapReaderAndWriter()) + assert.NoError(t, err) + + _, err = newTextMapWrapperForInject(newTestOnlyTextMapWriter()) + assert.NoError(t, err) + + _, err = newTextMapWrapperForInject(newTestOnlyTextMapReader()) + assert.True(t, errors.Is(err, ot.ErrInvalidCarrier)) + + _, err = newTextMapWrapperForInject(newTestTextMapReaderAndWriter()) + assert.NoError(t, err) +} + +func TestTextMapWrapper_action(t *testing.T) { + testExtractFunc := func(carrier propagation.TextMapCarrier) { + str := carrier.Keys() + assert.Len(t, str, 2) + assert.Contains(t, str, "key1", "key2") + + assert.Equal(t, carrier.Get("key1"), "val1") + assert.Equal(t, carrier.Get("key2"), "val2") + } + + testInjectFunc := func(carrier propagation.TextMapCarrier) { + carrier.Set("key1", "val1") + carrier.Set("key2", "val2") + + wrap, ok := carrier.(*textMapWrapper) + assert.True(t, ok) + + writer, ok := wrap.TextMapWriter.(*testOnlyTextMapWriter) + if ok { + assert.Contains(t, writer.m, "key1", "key2", "val1", "val2") + return + } + + writer2, ok := wrap.TextMapWriter.(*testTextMapReaderAndWriter) + assert.True(t, ok) + assert.Contains(t, writer2.m, "key1", "key2", "val1", "val2") + } + + onlyWriter, err := newTextMapWrapperForExtract(newTestOnlyTextMapReader()) + assert.NoError(t, err) + testExtractFunc(onlyWriter) + + onlyReader, err := newTextMapWrapperForInject(&testOnlyTextMapWriter{m: map[string]string{}}) + assert.NoError(t, err) + testInjectFunc(onlyReader) + + both, err := newTextMapWrapperForExtract(newTestTextMapReaderAndWriter()) + assert.NoError(t, err) + testExtractFunc(both) + + both, err = newTextMapWrapperForInject(newTestTextMapReaderAndWriter()) + assert.NoError(t, err) + testInjectFunc(both) +} + +var ( + testHeader = "test-trace-id" + traceID trace.TraceID = [16]byte{byte(10)} + spanID trace.SpanID = [8]byte{byte(11)} +) + +type testTextMapPropagator struct { +} + +func (t testTextMapPropagator) Inject(ctx context.Context, carrier propagation.TextMapCarrier) { + carrier.Set(testHeader, strings.Join([]string{traceID.String(), spanID.String()}, ":")) + + // Test for panic + _ = carrier.Get("test") + _ = carrier.Keys() +} + +func (t testTextMapPropagator) Extract(ctx context.Context, carrier propagation.TextMapCarrier) context.Context { + traces := carrier.Get(testHeader) + + str := strings.Split(traces, ":") + if len(str) != 2 { + return ctx + } + + var exist = false + + for _, key := range carrier.Keys() { + if strings.EqualFold(testHeader, key) { + exist = true + + break + } + } + + if !exist { + return ctx + } + + var ( + traceID, _ = trace.TraceIDFromHex(str[0]) + spanID, _ = trace.SpanIDFromHex(str[1]) + sc = trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + }) + ) + + // Test for panic + carrier.Set("key", "val") + + return trace.ContextWithRemoteSpanContext(ctx, sc) +} + +func (t testTextMapPropagator) Fields() []string { + return []string{"test"} +} + +// textMapCarrier Implemented propagation.TextMapCarrier interface. +type textMapCarrier struct { + m map[string]string +} + +func newTextCarrier() *textMapCarrier { + return &textMapCarrier{m: map[string]string{}} +} + +func (t *textMapCarrier) Get(key string) string { + return t.m[key] +} + +func (t *textMapCarrier) Set(key string, value string) { + t.m[key] = value +} + +func (t *textMapCarrier) Keys() []string { + str := make([]string, 0, len(t.m)) + + for key := range t.m { + str = append(str, key) + } + + return str +} + +// testTextMapReader only implemented opentracing.TextMapReader interface. +type testTextMapReader struct { + m *map[string]string +} + +func newTestTextMapReader(m *map[string]string) *testTextMapReader { + return &testTextMapReader{m: m} +} + +func (t *testTextMapReader) ForeachKey(handler func(key string, val string) error) error { + for key, val := range *t.m { + if err := handler(key, val); err != nil { + return err + } + } + + return nil +} + +// testTextMapWriter only implemented opentracing.TextMapWriter interface. +type testTextMapWriter struct { + m *map[string]string +} + +func newTestTextMapWriter(m *map[string]string) *testTextMapWriter { + return &testTextMapWriter{m: m} +} + +func (t *testTextMapWriter) Set(key, val string) { + (*t.m)[key] = val +} + +func TestBridgeTracer_ExtractAndInject(t *testing.T) { + bridge := NewBridgeTracer() + bridge.SetTextMapPropagator(new(testTextMapPropagator)) + + tmc := newTextCarrier() + shareMap := map[string]string{} + otTextMap := ot.TextMapCarrier{} + httpHeader := ot.HTTPHeadersCarrier(http.Header{}) + + testCases := []struct { + name string + injectCarrierType ot.BuiltinFormat + extractCarrierType ot.BuiltinFormat + extractCarrier interface{} + injectCarrier interface{} + extractErr error + injectErr error + }{ + { + name: "support for propagation.TextMapCarrier", + injectCarrierType: ot.TextMap, + injectCarrier: tmc, + extractCarrierType: ot.TextMap, + extractCarrier: tmc, + }, + { + name: "support for opentracing.TextMapReader and opentracing.TextMapWriter", + injectCarrierType: ot.TextMap, + injectCarrier: otTextMap, + extractCarrierType: ot.TextMap, + extractCarrier: otTextMap, + }, + { + name: "support for HTTPHeaders", + injectCarrierType: ot.HTTPHeaders, + injectCarrier: httpHeader, + extractCarrierType: ot.HTTPHeaders, + extractCarrier: httpHeader, + }, + { + name: "support for opentracing.TextMapReader and opentracing.TextMapWriter,non-same instance", + injectCarrierType: ot.TextMap, + injectCarrier: newTestTextMapWriter(&shareMap), + extractCarrierType: ot.TextMap, + extractCarrier: newTestTextMapReader(&shareMap), + }, + { + name: "inject: format type is HTTPHeaders, but carrier is not HTTPHeadersCarrier", + injectCarrierType: ot.HTTPHeaders, + injectCarrier: struct{}{}, + injectErr: ot.ErrInvalidCarrier, + }, + { + name: "extract: format type is HTTPHeaders, but carrier is not HTTPHeadersCarrier", + injectCarrierType: ot.HTTPHeaders, + injectCarrier: httpHeader, + extractCarrierType: ot.HTTPHeaders, + extractCarrier: struct{}{}, + extractErr: ot.ErrInvalidCarrier, + }, + { + name: "inject: format type is TextMap, but carrier is cannot be wrapped into propagation.TextMapCarrier", + injectCarrierType: ot.TextMap, + injectCarrier: struct{}{}, + injectErr: ot.ErrInvalidCarrier, + }, + { + name: "extract: format type is TextMap, but carrier is cannot be wrapped into propagation.TextMapCarrier", + injectCarrierType: ot.TextMap, + injectCarrier: otTextMap, + extractCarrierType: ot.TextMap, + extractCarrier: struct{}{}, + extractErr: ot.ErrInvalidCarrier, + }, + { + name: "inject: unsupported format type", + injectCarrierType: ot.Binary, + injectErr: ot.ErrUnsupportedFormat, + }, + { + name: "extract: unsupported format type", + injectCarrierType: ot.TextMap, + injectCarrier: otTextMap, + extractCarrierType: ot.Binary, + extractCarrier: struct{}{}, + extractErr: ot.ErrUnsupportedFormat, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := bridge.Inject(newBridgeSpanContext(trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: [16]byte{byte(1)}, + SpanID: [8]byte{byte(2)}, + }), nil), tc.injectCarrierType, tc.injectCarrier) + assert.Equal(t, tc.injectErr, err) + + if tc.injectErr == nil { + spanContext, err := bridge.Extract(tc.extractCarrierType, tc.extractCarrier) + assert.Equal(t, tc.extractErr, err) + + if tc.extractErr == nil { + bsc, ok := spanContext.(*bridgeSpanContext) + assert.True(t, ok) + + assert.Equal(t, spanID.String(), bsc.otelSpanContext.SpanID().String()) + assert.Equal(t, traceID.String(), bsc.otelSpanContext.TraceID().String()) + } + } + }) + } +} diff --git a/bridge/opentracing/go.mod b/bridge/opentracing/go.mod index ef802c32103..765d1bad47f 100644 --- a/bridge/opentracing/go.mod +++ b/bridge/opentracing/go.mod @@ -6,13 +6,17 @@ replace go.opentelemetry.io/otel => ../.. require ( github.com/opentracing/opentracing-go v1.2.0 + github.com/stretchr/testify v1.7.2 go.opentelemetry.io/otel v1.7.0 go.opentelemetry.io/otel/trace v1.7.0 ) require ( + github.com/davecgh/go-spew v1.1.0 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) replace go.opentelemetry.io/otel/trace => ../../trace diff --git a/bridge/opentracing/go.sum b/bridge/opentracing/go.sum index 5ec9e745497..4814eca5ba9 100644 --- a/bridge/opentracing/go.sum +++ b/bridge/opentracing/go.sum @@ -13,8 +13,11 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s= +github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=