From 3fed2828c3f26b0b1581c768d5dc4730f0e6a222 Mon Sep 17 00:00:00 2001 From: Nadir Boukeffa Date: Wed, 9 Jan 2019 14:53:32 +0100 Subject: [PATCH] Change span injection/extraction from HTTP request Today, `TextMap` format is used when injecting or extracing *span* from HTTP requests. This format will treat the HTTP header value (that contains the span) as is (without decoding). Conversely the `HTTPHeaders` format will try to URL decode the value first. The issue is that some opentracing clients like [java jaeger client](https://github.com/jaegertracing/jaeger-client-java/blob/f5f6ad1f4eaf606aba07562b7543958ec4658ebe/jaeger-core/src/main/java/io/jaegertracing/internal/propagation/TextMapCodec.java#L206) is URL encoding the HTTP header value before sending it over the wire. Then when the server receive the request and try to extract the span from it it will fail if `TextMap` format is used istead `HTTPHeaders` format. For example the java jaeger client will send something like this: `Uber-Trace-Id: 9ec076efd89825a%3A9c876c01059545b2%3Ab9ec076efd89825a%3A1` but the golang jaeger client is expecting this `Uber-Trace-Id: 9ec076efd89825a:9c876c01059545b2:b9ec076efd89825a:1` when the `TextMap` format is used. In this case the call results to the following [error](https://github.com/jaegertracing/jaeger-client-go/blob/master/context.go#L111): `String does not match tracer state format` Because the `http.go` file is used precisely for dealing with span in a HTTP context, I propose to default the span format to `HTTPHeaders` --- tracing/opentracing/http.go | 6 +++--- tracing/opentracing/http_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tracing/opentracing/http.go b/tracing/opentracing/http.go index 09dd7149a..b598a3e2d 100644 --- a/tracing/opentracing/http.go +++ b/tracing/opentracing/http.go @@ -6,7 +6,7 @@ import ( "net/http" "strconv" - "github.com/opentracing/opentracing-go" + opentracing "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" "github.com/go-kit/kit/log" @@ -36,7 +36,7 @@ func ContextToHTTP(tracer opentracing.Tracer, logger log.Logger) kithttp.Request // There's nothing we can do with any errors here. if err = tracer.Inject( span.Context(), - opentracing.TextMap, + opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header), ); err != nil { logger.Log("err", err) @@ -56,7 +56,7 @@ func HTTPToContext(tracer opentracing.Tracer, operationName string, logger log.L // Try to join to a trace propagated in `req`. var span opentracing.Span wireContext, err := tracer.Extract( - opentracing.TextMap, + opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header), ) if err != nil && err != opentracing.ErrSpanContextNotFound { diff --git a/tracing/opentracing/http_test.go b/tracing/opentracing/http_test.go index 548777e78..7d5571e05 100644 --- a/tracing/opentracing/http_test.go +++ b/tracing/opentracing/http_test.go @@ -6,7 +6,7 @@ import ( "reflect" "testing" - "github.com/opentracing/opentracing-go" + opentracing "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" "github.com/opentracing/opentracing-go/mocktracer" @@ -89,7 +89,7 @@ func TestHTTPToContextTags(t *testing.T) { parentSpan := tracer.StartSpan("to_extract").(*mocktracer.MockSpan) defer parentSpan.Finish() req, _ := http.NewRequest("GET", "http://test.biz/path", nil) - tracer.Inject(parentSpan.Context(), opentracing.TextMap, opentracing.HTTPHeadersCarrier(req.Header)) + tracer.Inject(parentSpan.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header)) ctx := kitot.HTTPToContext(tracer, "op", log.NewNopLogger())(context.Background(), req) opentracing.SpanFromContext(ctx).Finish()