From 27617015d45e6cd550b9a7ac7715c37cc2f7d020 Mon Sep 17 00:00:00 2001 From: Gabriel Aszalos Date: Fri, 19 Jan 2018 10:09:06 -0500 Subject: [PATCH] opentracing: allow customizing propagator headers (#155) --- opentracing/config.go | 18 +++++--- opentracing/propagators.go | 69 +++++++++++++++++++++------- opentracing/propagators_test.go | 81 +++++++++++++++++++++++++++++++++ opentracing/tracer.go | 9 +--- opentracing/tracer_test.go | 42 ----------------- 5 files changed, 146 insertions(+), 73 deletions(-) create mode 100644 opentracing/propagators_test.go diff --git a/opentracing/config.go b/opentracing/config.go index 7a7534f676..4888691a35 100644 --- a/opentracing/config.go +++ b/opentracing/config.go @@ -30,6 +30,9 @@ type Configuration struct { // GlobalTags holds a set of tags that will be automatically applied to // all spans. GlobalTags map[string]interface{} + + // TextMapPropagator is an injector used for Context propagation. + TextMapPropagator Propagator } // NewConfiguration creates a `Configuration` object with default values. @@ -39,13 +42,14 @@ func NewConfiguration() *Configuration { // Configuration struct with default values return &Configuration{ - Enabled: true, - Debug: false, - ServiceName: binaryName, - SampleRate: 1, - AgentHostname: "localhost", - AgentPort: "8126", - GlobalTags: make(map[string]interface{}), + Enabled: true, + Debug: false, + ServiceName: binaryName, + SampleRate: 1, + AgentHostname: "localhost", + AgentPort: "8126", + GlobalTags: make(map[string]interface{}), + TextMapPropagator: NewTextMapPropagator("", "", ""), } } diff --git a/opentracing/propagators.go b/opentracing/propagators.go index 2559a7990b..e590c4f3c5 100644 --- a/opentracing/propagators.go +++ b/opentracing/propagators.go @@ -7,20 +7,55 @@ import ( ot "github.com/opentracing/opentracing-go" ) -type textMapPropagator struct{} +// Propagator implementations should be able to inject and extract +// SpanContexts into an implementation specific carrier. +type Propagator interface { + // Inject takes the SpanContext and injects it into the carrier using + // an implementation specific method. + Inject(context ot.SpanContext, carrier interface{}) error -const ( - prefixBaggage = "ot-baggage-" - prefixTracerState = "x-datadog-" + // Extract returns the SpanContext from the given carrier using an + // implementation specific method. + Extract(carrier interface{}) (ot.SpanContext, error) +} - fieldNameTraceID = prefixTracerState + "trace-id" - fieldNameParentID = prefixTracerState + "parent-id" +const ( + defaultBaggageHeaderPrefix = "ot-baggage-" + defaultTraceIDHeader = "x-datadog-trace-id" + defaultParentIDHeader = "x-datadog-parent-id" ) -// Inject defines the textMapPropagator to propagate SpanContext data +// NewTextMapPropagator returns a new propagator which uses opentracing.TextMap +// to inject and extract values. The parameters specify the prefix that will +// be used to prefix baggage header keys along with the trace and parent header. +// Empty strings may be provided to use the defaults, which are: "ot-baggage-" as +// prefix for baggage headers, "x-datadog-trace-id" and "x-datadog-parent-id" for +// trace and parent ID headers. +func NewTextMapPropagator(baggagePrefix, traceHeader, parentHeader string) *TextMapPropagator { + if baggagePrefix == "" { + baggagePrefix = defaultBaggageHeaderPrefix + } + if traceHeader == "" { + traceHeader = defaultTraceIDHeader + } + if parentHeader == "" { + parentHeader = defaultParentIDHeader + } + return &TextMapPropagator{baggagePrefix, traceHeader, parentHeader} +} + +// TextMapPropagator implements a propagator which uses opentracing.TextMap +// internally. +type TextMapPropagator struct { + baggagePrefix string + traceHeader string + parentHeader string +} + +// Inject defines the TextMapPropagator to propagate SpanContext data // out of the current process. The implementation propagates the // TraceID and the current active SpanID, as well as the Span baggage. -func (p *textMapPropagator) Inject(context ot.SpanContext, carrier interface{}) error { +func (p *TextMapPropagator) Inject(context ot.SpanContext, carrier interface{}) error { ctx, ok := context.(SpanContext) if !ok { return ot.ErrInvalidSpanContext @@ -31,18 +66,18 @@ func (p *textMapPropagator) Inject(context ot.SpanContext, carrier interface{}) } // propagate the TraceID and the current active SpanID - writer.Set(fieldNameTraceID, strconv.FormatUint(ctx.traceID, 10)) - writer.Set(fieldNameParentID, strconv.FormatUint(ctx.spanID, 10)) + writer.Set(p.traceHeader, strconv.FormatUint(ctx.traceID, 10)) + writer.Set(p.parentHeader, strconv.FormatUint(ctx.spanID, 10)) // propagate OpenTracing baggage for k, v := range ctx.baggage { - writer.Set(prefixBaggage+k, v) + writer.Set(p.baggagePrefix+k, v) } return nil } -// Extract does -func (p *textMapPropagator) Extract(carrier interface{}) (ot.SpanContext, error) { +// Extract implements Propagator. +func (p *TextMapPropagator) Extract(carrier interface{}) (ot.SpanContext, error) { reader, ok := carrier.(ot.TextMapReader) if !ok { return nil, ot.ErrInvalidCarrier @@ -54,20 +89,20 @@ func (p *textMapPropagator) Extract(carrier interface{}) (ot.SpanContext, error) // extract SpanContext fields err = reader.ForeachKey(func(k, v string) error { switch strings.ToLower(k) { - case fieldNameTraceID: + case p.traceHeader: traceID, err = strconv.ParseUint(v, 10, 64) if err != nil { return ot.ErrSpanContextCorrupted } - case fieldNameParentID: + case p.parentHeader: parentID, err = strconv.ParseUint(v, 10, 64) if err != nil { return ot.ErrSpanContextCorrupted } default: lowercaseK := strings.ToLower(k) - if strings.HasPrefix(lowercaseK, prefixBaggage) { - decodedBaggage[strings.TrimPrefix(lowercaseK, prefixBaggage)] = v + if strings.HasPrefix(lowercaseK, p.baggagePrefix) { + decodedBaggage[strings.TrimPrefix(lowercaseK, p.baggagePrefix)] = v } } diff --git a/opentracing/propagators_test.go b/opentracing/propagators_test.go new file mode 100644 index 0000000000..2c276ddc07 --- /dev/null +++ b/opentracing/propagators_test.go @@ -0,0 +1,81 @@ +package opentracing + +import ( + "net/http" + "strconv" + "testing" + + opentracing "github.com/opentracing/opentracing-go" + "github.com/stretchr/testify/assert" +) + +func TestTracerPropagationDefaults(t *testing.T) { + assert := assert.New(t) + + config := NewConfiguration() + tracer, _, _ := NewTracer(config) + + root := tracer.StartSpan("web.request") + ctx := root.Context() + headers := http.Header{} + + // inject the SpanContext + carrier := opentracing.HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, opentracing.HTTPHeaders, carrier) + assert.Nil(err) + + // retrieve the SpanContext + propagated, err := tracer.Extract(opentracing.HTTPHeaders, carrier) + assert.Nil(err) + + tCtx, ok := ctx.(SpanContext) + assert.True(ok) + tPropagated, ok := propagated.(SpanContext) + assert.True(ok) + + // compare if there is a Context match + assert.Equal(tCtx.traceID, tPropagated.traceID) + assert.Equal(tCtx.spanID, tPropagated.spanID) + + // ensure a child can be created + child := tracer.StartSpan("db.query", opentracing.ChildOf(propagated)) + tRoot, ok := root.(*Span) + assert.True(ok) + tChild, ok := child.(*Span) + assert.True(ok) + + assert.NotEqual(uint64(0), tChild.Span.TraceID) + assert.NotEqual(uint64(0), tChild.Span.SpanID) + assert.Equal(tRoot.Span.SpanID, tChild.Span.ParentID) + assert.Equal(tRoot.Span.TraceID, tChild.Span.ParentID) + + tid := strconv.FormatUint(tRoot.Span.TraceID, 10) + pid := strconv.FormatUint(tRoot.Span.SpanID, 10) + + // hardcode header names to fail test if defaults are changed + assert.Equal(headers.Get("x-datadog-trace-id"), tid) + assert.Equal(headers.Get("x-datadog-parent-id"), pid) +} + +func TestTracerTextMapPropagationHeader(t *testing.T) { + assert := assert.New(t) + + config := NewConfiguration() + config.TextMapPropagator = NewTextMapPropagator("bg-", "tid", "pid") + tracer, _, _ := NewTracer(config) + + root := tracer.StartSpan("web.request").SetBaggageItem("item", "x").(*Span) + ctx := root.Context() + headers := http.Header{} + + carrier := opentracing.HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, opentracing.HTTPHeaders, carrier) + assert.Nil(err) + + tid := strconv.FormatUint(root.Span.TraceID, 10) + pid := strconv.FormatUint(root.Span.SpanID, 10) + + assert.Equal(headers.Get("tid"), tid) + assert.Equal(headers.Get("pid"), pid) + assert.Equal(headers.Get("bg-item"), "x") +} diff --git a/opentracing/tracer.go b/opentracing/tracer.go index 4339509a59..80ea552b07 100644 --- a/opentracing/tracer.go +++ b/opentracing/tracer.go @@ -18,9 +18,6 @@ type Tracer struct { // config holds the Configuration used to create the Tracer. config *Configuration - - // textPropagator is an injector used for Context propagation. - textPropagator *textMapPropagator } // StartSpan creates, starts, and returns a new Span with the given `operationName` @@ -125,9 +122,8 @@ func (t *Tracer) startSpanWithOptions(operationName string, options ot.StartSpan func (t *Tracer) Inject(ctx ot.SpanContext, format interface{}, carrier interface{}) error { switch format { case ot.TextMap, ot.HTTPHeaders: - return t.textPropagator.Inject(ctx, carrier) + return t.config.TextMapPropagator.Inject(ctx, carrier) } - return ot.ErrUnsupportedFormat } @@ -135,9 +131,8 @@ func (t *Tracer) Inject(ctx ot.SpanContext, format interface{}, carrier interfac func (t *Tracer) Extract(format interface{}, carrier interface{}) (ot.SpanContext, error) { switch format { case ot.TextMap, ot.HTTPHeaders: - return t.textPropagator.Extract(carrier) + return t.config.TextMapPropagator.Extract(carrier) } - return nil, ot.ErrUnsupportedFormat } diff --git a/opentracing/tracer_test.go b/opentracing/tracer_test.go index 87b68414d4..a083dc43fe 100644 --- a/opentracing/tracer_test.go +++ b/opentracing/tracer_test.go @@ -1,7 +1,6 @@ package opentracing import ( - "net/http" "testing" "time" @@ -130,44 +129,3 @@ func TestTracerSpanStartTime(t *testing.T) { assert.Equal(startTime.UnixNano(), span.Span.Start) } - -func TestTracerPropagation(t *testing.T) { - assert := assert.New(t) - - config := NewConfiguration() - tracer, _, _ := NewTracer(config) - - root := tracer.StartSpan("web.request") - ctx := root.Context() - headers := http.Header{} - - // inject the SpanContext - carrier := opentracing.HTTPHeadersCarrier(headers) - err := tracer.Inject(ctx, opentracing.HTTPHeaders, carrier) - assert.Nil(err) - - // retrieve the SpanContext - propagated, err := tracer.Extract(opentracing.HTTPHeaders, carrier) - assert.Nil(err) - - tCtx, ok := ctx.(SpanContext) - assert.True(ok) - tPropagated, ok := propagated.(SpanContext) - assert.True(ok) - - // compare if there is a Context match - assert.Equal(tCtx.traceID, tPropagated.traceID) - assert.Equal(tCtx.spanID, tPropagated.spanID) - - // ensure a child can be created - child := tracer.StartSpan("db.query", opentracing.ChildOf(propagated)) - tRoot, ok := root.(*Span) - assert.True(ok) - tChild, ok := child.(*Span) - assert.True(ok) - - assert.NotEqual(uint64(0), tChild.Span.TraceID) - assert.NotEqual(uint64(0), tChild.Span.SpanID) - assert.Equal(tRoot.Span.SpanID, tChild.Span.ParentID) - assert.Equal(tRoot.Span.TraceID, tChild.Span.ParentID) -}