From 8cddf30cb271465b13ed7b309f9ae08cd0fea819 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Tue, 10 Mar 2020 18:32:01 +0100 Subject: [PATCH] Context propagation in opentracing bridge (#525) * Propagate context changes in mix tests We will need this for testing the correlation context and baggage items propagation between the APIs. * Add baggage interoperation tests The test adds a baggage item to active OT span and some correlation key value to current Otel span. Then makes sure that the OT span contains both the baggage item and some translated version of the correlation key value its Otel sibling got, and that the Otel span contains both the correlation key value and the baggage item its OT sibling got. * Add hooks functionality to baggage propagation This introduces two kinds of hooks into the correlation context code. The set hook gets called every time we set a Map in the context. The hook receives a context with the Map and returns a new context. The get hook gets called every time we get a Map from the context. The hook receives the context and the map, and returns a new Map. These hooks will be used for correlation context and baggage items propagation between the Otel and OT APIs. * Warn on foreign opentracing span * fixup for using otel propagators * Add utility function for setting up bridge and context This prepares the context by installing the hooks, so the correlation context and baggage items can be propagated between the APIs. * Add bridge span constructor So I do not need to remember about initializing a newly added member in several places now. * Propagate baggage across otel and OT APIs This uses the set hook functionality to propagate correlation context changes from Otel to OT spans by inserting keys and values into the baggage items. The get hook functionality is used to propagate baggage items from active OT span into the otel correlation context. * Use correlation Map for baggage items We will put this map into the context with correlation context functions, and that is easier if we have correlation.Map, not map[string]string. * Use otel propagators in bridge The otel propagators are now kinda sorta usable for opentracing bridge. Some more work is needed to make it fully work, though - correlation context set with the otel API is not propagated to OT spans as baggage items yet. Co-authored-by: Joshua MacDonald --- api/correlation/context.go | 134 ++++++++++++++++++- bridge/opentracing/bridge.go | 229 ++++++++++++++++++++------------- bridge/opentracing/mix_test.go | 196 ++++++++++++++++++++++++---- bridge/opentracing/util.go | 8 ++ 4 files changed, 448 insertions(+), 119 deletions(-) diff --git a/api/correlation/context.go b/api/correlation/context.go index cda4f1518..8c4932ef9 100644 --- a/api/correlation/context.go +++ b/api/correlation/context.go @@ -22,11 +22,130 @@ import ( type correlationsType struct{} +// SetHookFunc describes a type of a callback that is called when +// storing baggage in the context. +type SetHookFunc func(context.Context) context.Context + +// GetHookFunc describes a type of a callback that is called when +// getting baggage from the context. +type GetHookFunc func(context.Context, Map) Map + +// value under this key is either of type Map or correlationsData var correlationsKey = &correlationsType{} +type correlationsData struct { + m Map + setHook SetHookFunc + getHook GetHookFunc +} + +func (d correlationsData) isHookless() bool { + return d.setHook == nil && d.getHook == nil +} + +type hookKind int + +const ( + hookKindSet hookKind = iota + hookKindGet +) + +func (d *correlationsData) overrideHook(kind hookKind, setHook SetHookFunc, getHook GetHookFunc) { + switch kind { + case hookKindSet: + d.setHook = setHook + case hookKindGet: + d.getHook = getHook + } +} + +// ContextWithSetHook installs a hook function that will be invoked +// every time ContextWithMap is called. To avoid unnecessary callback +// invocations (recursive or not), the callback can temporarily clear +// the hooks from the context with the ContextWithNoHooks function. +// +// Note that NewContext also calls ContextWithMap, so the hook will be +// invoked. +// +// Passing nil SetHookFunc creates a context with no set hook to call. +// +// This function should not be used by applications or libraries. It +// is mostly for interoperation with other observability APIs. +func ContextWithSetHook(ctx context.Context, hook SetHookFunc) context.Context { + return contextWithHook(ctx, hookKindSet, hook, nil) +} + +// ContextWithGetHook installs a hook function that will be invoked +// every time MapFromContext is called. To avoid unnecessary callback +// invocations (recursive or not), the callback can temporarily clear +// the hooks from the context with the ContextWithNoHooks function. +// +// Note that NewContext also calls MapFromContext, so the hook will be +// invoked. +// +// Passing nil GetHookFunc creates a context with no get hook to call. +// +// This function should not be used by applications or libraries. It +// is mostly for interoperation with other observability APIs. +func ContextWithGetHook(ctx context.Context, hook GetHookFunc) context.Context { + return contextWithHook(ctx, hookKindGet, nil, hook) +} + +func contextWithHook(ctx context.Context, kind hookKind, setHook SetHookFunc, getHook GetHookFunc) context.Context { + switch v := ctx.Value(correlationsKey).(type) { + case correlationsData: + v.overrideHook(kind, setHook, getHook) + if v.isHookless() { + return context.WithValue(ctx, correlationsKey, v.m) + } + return context.WithValue(ctx, correlationsKey, v) + case Map: + return contextWithOneHookAndMap(ctx, kind, setHook, getHook, v) + default: + m := NewEmptyMap() + return contextWithOneHookAndMap(ctx, kind, setHook, getHook, m) + } +} + +func contextWithOneHookAndMap(ctx context.Context, kind hookKind, setHook SetHookFunc, getHook GetHookFunc, m Map) context.Context { + d := correlationsData{m: m} + d.overrideHook(kind, setHook, getHook) + if d.isHookless() { + return ctx + } + return context.WithValue(ctx, correlationsKey, d) +} + +// ContextWithNoHooks creates a context with all the hooks +// disabled. Also returns old set and get hooks. This function can be +// used to temporarily clear the context from hooks and then reinstate +// them by calling ContextWithSetHook and ContextWithGetHook functions +// passing the hooks returned by this function. +// +// This function should not be used by applications or libraries. It +// is mostly for interoperation with other observability APIs. +func ContextWithNoHooks(ctx context.Context) (context.Context, SetHookFunc, GetHookFunc) { + switch v := ctx.Value(correlationsKey).(type) { + case correlationsData: + return context.WithValue(ctx, correlationsKey, v.m), v.setHook, v.getHook + default: + return ctx, nil, nil + } +} + // ContextWithMap returns a context with the Map entered into it. func ContextWithMap(ctx context.Context, m Map) context.Context { - return context.WithValue(ctx, correlationsKey, m) + switch v := ctx.Value(correlationsKey).(type) { + case correlationsData: + v.m = m + ctx = context.WithValue(ctx, correlationsKey, v) + if v.setHook != nil { + ctx = v.setHook(ctx) + } + return ctx + default: + return context.WithValue(ctx, correlationsKey, m) + } } // NewContext returns a context with the map from passed context @@ -39,8 +158,15 @@ func NewContext(ctx context.Context, keyvalues ...core.KeyValue) context.Context // MapFromContext gets the current Map from a Context. func MapFromContext(ctx context.Context) Map { - if m, ok := ctx.Value(correlationsKey).(Map); ok { - return m + switch v := ctx.Value(correlationsKey).(type) { + case correlationsData: + if v.getHook != nil { + return v.getHook(ctx, v.m) + } + return v.m + case Map: + return v + default: + return NewEmptyMap() } - return NewEmptyMap() } diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index e15e1aee9..af0dc3b56 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -28,16 +28,18 @@ import ( otlog "github.com/opentracing/opentracing-go/log" otelcore "go.opentelemetry.io/otel/api/core" + otelcorrelation "go.opentelemetry.io/otel/api/correlation" + otelglobal "go.opentelemetry.io/otel/api/global" otelkey "go.opentelemetry.io/otel/api/key" + otelpropagation "go.opentelemetry.io/otel/api/propagation" oteltrace "go.opentelemetry.io/otel/api/trace" + otelparent "go.opentelemetry.io/otel/internal/trace/parent" "go.opentelemetry.io/otel/bridge/opentracing/migration" ) type bridgeSpanContext struct { - // TODO: have a look at the java implementation of the shim to - // see what do they do with the baggage items - baggageItems map[string]string + baggageItems otelcorrelation.Map otelSpanContext otelcore.SpanContext } @@ -45,7 +47,7 @@ var _ ot.SpanContext = &bridgeSpanContext{} func newBridgeSpanContext(otelSpanContext otelcore.SpanContext, parentOtSpanContext ot.SpanContext) *bridgeSpanContext { bCtx := &bridgeSpanContext{ - baggageItems: nil, + baggageItems: otelcorrelation.NewEmptyMap(), otelSpanContext: otelSpanContext, } if parentOtSpanContext != nil { @@ -58,35 +60,42 @@ func newBridgeSpanContext(otelSpanContext otelcore.SpanContext, parentOtSpanCont } func (c *bridgeSpanContext) ForeachBaggageItem(handler func(k, v string) bool) { - for k, v := range c.baggageItems { - if !handler(k, v) { - break - } - } + c.baggageItems.Foreach(func(kv otelcore.KeyValue) bool { + return handler(string(kv.Key), kv.Value.Emit()) + }) } func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) { - if c.baggageItems == nil { - c.baggageItems = make(map[string]string) - } crk := http.CanonicalHeaderKey(restrictedKey) - c.baggageItems[crk] = value + c.baggageItems = c.baggageItems.Apply(otelcorrelation.MapUpdate{SingleKV: otelkey.New(crk).String(value)}) } func (c *bridgeSpanContext) baggageItem(restrictedKey string) string { crk := http.CanonicalHeaderKey(restrictedKey) - return c.baggageItems[crk] + val, _ := c.baggageItems.Value(otelkey.New(crk)) + return val.Emit() } type bridgeSpan struct { - otelSpan oteltrace.Span - ctx *bridgeSpanContext - tracer *BridgeTracer - skipDeferHook bool + otelSpan oteltrace.Span + ctx *bridgeSpanContext + tracer *BridgeTracer + skipDeferHook bool + extraBaggageItems map[string]string } var _ ot.Span = &bridgeSpan{} +func newBridgeSpan(otelSpan oteltrace.Span, bridgeSC *bridgeSpanContext, tracer *BridgeTracer) *bridgeSpan { + return &bridgeSpan{ + otelSpan: otelSpan, + ctx: bridgeSC, + tracer: tracer, + skipDeferHook: false, + extraBaggageItems: nil, + } +} + func (s *bridgeSpan) Finish() { s.otelSpan.End() } @@ -208,10 +217,22 @@ func (s *bridgeSpan) LogKV(alternatingKeyValues ...interface{}) { } func (s *bridgeSpan) SetBaggageItem(restrictedKey, value string) ot.Span { - s.ctx.setBaggageItem(restrictedKey, value) + s.updateOtelContext(restrictedKey, value) + s.setBaggageItemOnly(restrictedKey, value) return s } +func (s *bridgeSpan) setBaggageItemOnly(restrictedKey, value string) { + s.ctx.setBaggageItem(restrictedKey, value) +} + +func (s *bridgeSpan) updateOtelContext(restrictedKey, value string) { + if s.extraBaggageItems == nil { + s.extraBaggageItems = make(map[string]string) + } + s.extraBaggageItems[restrictedKey] = value +} + func (s *bridgeSpan) BaggageItem(restrictedKey string) string { return s.ctx.baggageItem(restrictedKey) } @@ -266,6 +287,8 @@ type BridgeTracer struct { warningHandler BridgeWarningHandler warnOnce sync.Once + + propagators otelpropagation.Propagators } var _ ot.Tracer = &BridgeTracer{} @@ -282,6 +305,7 @@ func NewBridgeTracer() *BridgeTracer { otelTracer: oteltrace.NoopTracer{}, }, warningHandler: func(msg string) {}, + propagators: nil, } } @@ -299,6 +323,61 @@ func (t *BridgeTracer) SetOpenTelemetryTracer(tracer oteltrace.Tracer) { t.setTracer.isSet = true } +func (t *BridgeTracer) SetPropagators(propagators otelpropagation.Propagators) { + t.propagators = propagators +} + +func (t *BridgeTracer) NewHookedContext(ctx context.Context) context.Context { + ctx = otelcorrelation.ContextWithSetHook(ctx, t.correlationSetHook) + ctx = otelcorrelation.ContextWithGetHook(ctx, t.correlationGetHook) + return ctx +} + +func (t *BridgeTracer) correlationSetHook(ctx context.Context) context.Context { + span := ot.SpanFromContext(ctx) + if span == nil { + t.warningHandler("No active OpenTracing span, can not propagate the baggage items from OpenTelemetry context\n") + return ctx + } + bSpan, ok := span.(*bridgeSpan) + if !ok { + t.warningHandler("Encountered a foreign OpenTracing span, will not propagate the baggage items from OpenTelemetry context\n") + return ctx + } + // we clear the context only to avoid calling a get hook + // during MapFromContext, but otherwise we don't change the + // context, so we don't care about the old hooks. + clearCtx, _, _ := otelcorrelation.ContextWithNoHooks(ctx) + m := otelcorrelation.MapFromContext(clearCtx) + m.Foreach(func(kv otelcore.KeyValue) bool { + bSpan.setBaggageItemOnly(string(kv.Key), kv.Value.Emit()) + return true + }) + return ctx +} + +func (t *BridgeTracer) correlationGetHook(ctx context.Context, m otelcorrelation.Map) otelcorrelation.Map { + span := ot.SpanFromContext(ctx) + if span == nil { + t.warningHandler("No active OpenTracing span, can not propagate the baggage items from OpenTracing span context\n") + return m + } + bSpan, ok := span.(*bridgeSpan) + if !ok { + t.warningHandler("Encountered a foreign OpenTracing span, will not propagate the baggage items from OpenTracing span context\n") + return m + } + items := bSpan.extraBaggageItems + if len(items) == 0 { + return m + } + kv := make([]otelcore.KeyValue, 0, len(items)) + for k, v := range items { + kv = append(kv, otelkey.String(k, v)) + } + return m.Apply(otelcorrelation.MapUpdate{MultiKV: kv}) +} + // StartSpan is a part of the implementation of the OpenTracing Tracer // interface. func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOption) ot.Span { @@ -339,11 +418,7 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio otSpanContext = parentBridgeSC } sctx := newBridgeSpanContext(otelSpan.SpanContext(), otSpanContext) - span := &bridgeSpan{ - otelSpan: otelSpan, - ctx: sctx, - tracer: t, - } + span := newBridgeSpan(otelSpan, sctx, t) return span } @@ -360,12 +435,8 @@ func (t *BridgeTracer) ContextWithBridgeSpan(ctx context.Context, span oteltrace otSpanContext = parentSpan.Context() } bCtx := newBridgeSpanContext(span.SpanContext(), otSpanContext) - bSpan := &bridgeSpan{ - otelSpan: span, - ctx: bCtx, - tracer: t, - skipDeferHook: true, - } + bSpan := newBridgeSpan(span, bCtx, t) + bSpan.skipDeferHook = true return ot.ContextWithSpan(ctx, bSpan) } @@ -375,7 +446,11 @@ func (t *BridgeTracer) ContextWithBridgeSpan(ctx context.Context, span oteltrace // DeferredContextSetupTracerExtension interface. func (t *BridgeTracer) ContextWithSpanHook(ctx context.Context, span ot.Span) context.Context { bSpan, ok := span.(*bridgeSpan) - if !ok || bSpan.skipDeferHook { + if !ok { + t.warningHandler("Encountered a foreign OpenTracing span, will not run a possible deferred context setup hook\n") + return ctx + } + if bSpan.skipDeferHook { return ctx } if tracerWithExtension, ok := bSpan.tracer.setTracer.tracer().(migration.DeferredContextSetupTracerExtension); ok { @@ -502,18 +577,22 @@ func otSpanReferenceTypeToString(refType ot.SpanReferenceType) string { } } -// TODO: these headers are most likely bogus -var ( - traceIDHeader = http.CanonicalHeaderKey("x-otelbridge-trace-id") - spanIDHeader = http.CanonicalHeaderKey("x-otelbridge-span-id") - traceFlagsHeader = http.CanonicalHeaderKey("x-otelbridge-trace-flags") - baggageHeaderPrefix = http.CanonicalHeaderKey("x-otelbridge-baggage-") -) +// fakeSpan is just a holder of span context, nothing more. It's for +// propagators, so they can get the span context from Go context. +type fakeSpan struct { + oteltrace.NoopSpan + + sc otelcore.SpanContext +} + +func (s fakeSpan) SpanContext() otelcore.SpanContext { + return s.sc +} // Inject is a part of the implementation of the OpenTracing Tracer // interface. // -// Currently only the HTTPHeaders format is kinda sorta supported. +// Currently only the HTTPHeaders format is supported. func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier interface{}) error { bridgeSC, ok := sm.(*bridgeSpanContext) if !ok { @@ -529,29 +608,20 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int if !ok { return ot.ErrInvalidCarrier } - hhcarrier.Set(traceIDHeader, bridgeSC.otelSpanContext.TraceIDString()) - hhcarrier.Set(spanIDHeader, bridgeSC.otelSpanContext.SpanIDString()) - hhcarrier.Set(traceFlagsHeader, traceFlagsToString(bridgeSC.otelSpanContext.TraceFlags)) - bridgeSC.ForeachBaggageItem(func(k, v string) bool { - // we assume that keys are already canonicalized - hhcarrier.Set(baggageHeaderPrefix+k, v) - return true - }) - return nil -} - -func traceFlagsToString(opts byte) string { - var parts []string - if opts&otelcore.TraceFlagsSampled == otelcore.TraceFlagsSampled { - parts = append(parts, "sampled") + header := http.Header(hhcarrier) + fs := fakeSpan{ + sc: bridgeSC.otelSpanContext, } - return strings.Join(parts, ",") + ctx := oteltrace.ContextWithSpan(context.Background(), fs) + ctx = otelcorrelation.ContextWithMap(ctx, bridgeSC.baggageItems) + otelpropagation.InjectHTTP(ctx, t.getPropagators(), header) + return nil } // Extract is a part of the implementation of the OpenTracing Tracer // interface. // -// Currently only the HTTPHeaders format is kinda sorta supported. +// Currently only the HTTPHeaders format is supported. func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.SpanContext, error) { if builtinFormat, ok := format.(ot.BuiltinFormat); !ok || builtinFormat != ot.HTTPHeaders { return nil, ot.ErrUnsupportedFormat @@ -560,34 +630,13 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span if !ok { return nil, ot.ErrInvalidCarrier } - bridgeSC := &bridgeSpanContext{} - err := hhcarrier.ForeachKey(func(k, v string) error { - ck := http.CanonicalHeaderKey(k) - switch ck { - case traceIDHeader: - traceID, err := otelcore.TraceIDFromHex(v) - if err != nil { - return err - } - bridgeSC.otelSpanContext.TraceID = traceID - case spanIDHeader: - spanID, err := otelcore.SpanIDFromHex(v) - if err != nil { - return err - } - bridgeSC.otelSpanContext.SpanID = spanID - case traceFlagsHeader: - bridgeSC.otelSpanContext.TraceFlags = stringToTraceFlags(v) - default: - if strings.HasPrefix(ck, baggageHeaderPrefix) { - bk := strings.TrimPrefix(ck, baggageHeaderPrefix) - bridgeSC.setBaggageItem(bk, v) - } - } - return nil - }) - if err != nil { - return nil, err + header := http.Header(hhcarrier) + ctx := otelpropagation.ExtractHTTP(context.Background(), t.getPropagators(), header) + baggage := otelcorrelation.MapFromContext(ctx) + otelSC, _, _ := otelparent.GetSpanContextAndLinks(ctx, false) + bridgeSC := &bridgeSpanContext{ + baggageItems: baggage, + otelSpanContext: otelSC, } if !bridgeSC.otelSpanContext.IsValid() { return nil, ot.ErrSpanContextNotFound @@ -595,13 +644,9 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span return bridgeSC, nil } -func stringToTraceFlags(s string) byte { - var opts byte - for _, part := range strings.Split(s, ",") { - switch part { - case "sampled": - opts |= otelcore.TraceFlagsSampled - } +func (t *BridgeTracer) getPropagators() otelpropagation.Propagators { + if t.propagators != nil { + return t.propagators } - return opts + return otelglobal.Propagators() } diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index ec1ec9fa4..5651ff773 100644 --- a/bridge/opentracing/mix_test.go +++ b/bridge/opentracing/mix_test.go @@ -22,7 +22,9 @@ import ( ot "github.com/opentracing/opentracing-go" otelcore "go.opentelemetry.io/otel/api/core" - "go.opentelemetry.io/otel/api/global" + otelcorrelation "go.opentelemetry.io/otel/api/correlation" + otelglobal "go.opentelemetry.io/otel/api/global" + otelkey "go.opentelemetry.io/otel/api/key" oteltrace "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/bridge/opentracing/internal" @@ -42,6 +44,7 @@ func getMixedAPIsTestCases() []mixedAPIsTestCase { coin := newContextIntactTest() bip := newBaggageItemsPreservationTest() tm := newTracerMessTest() + bio := newBaggageInteroperationTest() return []mixedAPIsTestCase{ { @@ -104,6 +107,18 @@ func getMixedAPIsTestCases() []mixedAPIsTestCase { run: tm.runOTOtelOT, check: tm.check, }, + { + desc: "baggage items interoperation across layers ot -> otel -> ot", + setup: bio.setup, + run: bio.runOTOtelOT, + check: bio.check, + }, + { + desc: "baggage items interoperation across layers otel -> ot -> otel", + setup: bio.setup, + run: bio.runOtelOTOtel, + check: bio.check, + }, } } @@ -111,13 +126,12 @@ func TestMixedAPIs(t *testing.T) { for idx, tc := range getMixedAPIsTestCases() { t.Logf("Running test case %d: %s", idx, tc.desc) mockOtelTracer := internal.NewMockTracer() - otTracer, otelProvider := NewTracerPair(mockOtelTracer) + ctx, otTracer, otelProvider := NewTracerPairWithContext(context.Background(), mockOtelTracer) otTracer.SetWarningHandler(func(msg string) { t.Log(msg) }) - ctx := context.Background() - global.SetTraceProvider(otelProvider) + otelglobal.SetTraceProvider(otelProvider) ot.SetGlobalTracer(otTracer) tc.setup(t, mockOtelTracer) @@ -157,7 +171,8 @@ func (st *simpleTest) runOTOtelOT(t *testing.T, ctx context.Context) { runOTOtelOT(t, ctx, "simple", st.noop) } -func (st *simpleTest) noop(t *testing.T, ctx context.Context) { +func (st *simpleTest) noop(t *testing.T, ctx context.Context) context.Context { + return ctx } // current/active span test @@ -214,7 +229,7 @@ func (cast *currentActiveSpanTest) runOTOtelOT(t *testing.T, ctx context.Context runOTOtelOT(t, ctx, "cast", cast.recordSpans) } -func (cast *currentActiveSpanTest) recordSpans(t *testing.T, ctx context.Context) { +func (cast *currentActiveSpanTest) recordSpans(t *testing.T, ctx context.Context) context.Context { spanID := oteltrace.SpanFromContext(ctx).SpanContext().SpanID cast.recordedCurrentOtelSpanIDs = append(cast.recordedCurrentOtelSpanIDs, spanID) @@ -223,6 +238,7 @@ func (cast *currentActiveSpanTest) recordSpans(t *testing.T, ctx context.Context spanID = bridgeSpan.otelSpan.SpanContext().SpanID } cast.recordedActiveOTSpanIDs = append(cast.recordedActiveOTSpanIDs, spanID) + return ctx } // context intact test @@ -296,14 +312,15 @@ func (coin *contextIntactTest) runOTOtelOT(t *testing.T, ctx context.Context) { runOTOtelOT(t, ctx, "coin", coin.recordValue) } -func (coin *contextIntactTest) recordValue(t *testing.T, ctx context.Context) { +func (coin *contextIntactTest) recordValue(t *testing.T, ctx context.Context) context.Context { if coin.recordIdx >= len(coin.contextKeyValues) { t.Errorf("Too many steps?") - return + return ctx } key := coin.contextKeyValues[coin.recordIdx].Key coin.recordIdx++ coin.recordedContextValues = append(coin.recordedContextValues, ctx.Value(key)) + return ctx } // baggage items preservation test @@ -380,15 +397,15 @@ func (bip *baggageItemsPreservationTest) runOTOtelOT(t *testing.T, ctx context.C runOTOtelOT(t, ctx, "bip", bip.addAndRecordBaggage) } -func (bip *baggageItemsPreservationTest) addAndRecordBaggage(t *testing.T, ctx context.Context) { +func (bip *baggageItemsPreservationTest) addAndRecordBaggage(t *testing.T, ctx context.Context) context.Context { if bip.step >= len(bip.baggageItems) { t.Errorf("Too many steps?") - return + return ctx } span := ot.SpanFromContext(ctx) if span == nil { t.Errorf("No active OpenTracing span") - return + return ctx } idx := bip.step bip.step++ @@ -400,6 +417,7 @@ func (bip *baggageItemsPreservationTest) addAndRecordBaggage(t *testing.T, ctx c return true }) bip.recordedBaggage = append(bip.recordedBaggage, recording) + return ctx } // tracer mess test @@ -423,7 +441,7 @@ func (tm *tracerMessTest) setup(t *testing.T, tracer *internal.MockTracer) { func (tm *tracerMessTest) check(t *testing.T, tracer *internal.MockTracer) { globalOtTracer := ot.GlobalTracer() - globalOtelTracer := global.TraceProvider().Tracer("") + globalOtelTracer := otelglobal.TraceProvider().Tracer("") if len(tm.recordedOTSpanTracers) != 3 { t.Errorf("Expected 3 recorded OpenTracing tracers from spans, got %d", len(tm.recordedOTSpanTracers)) } @@ -450,7 +468,7 @@ func (tm *tracerMessTest) runOTOtelOT(t *testing.T, ctx context.Context) { runOTOtelOT(t, ctx, "tm", tm.recordTracers) } -func (tm *tracerMessTest) recordTracers(t *testing.T, ctx context.Context) { +func (tm *tracerMessTest) recordTracers(t *testing.T, ctx context.Context) context.Context { otSpan := ot.SpanFromContext(ctx) if otSpan == nil { t.Errorf("No current OpenTracing span?") @@ -460,6 +478,138 @@ func (tm *tracerMessTest) recordTracers(t *testing.T, ctx context.Context) { otelSpan := oteltrace.SpanFromContext(ctx) tm.recordedOtelSpanTracers = append(tm.recordedOtelSpanTracers, otelSpan.Tracer()) + return ctx +} + +// baggage interoperation test + +type baggageInteroperationTest struct { + baggageItems []bipBaggage + + step int + recordedOTBaggage []map[string]string + recordedOtelBaggage []map[string]string +} + +func newBaggageInteroperationTest() *baggageInteroperationTest { + return &baggageInteroperationTest{ + baggageItems: []bipBaggage{ + { + key: "First", + value: "one", + }, + { + key: "Second", + value: "two", + }, + { + key: "Third", + value: "three", + }, + }, + } +} + +func (bio *baggageInteroperationTest) setup(t *testing.T, tracer *internal.MockTracer) { + bio.step = 0 + bio.recordedOTBaggage = nil + bio.recordedOtelBaggage = nil +} + +func (bio *baggageInteroperationTest) check(t *testing.T, tracer *internal.MockTracer) { + checkBIORecording(t, "OT", bio.baggageItems, bio.recordedOTBaggage) + checkBIORecording(t, "Otel", bio.baggageItems, bio.recordedOtelBaggage) +} + +func checkBIORecording(t *testing.T, apiDesc string, initialItems []bipBaggage, recordings []map[string]string) { + // expect recordings count to equal the number of initial + // items + + // each recording should have a duplicated item from initial + // items, one with OT suffix, another one with Otel suffix + + // expect each subsequent recording to have two more items, up + // to double of the count of the initial items + + if len(initialItems) != len(recordings) { + t.Errorf("Expected %d recordings from %s, got %d", len(initialItems), apiDesc, len(recordings)) + } + minRecLen := min(len(initialItems), len(recordings)) + for i := 0; i < minRecLen; i++ { + recordedItems := recordings[i] + expectedItemsInStep := (i + 1) * 2 + if expectedItemsInStep != len(recordedItems) { + t.Errorf("Expected %d recorded items in recording %d from %s, got %d", expectedItemsInStep, i, apiDesc, len(recordedItems)) + } + recordedItemsCopy := make(map[string]string, len(recordedItems)) + for k, v := range recordedItems { + recordedItemsCopy[k] = v + } + for j := 0; j < i+1; j++ { + otKey, otelKey := generateBaggageKeys(initialItems[j].key) + value := initialItems[j].value + for _, k := range []string{otKey, otelKey} { + if v, ok := recordedItemsCopy[k]; ok { + if value != v { + t.Errorf("Expected value %s under key %s in recording %d from %s, got %s", value, k, i, apiDesc, v) + } + delete(recordedItemsCopy, k) + } else { + t.Errorf("Missing key %s in recording %d from %s", k, i, apiDesc) + } + } + } + for k, v := range recordedItemsCopy { + t.Errorf("Unexpected key-value pair %s = %s in recording %d from %s", k, v, i, apiDesc) + } + } +} + +func (bio *baggageInteroperationTest) runOtelOTOtel(t *testing.T, ctx context.Context) { + runOtelOTOtel(t, ctx, "bio", bio.addAndRecordBaggage) +} + +func (bio *baggageInteroperationTest) runOTOtelOT(t *testing.T, ctx context.Context) { + runOTOtelOT(t, ctx, "bio", bio.addAndRecordBaggage) +} + +func (bio *baggageInteroperationTest) addAndRecordBaggage(t *testing.T, ctx context.Context) context.Context { + if bio.step >= len(bio.baggageItems) { + t.Errorf("Too many steps?") + return ctx + } + otSpan := ot.SpanFromContext(ctx) + if otSpan == nil { + t.Errorf("No active OpenTracing span") + return ctx + } + idx := bio.step + bio.step++ + key := bio.baggageItems[idx].key + otKey, otelKey := generateBaggageKeys(key) + value := bio.baggageItems[idx].value + + otSpan.SetBaggageItem(otKey, value) + ctx = otelcorrelation.NewContext(ctx, otelkey.String(otelKey, value)) + + otRecording := make(map[string]string) + otSpan.Context().ForeachBaggageItem(func(key, value string) bool { + otRecording[key] = value + return true + }) + otelRecording := make(map[string]string) + otelcorrelation.MapFromContext(ctx).Foreach(func(kv otelcore.KeyValue) bool { + otelRecording[string(kv.Key)] = kv.Value.Emit() + return true + }) + bio.recordedOTBaggage = append(bio.recordedOTBaggage, otRecording) + bio.recordedOtelBaggage = append(bio.recordedOtelBaggage, otelRecording) + return ctx +} + +func generateBaggageKeys(key string) (otKey, otelKey string) { + otKey, otelKey = key+"-Ot", key+"-Otel" + return } // helpers @@ -534,36 +684,36 @@ func min(a, b int) int { return a } -func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context)) { - tr := global.TraceProvider().Tracer("") +func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context) context.Context) { + tr := otelglobal.TraceProvider().Tracer("") ctx, span := tr.Start(ctx, fmt.Sprintf("%s_Otel_OTOtel", name), oteltrace.WithSpanKind(oteltrace.SpanKindClient)) defer span.End() - callback(t, ctx) + ctx = callback(t, ctx) func(ctx2 context.Context) { span, ctx2 := ot.StartSpanFromContext(ctx2, fmt.Sprintf("%sOtel_OT_Otel", name)) defer span.Finish() - callback(t, ctx2) + ctx2 = callback(t, ctx2) func(ctx3 context.Context) { ctx3, span := tr.Start(ctx3, fmt.Sprintf("%sOtelOT_Otel_", name), oteltrace.WithSpanKind(oteltrace.SpanKindProducer)) defer span.End() - callback(t, ctx3) + _ = callback(t, ctx3) }(ctx2) }(ctx) } -func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context)) { - tr := global.TraceProvider().Tracer("") +func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context) context.Context) { + tr := otelglobal.TraceProvider().Tracer("") span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: "client"}) defer span.Finish() - callback(t, ctx) + ctx = callback(t, ctx) func(ctx2 context.Context) { ctx2, span := tr.Start(ctx2, fmt.Sprintf("%sOT_Otel_OT", name)) defer span.End() - callback(t, ctx2) + ctx2 = callback(t, ctx2) func(ctx3 context.Context) { span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: "producer"}) defer span.Finish() - callback(t, ctx3) + _ = callback(t, ctx3) }(ctx2) }(ctx) } diff --git a/bridge/opentracing/util.go b/bridge/opentracing/util.go index 10bdd114f..fcddc2db2 100644 --- a/bridge/opentracing/util.go +++ b/bridge/opentracing/util.go @@ -15,6 +15,8 @@ package opentracing import ( + "context" + oteltrace "go.opentelemetry.io/otel/api/trace" ) @@ -30,3 +32,9 @@ func NewTracerPair(tracer oteltrace.Tracer) (*BridgeTracer, *WrapperProvider) { bridgeTracer.SetOpenTelemetryTracer(wrapperProvider.Tracer("")) return bridgeTracer, wrapperProvider } + +func NewTracerPairWithContext(ctx context.Context, tracer oteltrace.Tracer) (context.Context, *BridgeTracer, *WrapperProvider) { + bridgeTracer, wrapperProvider := NewTracerPair(tracer) + ctx = bridgeTracer.NewHookedContext(ctx) + return ctx, bridgeTracer, wrapperProvider +}