From 1dc6bc09d4abadcb302c6348a3baf2a759abebbe Mon Sep 17 00:00:00 2001 From: Anthony J Mirabella Date: Sat, 2 May 2020 15:23:03 -0400 Subject: [PATCH 1/2] propagate at least the first w3c tracestate header --- .../trace_context_propagator_test.go | 19 +++++++- api/trace/trace_context_propagator.go | 48 ++++++++++++------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/api/trace/testtrace/trace_context_propagator_test.go b/api/trace/testtrace/trace_context_propagator_test.go index 2487dcaec..6d320dc5d 100644 --- a/api/trace/testtrace/trace_context_propagator_test.go +++ b/api/trace/testtrace/trace_context_propagator_test.go @@ -288,9 +288,26 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { func TestTraceContextPropagator_GetAllKeys(t *testing.T) { var propagator trace.TraceContext - want := []string{"traceparent"} + want := []string{"traceparent", "tracestate"} got := propagator.GetAllKeys() if diff := cmp.Diff(got, want); diff != "" { t.Errorf("GetAllKeys: -got +want %s", diff) } } + +func TestTraceStatePropagation(t *testing.T) { + props := propagation.New(propagation.WithInjectors(trace.TraceContext{}), propagation.WithExtractors(trace.TraceContext{})) + want := "opaquevalue" + headerName := "tracestate" + + inReq, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + inReq.Header.Add(headerName, want) + ctx := propagation.ExtractHTTP(context.Background(), props, inReq.Header) + + outReq, _ := http.NewRequest(http.MethodGet, "http://www.example.com", nil) + propagation.InjectHTTP(ctx, props, outReq.Header) + + if diff := cmp.Diff(outReq.Header.Get(headerName), want); diff != "" { + t.Errorf("Propagate tracestate: -got +want %s", diff) + } +} diff --git a/api/trace/trace_context_propagator.go b/api/trace/trace_context_propagator.go index ab54ffbf0..fde0ab536 100644 --- a/api/trace/trace_context_propagator.go +++ b/api/trace/trace_context_propagator.go @@ -19,7 +19,6 @@ import ( "encoding/hex" "fmt" "regexp" - "strings" "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/api/propagation" @@ -29,6 +28,13 @@ const ( supportedVersion = 0 maxVersion = 254 traceparentHeader = "traceparent" + tracestateHeader = "tracestate" +) + +type traceContextPropagatorKeyType uint + +const ( + tracestateKey traceContextPropagatorKeyType = 0 ) // TraceContext propagates SpanContext in W3C TraceContext format. @@ -36,7 +42,7 @@ const ( type TraceContext struct{} var _ propagation.HTTPPropagator = TraceContext{} -var traceCtxRegExp = regexp.MustCompile("^[0-9a-f]{2}-[a-f0-9]{32}-[a-f0-9]{16}-[a-f0-9]{2}-?") +var traceCtxRegExp = regexp.MustCompile("^([0-9a-f]{2})-([a-f0-9]{32})-([a-f0-9]{16})-([a-f0-9]{2})(?:-.*)?$") // DefaultHTTPPropagator returns the default trace HTTP propagator. func DefaultHTTPPropagator() propagation.HTTPPropagator { @@ -44,6 +50,11 @@ func DefaultHTTPPropagator() propagation.HTTPPropagator { } func (TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { + tracestate := ctx.Value(tracestateKey) + if tracestate != nil { + supplier.Set(tracestateHeader, tracestate.(string)) + } + sc := SpanFromContext(ctx).SpanContext() if !sc.IsValid() { return @@ -57,6 +68,11 @@ func (TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupplie } func (tc TraceContext) Extract(ctx context.Context, supplier propagation.HTTPSupplier) context.Context { + state := supplier.Get(tracestateHeader) + if state != "" { + ctx = context.WithValue(ctx, tracestateKey, state) + } + sc := tc.extract(supplier) if !sc.IsValid() { return ctx @@ -70,20 +86,20 @@ func (TraceContext) extract(supplier propagation.HTTPSupplier) core.SpanContext return core.EmptySpanContext() } - h = strings.Trim(h, "-") - if !traceCtxRegExp.MatchString(h) { + matches := traceCtxRegExp.FindStringSubmatch(h) + + if len(matches) == 0 { return core.EmptySpanContext() } - sections := strings.Split(h, "-") - if len(sections) < 4 { + if len(matches) < 5 { // four subgroups plus the overall match return core.EmptySpanContext() } - if len(sections[0]) != 2 { + if len(matches[1]) != 2 { return core.EmptySpanContext() } - ver, err := hex.DecodeString(sections[0]) + ver, err := hex.DecodeString(matches[1]) if err != nil { return core.EmptySpanContext() } @@ -92,33 +108,33 @@ func (TraceContext) extract(supplier propagation.HTTPSupplier) core.SpanContext return core.EmptySpanContext() } - if version == 0 && len(sections) != 4 { + if version == 0 && len(matches) != 5 { // four subgroups plus the overall match return core.EmptySpanContext() } - if len(sections[1]) != 32 { + if len(matches[2]) != 32 { return core.EmptySpanContext() } var sc core.SpanContext - sc.TraceID, err = core.TraceIDFromHex(sections[1][:32]) + sc.TraceID, err = core.TraceIDFromHex(matches[2][:32]) if err != nil { return core.EmptySpanContext() } - if len(sections[2]) != 16 { + if len(matches[3]) != 16 { return core.EmptySpanContext() } - sc.SpanID, err = core.SpanIDFromHex(sections[2]) + sc.SpanID, err = core.SpanIDFromHex(matches[3]) if err != nil { return core.EmptySpanContext() } - if len(sections[3]) != 2 { + if len(matches[4]) != 2 { return core.EmptySpanContext() } - opts, err := hex.DecodeString(sections[3]) + opts, err := hex.DecodeString(matches[4]) if err != nil || len(opts) < 1 || (version == 0 && opts[0] > 2) { return core.EmptySpanContext() } @@ -132,5 +148,5 @@ func (TraceContext) extract(supplier propagation.HTTPSupplier) core.SpanContext } func (TraceContext) GetAllKeys() []string { - return []string{traceparentHeader} + return []string{traceparentHeader, tracestateHeader} } From 7a605a3345b8f35cfbb3feb0c38fe9dffc228c37 Mon Sep 17 00:00:00 2001 From: Anthony J Mirabella Date: Mon, 4 May 2020 16:34:07 -0400 Subject: [PATCH 2/2] Add named capture groups to traceCtxRegExp, avoid potential panic when injecting tracestate --- api/trace/trace_context_propagator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/trace/trace_context_propagator.go b/api/trace/trace_context_propagator.go index fde0ab536..ec6f147b4 100644 --- a/api/trace/trace_context_propagator.go +++ b/api/trace/trace_context_propagator.go @@ -42,7 +42,7 @@ const ( type TraceContext struct{} var _ propagation.HTTPPropagator = TraceContext{} -var traceCtxRegExp = regexp.MustCompile("^([0-9a-f]{2})-([a-f0-9]{32})-([a-f0-9]{16})-([a-f0-9]{2})(?:-.*)?$") +var traceCtxRegExp = regexp.MustCompile("^(?P[0-9a-f]{2})-(?P[a-f0-9]{32})-(?P[a-f0-9]{16})-(?P[a-f0-9]{2})(?:-.*)?$") // DefaultHTTPPropagator returns the default trace HTTP propagator. func DefaultHTTPPropagator() propagation.HTTPPropagator { @@ -51,8 +51,8 @@ func DefaultHTTPPropagator() propagation.HTTPPropagator { func (TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { tracestate := ctx.Value(tracestateKey) - if tracestate != nil { - supplier.Set(tracestateHeader, tracestate.(string)) + if state, ok := tracestate.(string); tracestate != nil && ok { + supplier.Set(tracestateHeader, state) } sc := SpanFromContext(ctx).SpanContext()