From ae11cd04b7789fa938bb4f0e696fd6bd76463fa4 Mon Sep 17 00:00:00 2001 From: Reiley Yang Date: Thu, 27 Sep 2018 10:25:11 -0700 Subject: [PATCH] Improve W3C Trace Context compliance (#934) --- .../propagation/tracecontext/propagation.go | 59 +++++++++++++------ .../tracecontext/propagation_test.go | 22 +++---- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/plugin/ochttp/propagation/tracecontext/propagation.go b/plugin/ochttp/propagation/tracecontext/propagation.go index f6faff079..65ab1e996 100644 --- a/plugin/ochttp/propagation/tracecontext/propagation.go +++ b/plugin/ochttp/propagation/tracecontext/propagation.go @@ -20,6 +20,7 @@ import ( "encoding/hex" "fmt" "net/http" + "net/textproto" "regexp" "strings" @@ -46,48 +47,54 @@ type HTTPFormat struct{} // SpanContextFromRequest extracts a span context from incoming requests. func (f *HTTPFormat) SpanContextFromRequest(req *http.Request) (sc trace.SpanContext, ok bool) { - h := req.Header.Get(traceparentHeader) - if h == "" { + h, ok := getRequestHeader(req, traceparentHeader, false) + if !ok { return trace.SpanContext{}, false } sections := strings.Split(h, "-") - if len(sections) < 3 { + if len(sections) < 4 { return trace.SpanContext{}, false } + if len(sections[0]) != 2 { + return trace.SpanContext{}, false + } ver, err := hex.DecodeString(sections[0]) if err != nil { return trace.SpanContext{}, false } - if len(ver) == 0 || int(ver[0]) > supportedVersion || int(ver[0]) > maxVersion { + version := int(ver[0]) + if version > maxVersion { return trace.SpanContext{}, false } - tid, err := hex.DecodeString(sections[1]) - if err != nil { + if version == 0 && len(sections) != 4 { return trace.SpanContext{}, false } - if len(tid) != 16 { + + if len(sections[1]) != 32 { + return trace.SpanContext{}, false + } + tid, err := hex.DecodeString(sections[1]) + if err != nil { return trace.SpanContext{}, false } copy(sc.TraceID[:], tid) - sid, err := hex.DecodeString(sections[2]) - if err != nil { + if len(sections[2]) != 16 { return trace.SpanContext{}, false } - if len(sid) != 8 { + sid, err := hex.DecodeString(sections[2]) + if err != nil { return trace.SpanContext{}, false } copy(sc.SpanID[:], sid) - if len(sections) == 4 { - opts, err := hex.DecodeString(sections[3]) - if err != nil || len(opts) < 1 { - return trace.SpanContext{}, false - } - sc.TraceOptions = trace.TraceOptions(opts[0]) + opts, err := hex.DecodeString(sections[3]) + if err != nil || len(opts) < 1 { + return trace.SpanContext{}, false } + sc.TraceOptions = trace.TraceOptions(opts[0]) // Don't allow all zero trace or span ID. if sc.TraceID == [16]byte{} || sc.SpanID == [8]byte{} { @@ -98,13 +105,31 @@ func (f *HTTPFormat) SpanContextFromRequest(req *http.Request) (sc trace.SpanCon return sc, true } +// getRequestHeader returns a combined header field according to RFC7230 section 3.2.2. +// If commaSeparated is true, multiple header fields with the same field name using be +// combined using ",". +// If no header was found using the given name, "ok" would be false. +// If more than one headers was found using the given name, while commaSeparated is false, +// "ok" would be false. +func getRequestHeader(req *http.Request, name string, commaSeparated bool) (hdr string, ok bool) { + v := req.Header[textproto.CanonicalMIMEHeaderKey(name)] + switch len(v) { + case 0: + return "", false + case 1: + return v[0], true + default: + return strings.Join(v, ","), commaSeparated + } +} + // TODO(rghetia): return an empty Tracestate when parsing tracestate header encounters an error. // Revisit to return additional boolean value to indicate parsing error when following issues // are resolved. // https://github.com/w3c/distributed-tracing/issues/172 // https://github.com/w3c/distributed-tracing/issues/175 func tracestateFromRequest(req *http.Request) *tracestate.Tracestate { - h := req.Header.Get(tracestateHeader) + h, _ := getRequestHeader(req, tracestateHeader, true) if h == "" { return nil } diff --git a/plugin/ochttp/propagation/tracecontext/propagation_test.go b/plugin/ochttp/propagation/tracecontext/propagation_test.go index 5b2a2d72d..996cfa883 100644 --- a/plugin/ochttp/propagation/tracecontext/propagation_test.go +++ b/plugin/ochttp/propagation/tracecontext/propagation_test.go @@ -48,10 +48,14 @@ func TestHTTPFormat_FromRequest(t *testing.T) { wantOk bool }{ { - name: "unsupported version", + name: "future version", header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - wantSc: trace.SpanContext{}, - wantOk: false, + wantSc: trace.SpanContext{ + TraceID: trace.TraceID{75, 249, 47, 53, 119, 179, 77, 166, 163, 206, 146, 157, 14, 14, 71, 54}, + SpanID: trace.SpanID{0, 240, 103, 170, 11, 169, 2, 183}, + TraceOptions: trace.TraceOptions(1), + }, + wantOk: true, }, { name: "zero trace ID and span ID", @@ -70,17 +74,13 @@ func TestHTTPFormat_FromRequest(t *testing.T) { wantOk: true, }, { - name: "no options", + name: "missing options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", - wantSc: trace.SpanContext{ - TraceID: trace.TraceID{75, 249, 47, 53, 119, 179, 77, 166, 163, 206, 146, 157, 14, 14, 71, 54}, - SpanID: trace.SpanID{0, 240, 103, 170, 11, 169, 2, 183}, - TraceOptions: trace.TraceOptions(0), - }, - wantOk: true, + wantSc: trace.SpanContext{}, + wantOk: false, }, { - name: "missing options", + name: "empty options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", wantSc: trace.SpanContext{}, wantOk: false,