From 5efea1c67297cf2fd68d217f6126177bd6d7337d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 30 Jun 2020 18:14:55 -0700 Subject: [PATCH 01/33] Correct B3 propagators and add tests --- api/trace/b3_propagator.go | 335 ++++++++++++-------- api/trace/b3_propagator_test.go | 523 ++++++++++++++++++++++++++++++++ 2 files changed, 733 insertions(+), 125 deletions(-) create mode 100644 api/trace/b3_propagator_test.go diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 6f5b15986ba..1fb89bc689a 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -16,7 +16,7 @@ package trace import ( "context" - "fmt" + "errors" "strings" "go.opentelemetry.io/otel/api/propagation" @@ -32,185 +32,270 @@ const ( b3TraceIDPadding = "0000000000000000" ) +var ( + empty = EmptySpanContext() + + errInvalidSampledByte = errors.New("invalid B3 Sampled found") + errInvalidSampledHeader = errors.New("invalid B3 Sampled header found") + errInvalidFlagsHeader = errors.New("invalid B3 Flags header found") + errInvalidTraceIDHeader = errors.New("invalid B3 TraceID header found") + errInvalidSpanIDHeader = errors.New("invalid B3 SpanID header found") + errInvalidParentSpanIDHeader = errors.New("invalid B3 ParentSpanID header found") + errInvalidScope = errors.New("require either both TraceID and SpanID or none") + errInvalidScopeParent = errors.New("ParentSpanID requires both TraceID and SpanID to be available") + errInvalidScopeParentSingle = errors.New("ParentSpanID requires TraceID, SpanID and Sampled to be available") + errEmptyContext = errors.New("empty request context") + errInvalidTraceIDValue = errors.New("invalid B3 TraceID value found") + errInvalidSpanIDValue = errors.New("invalid B3 SpanID value found") + errInvalidParentSpanIDValue = errors.New("invalid B3 ParentSpanID value found") +) + // B3 propagator serializes SpanContext to/from B3 Headers. // This propagator supports both version of B3 headers, // 1. Single Header : -// X-B3: {TraceId}-{SpanId}-{SamplingState}-{ParentSpanId} +// b3: {TraceId}-{SpanId}-{SamplingState}-{ParentSpanId} // 2. Multiple Headers: -// X-B3-TraceId: {TraceId} -// X-B3-ParentSpanId: {ParentSpanId} -// X-B3-SpanId: {SpanId} -// X-B3-Sampled: {SamplingState} -// X-B3-Flags: {DebugFlag} -// -// If SingleHeader is set to true then X-B3 header is used to inject and extract. Otherwise, -// separate headers are used to inject and extract. +// x-b3-traceid: {TraceId} +// x-b3-parentspanid: {ParentSpanId} +// x-b3-spanid: {SpanId} +// x-b3-sampled: {SamplingState} +// x-b3-flags: {DebugFlag} type B3 struct { + // SingleAndMultiHeader specifies if both the single and multiple + // headers should be included in the context injection. If this is + // `true` `SingleHeader` has no effect and the single header will be + // included regardless of its value. + SingleAndMultiHeader bool + + // SingleHeader specifies if the single header should be included in the + // context injection. SingleHeader bool } var _ propagation.HTTPPropagator = B3{} +// Inject injects a context into the supplier as B3 headers. func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { sc := SpanFromContext(ctx).SpanContext() if !sc.IsValid() { return } - if b3.SingleHeader { - sampled := sc.TraceFlags & FlagsSampled - supplier.Set(B3SingleHeader, - fmt.Sprintf("%s-%s-%.1d", sc.TraceID, sc.SpanID, sampled)) - } else { - supplier.Set(B3TraceIDHeader, sc.TraceID.String()) - supplier.Set(B3SpanIDHeader, sc.SpanID.String()) + if b3.SingleHeader || b3.SingleAndMultiHeader { + header := []string{} + if sc.TraceID.IsValid() && sc.SpanID.IsValid() { + header = append(header, sc.TraceID.String(), sc.SpanID.String()) + } - var sampled string - if sc.IsSampled() { - sampled = "1" - } else { - sampled = "0" + if sc.TraceFlags&FlagsUnused != FlagsUnused { + if sc.IsSampled() { + header = append(header, "1") + } else { + header = append(header, "0") + } + } + + supplier.Set(B3SingleHeader, strings.Join(header, "-")) + } + + if !b3.SingleHeader || b3.SingleAndMultiHeader { + if sc.TraceID.IsValid() && sc.SpanID.IsValid() { + supplier.Set(B3TraceIDHeader, sc.TraceID.String()) + supplier.Set(B3SpanIDHeader, sc.SpanID.String()) + } + + if sc.TraceFlags&FlagsUnused != FlagsUnused { + if sc.IsSampled() { + supplier.Set(B3SampledHeader, "1") + } else { + supplier.Set(B3SampledHeader, "0") + } } - supplier.Set(B3SampledHeader, sampled) } } -// Extract retrieves B3 Headers from the supplier +// Extract extracts a context from the supplier if it contains B3 headers. func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) context.Context { - var sc SpanContext - if b3.SingleHeader { - sc = b3.extractSingleHeader(supplier) - } else { - sc = b3.extract(supplier) + var ( + sc SpanContext + err error + ) + + if h := supplier.Get(B3SingleHeader); h != "" { + sc, err = extractSingle(h) + if err == nil && sc.IsValid() { + return ContextWithRemoteSpanContext(ctx, sc) + } } - if !sc.IsValid() { + + var ( + traceID = supplier.Get(B3TraceIDHeader) + spanID = supplier.Get(B3SpanIDHeader) + parentSpanID = supplier.Get(B3ParentSpanIDHeader) + sampled = supplier.Get(B3SampledHeader) + debugFlag = supplier.Get(B3DebugFlagHeader) + ) + sc, err = extractMultiple(traceID, spanID, parentSpanID, sampled, debugFlag) + if err != nil || !sc.IsValid() { return ctx } return ContextWithRemoteSpanContext(ctx, sc) } -func fixB3TID(in string) string { - if len(in) == 16 { - in = b3TraceIDPadding + in +func (b3 B3) GetAllKeys() []string { + if b3.SingleHeader { + return []string{B3SingleHeader} } - return in + return []string{B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader} } -func (b3 B3) extract(supplier propagation.HTTPSupplier) SpanContext { - tid, err := IDFromHex(fixB3TID(supplier.Get(B3TraceIDHeader))) - if err != nil { - return EmptySpanContext() - } - sid, err := SpanIDFromHex(supplier.Get(B3SpanIDHeader)) - if err != nil { - return EmptySpanContext() +// extractMultiple reconstructs a SpanContext from header values based on B3 +// Multiple header. It is based on the implementation found here: +// https://github.com/openzipkin/zipkin-go/blob/v0.2.2/propagation/b3/spancontext.go +// and adapted to support a SpanContext. +func extractMultiple(traceID, spanID, parentSpanID, sampled, flags string) (SpanContext, error) { + var ( + err error + requiredCount int + sc = SpanContext{} + ) + + // correct values for an existing sampled header are "0" and "1". + // For legacy support and being lenient to other tracing implementations we + // allow "true" and "false" as inputs for interop purposes. + switch strings.ToLower(sampled) { + case "0", "false": + case "1", "true": + sc.TraceFlags = FlagsSampled + case "": + sc.TraceFlags = FlagsUnused + default: + return empty, errInvalidSampledHeader } - sampled, ok := b3.extractSampledState(supplier.Get(B3SampledHeader)) - if !ok { - return EmptySpanContext() + + // The only accepted value for Flags is "1". This will set Debug to true. All + // other values and omission of header will be ignored. + if flags == "1" { + // We do not track debug status, but the sampling needs to be unset. + sc.TraceFlags = FlagsUnused } - debug, ok := b3.extracDebugFlag(supplier.Get(B3DebugFlagHeader)) - if !ok { - return EmptySpanContext() + if traceID != "" { + requiredCount++ + id := traceID + if len(traceID) == 16 { + // Pad 64-bit trace IDs. + id = b3TraceIDPadding + traceID + } + if sc.TraceID, err = IDFromHex(id); err != nil { + return empty, errInvalidTraceIDHeader + } } - if debug == FlagsSampled { - sampled = FlagsSampled + + if spanID != "" { + requiredCount++ + if sc.SpanID, err = SpanIDFromHex(spanID); err != nil { + return empty, errInvalidSpanIDHeader + } } - sc := SpanContext{ - TraceID: tid, - SpanID: sid, - TraceFlags: sampled, + if requiredCount != 0 && requiredCount != 2 { + return empty, errInvalidScope } - if !sc.IsValid() { - return EmptySpanContext() + if parentSpanID != "" { + if requiredCount == 0 { + return empty, errInvalidScopeParent + } + // Validate parent span ID but we do not use it so do not save it. + if _, err = SpanIDFromHex(parentSpanID); err != nil { + return empty, errInvalidParentSpanIDHeader + } } - return sc + return sc, nil } -func (b3 B3) extractSingleHeader(supplier propagation.HTTPSupplier) SpanContext { - h := supplier.Get(B3SingleHeader) - if h == "" || h == "0" { - return EmptySpanContext() - } - sc := SpanContext{} - parts := strings.Split(h, "-") - l := len(parts) - if l > 4 { - return EmptySpanContext() +// extractSingle reconstructs a SpanContext from contextHeader based on a B3 +// Single header. It is based on the implementation found here: +// https://github.com/openzipkin/zipkin-go/blob/v0.2.2/propagation/b3/spancontext.go +// and adapted to support a SpanContext. +func extractSingle(contextHeader string) (SpanContext, error) { + if contextHeader == "" { + return empty, errEmptyContext } - if l < 2 { - return EmptySpanContext() - } + var ( + sc = SpanContext{} + sampling string + ) - var err error - sc.TraceID, err = IDFromHex(fixB3TID(parts[0])) - if err != nil { - return EmptySpanContext() - } + headerLen := len(contextHeader) - sc.SpanID, err = SpanIDFromHex(parts[1]) - if err != nil { - return EmptySpanContext() - } - - if l > 2 { - var ok bool - sc.TraceFlags, ok = b3.extractSampledState(parts[2]) - if !ok { - return EmptySpanContext() + if headerLen == 1 { + sampling = contextHeader + } else if headerLen == 16 || headerLen == 32 { + return empty, errInvalidScope + } else if headerLen >= 16+16+1 { + pos := 0 + var traceID string + if string(contextHeader[16]) == "-" { + // traceID must be 64 bits + pos += 16 + 1 // {traceID}- + traceID = b3TraceIDPadding + string(contextHeader[0:16]) + } else if string(contextHeader[32]) == "-" { + // traceID must be 128 bits + pos += 32 + 1 // {traceID}- + traceID = string(contextHeader[0:32]) + } else { + return empty, errInvalidTraceIDValue } - } - if l == 4 { - _, err = SpanIDFromHex(parts[3]) + var err error + sc.TraceID, err = IDFromHex(traceID) if err != nil { - return EmptySpanContext() + return empty, errInvalidTraceIDValue } - } - if !sc.IsValid() { - return EmptySpanContext() - } + sc.SpanID, err = SpanIDFromHex(contextHeader[pos : pos+16]) + if err != nil { + return empty, errInvalidSpanIDValue + } + pos += 16 // {traceID}-{spanID} - return sc -} + if headerLen > pos { + if headerLen == pos+1 { + return empty, errInvalidSampledByte + } + pos += 1 // {traceID}-{spanID}- -// extractSampledState parses the value of the X-B3-Sampled b3Header. -func (b3 B3) extractSampledState(sampled string) (flag byte, ok bool) { - switch sampled { - case "", "0": - return 0, true - case "1": - return FlagsSampled, true - case "true": - if !b3.SingleHeader { - return FlagsSampled, true - } - case "d": - if b3.SingleHeader { - return FlagsSampled, true + if headerLen == pos+1 { + sampling = string(contextHeader[pos]) + } else if headerLen == pos+16 { + return empty, errInvalidScopeParentSingle + } else if headerLen == pos+1+16+1 { + sampling = string(contextHeader[pos]) + pos += 1 + 1 // {traceID}-{spanID}-{sampling}- + + // Validate parent span ID but we do not use it so do not + // save it. + _, err = SpanIDFromHex(contextHeader[pos:]) + if err != nil { + return empty, errInvalidParentSpanIDValue + } + } else { + return empty, errInvalidParentSpanIDValue + } } + } else { + return empty, errInvalidTraceIDValue } - return 0, false -} - -// extracDebugFlag parses the value of the X-B3-Sampled b3Header. -func (b3 B3) extracDebugFlag(debug string) (flag byte, ok bool) { - switch debug { - case "", "0": - return 0, true + switch sampling { case "1": - return FlagsSampled, true + sc.TraceFlags = FlagsSampled + case "", "d", "0": + // Valid but unsupported ("d"), or no-ops ("", "0"). + default: + return empty, errInvalidSampledByte } - return 0, false -} -func (b3 B3) GetAllKeys() []string { - if b3.SingleHeader { - return []string{B3SingleHeader} - } - return []string{B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader} + return sc, nil } diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go new file mode 100644 index 00000000000..32e329b5bf0 --- /dev/null +++ b/api/trace/b3_propagator_test.go @@ -0,0 +1,523 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trace + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +var ( + traceID = ID{0, 0, 0, 0, 0, 0, 0, 0x7b, 0, 0, 0, 0, 0, 0, 0x1, 0xc8} + traceIDStr = "000000000000007b00000000000001c8" + spanID = SpanID{0, 0, 0, 0, 0, 0, 0, 0x7b} + spanIDStr = "000000000000007b" +) + +type Supplier struct { + SingleHeader string + DebugFlagHeader string + TraceIDHeader string + SpanIDHeader string + SampledHeader string + ParentSpanIDHeader string +} + +func (s *Supplier) Get(key string) string { + switch key { + case B3SingleHeader: + return s.SingleHeader + case B3DebugFlagHeader: + return s.DebugFlagHeader + case B3TraceIDHeader: + return s.TraceIDHeader + case B3SpanIDHeader: + return s.SpanIDHeader + case B3SampledHeader: + return s.SampledHeader + case B3ParentSpanIDHeader: + return s.ParentSpanIDHeader + } + return "" +} + +func (s *Supplier) Set(key, value string) { + fmt.Println("called", key, value) + switch key { + case B3SingleHeader: + s.SingleHeader = value + case B3DebugFlagHeader: + s.DebugFlagHeader = value + case B3TraceIDHeader: + s.TraceIDHeader = value + case B3SpanIDHeader: + s.SpanIDHeader = value + case B3SampledHeader: + s.SampledHeader = value + case B3ParentSpanIDHeader: + s.ParentSpanIDHeader = value + } +} + +type TestSpan struct { + NoopSpan + sc SpanContext +} + +func (s TestSpan) SpanContext() SpanContext { + return s.sc +} + +func TestInject(t *testing.T) { + tests := []struct { + sc SpanContext + b3 B3 + expected *Supplier + }{ + { + sc: SpanContext{}, + b3: B3{}, + expected: &Supplier{}, + }, + { + sc: SpanContext{ + SpanID: spanID, + TraceFlags: FlagsSampled, + }, + b3: B3{}, + expected: &Supplier{}, + }, + { + sc: SpanContext{ + TraceID: traceID, + TraceFlags: FlagsSampled, + }, + b3: B3{}, + expected: &Supplier{}, + }, + { + sc: SpanContext{ + TraceFlags: FlagsSampled, + }, + b3: B3{}, + expected: &Supplier{}, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsSampled, + }, + b3: B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + b3: B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-0", + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsUnused, + }, + b3: B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b", + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsSampled, + }, + b3: B3{SingleHeader: true, SingleAndMultiHeader: true}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "1", + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsUnused, + }, + b3: B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + b3: B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "0", + }, + }, + { + sc: SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsSampled, + }, + b3: B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "1", + }, + }, + } + + for _, test := range tests { + ctx := ContextWithSpan(context.Background(), TestSpan{sc: test.sc}) + actual := new(Supplier) + test.b3.Inject(ctx, actual) + assert.Equal( + t, + test.expected, + actual, + "B3: %#v, SpanContext: %#v", test.b3, test.sc, + ) + } +} + +func TestExtract(t *testing.T) { + tests := []struct { + traceID string + spanID string + parentSpanID string + sampled string + flags string + single string + expected SpanContext + }{ + { + traceID: "", + spanID: "", + parentSpanID: "", + sampled: "", + flags: "", + single: "", + expected: empty, + }, + { + traceID: "", + spanID: "", + parentSpanID: "", + sampled: "", + flags: "", + single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", + expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + }, + { + traceID: traceIDStr, + spanID: spanIDStr, + parentSpanID: "00000000000001c8", + sampled: "1", + flags: "", + single: "", + expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + }, + { + traceID: traceIDStr, + spanID: spanIDStr, + parentSpanID: "00000000000001c8", + sampled: "1", + flags: "", + single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", + expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + }, + } + + b3 := B3{} + for _, test := range tests { + ctx := context.Background() + supplier := &Supplier{ + SingleHeader: test.single, + DebugFlagHeader: test.flags, + TraceIDHeader: test.traceID, + SpanIDHeader: test.spanID, + SampledHeader: test.sampled, + ParentSpanIDHeader: test.parentSpanID, + } + info := []interface{}{ + "trace ID: %q, span ID: %q, parent span ID: %q, sampled: %q, flags: %q, single: %q", + test.traceID, + test.spanID, + test.parentSpanID, + test.sampled, + test.flags, + test.single, + } + actual := RemoteSpanContextFromContext(b3.Extract(ctx, supplier)) + assert.Equal(t, test.expected, actual, info...) + } +} + +func TestExtractMultiple(t *testing.T) { + tests := []struct { + traceID string + spanID string + parentSpanID string + sampled string + flags string + expected SpanContext + err error + }{ + { + "", "", "", "0", "", + SpanContext{}, + nil, + }, + { + "", "", "", "", "", + SpanContext{TraceFlags: FlagsUnused}, + nil, + }, + { + "", "", "", "1", "", + SpanContext{TraceFlags: FlagsSampled}, + nil, + }, + { + traceIDStr, spanIDStr, "", "", "", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnused}, + nil, + }, + { + traceIDStr, spanIDStr, "", "0", "", + SpanContext{TraceID: traceID, SpanID: spanID}, + nil, + }, + // Ensure backwards compatibility. + { + traceIDStr, spanIDStr, "", "false", "", + SpanContext{TraceID: traceID, SpanID: spanID}, + nil, + }, + { + traceIDStr, spanIDStr, "", "1", "", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + nil, + }, + // Ensure backwards compatibility. + { + traceIDStr, spanIDStr, "", "true", "", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + nil, + }, + { + traceIDStr, spanIDStr, "", "a", "", + empty, + errInvalidSampledHeader, + }, + { + traceIDStr, spanIDStr, "", "1", "1", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnused}, + nil, + }, + // Invalid flags are discarded. + { + traceIDStr, spanIDStr, "", "1", "invalid", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + nil, + }, + // Support short trace IDs. + { + "00000000000001c8", spanIDStr, "", "0", "", + SpanContext{ + TraceID: ID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x1, 0xc8}, + SpanID: spanID, + }, + nil, + }, + { + "00000000000001c", spanIDStr, "", "0", "", + empty, + errInvalidTraceIDHeader, + }, + { + "00000000000001c80", spanIDStr, "", "0", "", + empty, + errInvalidTraceIDHeader, + }, + { + traceIDStr[:len(traceIDStr)-2], spanIDStr, "", "0", "", + empty, + errInvalidTraceIDHeader, + }, + { + traceIDStr + "0", spanIDStr, "", "0", "", + empty, + errInvalidTraceIDHeader, + }, + { + traceIDStr, "00000000000001c", "", "0", "", + empty, + errInvalidSpanIDHeader, + }, + { + traceIDStr, "00000000000001c80", "", "0", "", + empty, + errInvalidSpanIDHeader, + }, + { + traceIDStr, "", "", "0", "", + empty, + errInvalidScope, + }, + { + "", spanIDStr, "", "0", "", + empty, + errInvalidScope, + }, + { + "", "", spanIDStr, "0", "", + empty, + errInvalidScopeParent, + }, + { + traceIDStr, spanIDStr, "00000000000001c8", "0", "", + SpanContext{TraceID: traceID, SpanID: spanID}, + nil, + }, + { + traceIDStr, spanIDStr, "00000000000001c", "0", "", + empty, + errInvalidParentSpanIDHeader, + }, + { + traceIDStr, spanIDStr, "00000000000001c80", "0", "", + empty, + errInvalidParentSpanIDHeader, + }, + } + + for _, test := range tests { + actual, err := extractMultiple( + test.traceID, + test.spanID, + test.parentSpanID, + test.sampled, + test.flags, + ) + info := []interface{}{ + "trace ID: %q, span ID: %q, parent span ID: %q, sampled: %q, flags: %q", + test.traceID, + test.spanID, + test.parentSpanID, + test.sampled, + test.flags, + } + if !assert.Equal(t, test.err, err, info...) { + continue + } + assert.Equal(t, test.expected, actual, info...) + } +} + +func TestExtractSingle(t *testing.T) { + tests := []struct { + header string + expected SpanContext + err error + }{ + {"0", SpanContext{}, nil}, + {"1", SpanContext{TraceFlags: FlagsSampled}, nil}, + // debug flag is valid, but ignored. + {"d", SpanContext{}, nil}, + {"a", empty, errInvalidSampledByte}, + {"3", empty, errInvalidSampledByte}, + {"000000000000007b", empty, errInvalidScope}, + {"000000000000007b00000000000001c8", empty, errInvalidScope}, + // Support short trace IDs. + { + "00000000000001c8-000000000000007b", + SpanContext{ + TraceID: ID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x1, 0xc8}, + SpanID: spanID, + }, + nil, + }, + { + "000000000000007b00000000000001c8-000000000000007b", + SpanContext{TraceID: traceID, SpanID: spanID}, + nil, + }, + { + "000000000000007b00000000000001c8-000000000000007b-", + empty, + errInvalidSampledByte, + }, + { + "000000000000007b00000000000001c8-000000000000007b-3", + empty, + errInvalidSampledByte, + }, + { + "000000000000007b00000000000001c8-000000000000007b-00000000000001c8", + empty, + errInvalidScopeParentSingle, + }, + { + "000000000000007b00000000000001c8-000000000000007b-1", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + nil, + }, + // ParentSpanID is discarded, but should still restult in a parsable + // header. + { + "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, + nil, + }, + { + "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c", + empty, + errInvalidParentSpanIDValue, + }, + {"", empty, errEmptyContext}, + } + + for _, test := range tests { + actual, err := extractSingle(test.header) + if !assert.Equal(t, test.err, err, "header: %s", test.header) { + continue + } + assert.Equal(t, test.expected, actual, "header: %s", test.header) + } +} From 5b14cc4f5e9a52314ab701534788cdf39f8daee5 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 30 Jun 2020 18:20:35 -0700 Subject: [PATCH 02/33] Break up external integration and internal unit tests --- api/trace/b3_propagator_integration_test.go | 290 ++++++++++++++++++++ api/trace/b3_propagator_test.go | 261 ------------------ 2 files changed, 290 insertions(+), 261 deletions(-) create mode 100644 api/trace/b3_propagator_integration_test.go diff --git a/api/trace/b3_propagator_integration_test.go b/api/trace/b3_propagator_integration_test.go new file mode 100644 index 00000000000..449067ac4d4 --- /dev/null +++ b/api/trace/b3_propagator_integration_test.go @@ -0,0 +1,290 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trace_test + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/api/trace" +) + +var ( + traceID = trace.ID{0, 0, 0, 0, 0, 0, 0, 0x7b, 0, 0, 0, 0, 0, 0, 0x1, 0xc8} + traceIDStr = "000000000000007b00000000000001c8" + spanID = trace.SpanID{0, 0, 0, 0, 0, 0, 0, 0x7b} + spanIDStr = "000000000000007b" +) + +type Supplier struct { + SingleHeader string + DebugFlagHeader string + TraceIDHeader string + SpanIDHeader string + SampledHeader string + ParentSpanIDHeader string +} + +func (s *Supplier) Get(key string) string { + switch key { + case trace.B3SingleHeader: + return s.SingleHeader + case trace.B3DebugFlagHeader: + return s.DebugFlagHeader + case trace.B3TraceIDHeader: + return s.TraceIDHeader + case trace.B3SpanIDHeader: + return s.SpanIDHeader + case trace.B3SampledHeader: + return s.SampledHeader + case trace.B3ParentSpanIDHeader: + return s.ParentSpanIDHeader + } + return "" +} + +func (s *Supplier) Set(key, value string) { + fmt.Println("called", key, value) + switch key { + case trace.B3SingleHeader: + s.SingleHeader = value + case trace.B3DebugFlagHeader: + s.DebugFlagHeader = value + case trace.B3TraceIDHeader: + s.TraceIDHeader = value + case trace.B3SpanIDHeader: + s.SpanIDHeader = value + case trace.B3SampledHeader: + s.SampledHeader = value + case trace.B3ParentSpanIDHeader: + s.ParentSpanIDHeader = value + } +} + +type TestSpan struct { + trace.NoopSpan + sc trace.SpanContext +} + +func (s TestSpan) SpanContext() trace.SpanContext { + return s.sc +} + +func TestInject(t *testing.T) { + tests := []struct { + sc trace.SpanContext + b3 trace.B3 + expected *Supplier + }{ + { + sc: trace.SpanContext{}, + b3: trace.B3{}, + expected: &Supplier{}, + }, + { + sc: trace.SpanContext{ + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{}, + expected: &Supplier{}, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{}, + expected: &Supplier{}, + }, + { + sc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{}, + expected: &Supplier{}, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + b3: trace.B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-0", + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnused, + }, + b3: trace.B3{SingleHeader: true}, + expected: &Supplier{ + SingleHeader: "000000000000007b00000000000001c8-000000000000007b", + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{SingleHeader: true, SingleAndMultiHeader: true}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "1", + SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnused, + }, + b3: trace.B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + b3: trace.B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "0", + }, + }, + { + sc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + b3: trace.B3{}, + expected: &Supplier{ + TraceIDHeader: traceIDStr, + SpanIDHeader: spanIDStr, + SampledHeader: "1", + }, + }, + } + + for _, test := range tests { + ctx := trace.ContextWithSpan(context.Background(), TestSpan{sc: test.sc}) + actual := new(Supplier) + test.b3.Inject(ctx, actual) + assert.Equal( + t, + test.expected, + actual, + "B3: %#v, SpanContext: %#v", test.b3, test.sc, + ) + } +} + +func TestExtract(t *testing.T) { + tests := []struct { + traceID string + spanID string + parentSpanID string + sampled string + flags string + single string + expected trace.SpanContext + }{ + { + traceID: "", + spanID: "", + parentSpanID: "", + sampled: "", + flags: "", + single: "", + expected: trace.EmptySpanContext(), + }, + { + traceID: "", + spanID: "", + parentSpanID: "", + sampled: "", + flags: "", + single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", + expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, + }, + { + traceID: traceIDStr, + spanID: spanIDStr, + parentSpanID: "00000000000001c8", + sampled: "1", + flags: "", + single: "", + expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, + }, + { + traceID: traceIDStr, + spanID: spanIDStr, + parentSpanID: "00000000000001c8", + sampled: "1", + flags: "", + single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", + expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, + }, + } + + b3 := trace.B3{} + for _, test := range tests { + ctx := context.Background() + supplier := &Supplier{ + SingleHeader: test.single, + DebugFlagHeader: test.flags, + TraceIDHeader: test.traceID, + SpanIDHeader: test.spanID, + SampledHeader: test.sampled, + ParentSpanIDHeader: test.parentSpanID, + } + info := []interface{}{ + "trace ID: %q, span ID: %q, parent span ID: %q, sampled: %q, flags: %q, single: %q", + test.traceID, + test.spanID, + test.parentSpanID, + test.sampled, + test.flags, + test.single, + } + actual := trace.RemoteSpanContextFromContext(b3.Extract(ctx, supplier)) + assert.Equal(t, test.expected, actual, info...) + } +} diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go index 32e329b5bf0..753f97a408b 100644 --- a/api/trace/b3_propagator_test.go +++ b/api/trace/b3_propagator_test.go @@ -15,8 +15,6 @@ package trace import ( - "context" - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -29,265 +27,6 @@ var ( spanIDStr = "000000000000007b" ) -type Supplier struct { - SingleHeader string - DebugFlagHeader string - TraceIDHeader string - SpanIDHeader string - SampledHeader string - ParentSpanIDHeader string -} - -func (s *Supplier) Get(key string) string { - switch key { - case B3SingleHeader: - return s.SingleHeader - case B3DebugFlagHeader: - return s.DebugFlagHeader - case B3TraceIDHeader: - return s.TraceIDHeader - case B3SpanIDHeader: - return s.SpanIDHeader - case B3SampledHeader: - return s.SampledHeader - case B3ParentSpanIDHeader: - return s.ParentSpanIDHeader - } - return "" -} - -func (s *Supplier) Set(key, value string) { - fmt.Println("called", key, value) - switch key { - case B3SingleHeader: - s.SingleHeader = value - case B3DebugFlagHeader: - s.DebugFlagHeader = value - case B3TraceIDHeader: - s.TraceIDHeader = value - case B3SpanIDHeader: - s.SpanIDHeader = value - case B3SampledHeader: - s.SampledHeader = value - case B3ParentSpanIDHeader: - s.ParentSpanIDHeader = value - } -} - -type TestSpan struct { - NoopSpan - sc SpanContext -} - -func (s TestSpan) SpanContext() SpanContext { - return s.sc -} - -func TestInject(t *testing.T) { - tests := []struct { - sc SpanContext - b3 B3 - expected *Supplier - }{ - { - sc: SpanContext{}, - b3: B3{}, - expected: &Supplier{}, - }, - { - sc: SpanContext{ - SpanID: spanID, - TraceFlags: FlagsSampled, - }, - b3: B3{}, - expected: &Supplier{}, - }, - { - sc: SpanContext{ - TraceID: traceID, - TraceFlags: FlagsSampled, - }, - b3: B3{}, - expected: &Supplier{}, - }, - { - sc: SpanContext{ - TraceFlags: FlagsSampled, - }, - b3: B3{}, - expected: &Supplier{}, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: FlagsSampled, - }, - b3: B3{SingleHeader: true}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - }, - b3: B3{SingleHeader: true}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-0", - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: FlagsUnused, - }, - b3: B3{SingleHeader: true}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b", - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: FlagsSampled, - }, - b3: B3{SingleHeader: true, SingleAndMultiHeader: true}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "1", - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: FlagsUnused, - }, - b3: B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - }, - b3: B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "0", - }, - }, - { - sc: SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: FlagsSampled, - }, - b3: B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "1", - }, - }, - } - - for _, test := range tests { - ctx := ContextWithSpan(context.Background(), TestSpan{sc: test.sc}) - actual := new(Supplier) - test.b3.Inject(ctx, actual) - assert.Equal( - t, - test.expected, - actual, - "B3: %#v, SpanContext: %#v", test.b3, test.sc, - ) - } -} - -func TestExtract(t *testing.T) { - tests := []struct { - traceID string - spanID string - parentSpanID string - sampled string - flags string - single string - expected SpanContext - }{ - { - traceID: "", - spanID: "", - parentSpanID: "", - sampled: "", - flags: "", - single: "", - expected: empty, - }, - { - traceID: "", - spanID: "", - parentSpanID: "", - sampled: "", - flags: "", - single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", - expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, - }, - { - traceID: traceIDStr, - spanID: spanIDStr, - parentSpanID: "00000000000001c8", - sampled: "1", - flags: "", - single: "", - expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, - }, - { - traceID: traceIDStr, - spanID: spanIDStr, - parentSpanID: "00000000000001c8", - sampled: "1", - flags: "", - single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", - expected: SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled}, - }, - } - - b3 := B3{} - for _, test := range tests { - ctx := context.Background() - supplier := &Supplier{ - SingleHeader: test.single, - DebugFlagHeader: test.flags, - TraceIDHeader: test.traceID, - SpanIDHeader: test.spanID, - SampledHeader: test.sampled, - ParentSpanIDHeader: test.parentSpanID, - } - info := []interface{}{ - "trace ID: %q, span ID: %q, parent span ID: %q, sampled: %q, flags: %q, single: %q", - test.traceID, - test.spanID, - test.parentSpanID, - test.sampled, - test.flags, - test.single, - } - actual := RemoteSpanContextFromContext(b3.Extract(ctx, supplier)) - assert.Equal(t, test.expected, actual, info...) - } -} - func TestExtractMultiple(t *testing.T) { tests := []struct { traceID string From 0a7a3faac75e585833503b76d354e469e5bf7114 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 30 Jun 2020 18:30:27 -0700 Subject: [PATCH 03/33] Add changes to Changelog. --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 037d5043d27..de30b2284c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,14 +8,23 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- The B3 propagator can now be configured with the `SingleAndMultiple` `bool` field to inject both Single and Multiple B3 Headers into sent HTTP context. +- The B3 propagator and its extract and inject functionality is now tested. + ### Changed - Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879) - Use lowercase header names for B3 Multiple Headers. (#881) +- The B3 propagator now extract from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). + This is instead of only extracting based on the propagator's configuration. ### Fixed - The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881) +- The B3 propagator now correctly supports sampling only values for a Single B3 Header. +- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and correctly does not set a sampling value in this case when injecting. ## [0.7.0] - 2020-06-26 From a89f8e6fb5537b0bb47091b0460a3464d07ca226 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 30 Jun 2020 18:36:57 -0700 Subject: [PATCH 04/33] Update Changelog with PR number --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de30b2284c3..eca60dddd8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,21 +10,21 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- The B3 propagator can now be configured with the `SingleAndMultiple` `bool` field to inject both Single and Multiple B3 Headers into sent HTTP context. -- The B3 propagator and its extract and inject functionality is now tested. +- The B3 propagator can now be configured with the `SingleAndMultiple` `bool` field to inject both Single and Multiple B3 Headers into sent HTTP context. (#882) +- The B3 propagator and its extract and inject functionality is now tested. (#882) ### Changed - Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879) - Use lowercase header names for B3 Multiple Headers. (#881) - The B3 propagator now extract from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). - This is instead of only extracting based on the propagator's configuration. + This is instead of only extracting based on the propagator's configuration. (#882) ### Fixed - The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881) -- The B3 propagator now correctly supports sampling only values for a Single B3 Header. -- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and correctly does not set a sampling value in this case when injecting. +- The B3 propagator now correctly supports sampling only values for a Single B3 Header. (#882) +- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and correctly does not set a sampling value in this case when injecting. (#882) ## [0.7.0] - 2020-06-26 From bda1e3efaa1ed42e15632531b20db9aea682742c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:11:42 -0700 Subject: [PATCH 05/33] Fix lint issues --- api/trace/b3_propagator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 87b330f0590..e2c34aeeae4 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -39,7 +39,6 @@ var ( errInvalidSampledByte = errors.New("invalid B3 Sampled found") errInvalidSampledHeader = errors.New("invalid B3 Sampled header found") - errInvalidFlagsHeader = errors.New("invalid B3 Flags header found") errInvalidTraceIDHeader = errors.New("invalid B3 TraceID header found") errInvalidSpanIDHeader = errors.New("invalid B3 SpanID header found") errInvalidParentSpanIDHeader = errors.New("invalid B3 ParentSpanID header found") @@ -267,7 +266,7 @@ func extractSingle(contextHeader string) (SpanContext, error) { if headerLen == pos+1 { return empty, errInvalidSampledByte } - pos += 1 // {traceID}-{spanID}- + pos++ // {traceID}-{spanID}- if headerLen == pos+1 { sampling = string(contextHeader[pos]) From f16dc0c40a653e88efadd71bd5b9b298d3a3ad5a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:12:15 -0700 Subject: [PATCH 06/33] Update trace flags Add a new "not sampled" mask to complement the existing "sampled" one. Rename `FlagsUnused` to `FlagsUnset`. Add documentation for each of the flags to help understand their purpose. --- CHANGELOG.md | 3 +++ api/trace/b3_propagator.go | 9 +++++---- api/trace/b3_propagator_integration_test.go | 4 ++-- api/trace/b3_propagator_test.go | 6 +++--- api/trace/span_context.go | 18 +++++++++++++----- api/trace/span_context_test.go | 2 +- api/trace/trace_context_propagator.go | 2 +- 7 files changed, 28 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eca60dddd8e..48d4e92a161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The B3 propagator can now be configured with the `SingleAndMultiple` `bool` field to inject both Single and Multiple B3 Headers into sent HTTP context. (#882) - The B3 propagator and its extract and inject functionality is now tested. (#882) +- The `FlagsNotSampled` trace flag to complement the existing `FlagsSampled`. (#882) ### Changed @@ -19,6 +20,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use lowercase header names for B3 Multiple Headers. (#881) - The B3 propagator now extract from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). This is instead of only extracting based on the propagator's configuration. (#882) +- Rename `FlagsUnused` to `FlagsUnset` and clearly defined its purpose to act as a placeholder for systems that have a trinary sampling state (i.e. sample, don't sample, unspecified) to act as the unset state. (#882) + ### Fixed diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index e2c34aeeae4..0f95d738e15 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -87,7 +87,7 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { header = append(header, sc.TraceID.String(), sc.SpanID.String()) } - if sc.TraceFlags&FlagsUnused != FlagsUnused { + if sc.TraceFlags&FlagsUnset != FlagsUnset { if sc.IsSampled() { header = append(header, "1") } else { @@ -104,7 +104,7 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { supplier.Set(B3SpanIDHeader, sc.SpanID.String()) } - if sc.TraceFlags&FlagsUnused != FlagsUnused { + if sc.TraceFlags&FlagsUnset != FlagsUnset { if sc.IsSampled() { supplier.Set(B3SampledHeader, "1") } else { @@ -165,10 +165,11 @@ func extractMultiple(traceID, spanID, parentSpanID, sampled, flags string) (Span // allow "true" and "false" as inputs for interop purposes. switch strings.ToLower(sampled) { case "0", "false": + sc.TraceFlags = FlagsNotSampled case "1", "true": sc.TraceFlags = FlagsSampled case "": - sc.TraceFlags = FlagsUnused + sc.TraceFlags = FlagsUnset default: return empty, errInvalidSampledHeader } @@ -177,7 +178,7 @@ func extractMultiple(traceID, spanID, parentSpanID, sampled, flags string) (Span // other values and omission of header will be ignored. if flags == "1" { // We do not track debug status, but the sampling needs to be unset. - sc.TraceFlags = FlagsUnused + sc.TraceFlags = FlagsUnset } if traceID != "" { diff --git a/api/trace/b3_propagator_integration_test.go b/api/trace/b3_propagator_integration_test.go index 449067ac4d4..ce8739cf5e1 100644 --- a/api/trace/b3_propagator_integration_test.go +++ b/api/trace/b3_propagator_integration_test.go @@ -143,7 +143,7 @@ func TestInject(t *testing.T) { sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnused, + TraceFlags: trace.FlagsUnset, }, b3: trace.B3{SingleHeader: true}, expected: &Supplier{ @@ -168,7 +168,7 @@ func TestInject(t *testing.T) { sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnused, + TraceFlags: trace.FlagsUnset, }, b3: trace.B3{}, expected: &Supplier{ diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go index 753f97a408b..66cb3a98893 100644 --- a/api/trace/b3_propagator_test.go +++ b/api/trace/b3_propagator_test.go @@ -44,7 +44,7 @@ func TestExtractMultiple(t *testing.T) { }, { "", "", "", "", "", - SpanContext{TraceFlags: FlagsUnused}, + SpanContext{TraceFlags: FlagsUnset}, nil, }, { @@ -54,7 +54,7 @@ func TestExtractMultiple(t *testing.T) { }, { traceIDStr, spanIDStr, "", "", "", - SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnused}, + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnset}, nil, }, { @@ -86,7 +86,7 @@ func TestExtractMultiple(t *testing.T) { }, { traceIDStr, spanIDStr, "", "1", "1", - SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnused}, + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnset}, nil, }, // Invalid flags are discarded. diff --git a/api/trace/span_context.go b/api/trace/span_context.go index b15975568bb..8af0298efc5 100644 --- a/api/trace/span_context.go +++ b/api/trace/span_context.go @@ -21,13 +21,21 @@ import ( ) const ( - traceFlagsBitMaskSampled = byte(0x01) - traceFlagsBitMaskUnused = byte(0xFE) + traceFlagsBitMaskNotSampled = byte(0x00) + traceFlagsBitMaskSampled = byte(0x01) + traceFlagsBitMaskUnset = byte(0xFE) - // FlagsSampled is a byte with sampled bit set. It is a convenient value initializer - // for SpanContext TraceFlags field when a trace is sampled. + // FlagsSampled is a byte with the sampled bit set. FlagsSampled = traceFlagsBitMaskSampled - FlagsUnused = traceFlagsBitMaskUnused + // FlagsNotSampled is a byte with the sampled bit unset. + FlagsNotSampled = traceFlagsBitMaskNotSampled + // FlagsUnset is a byte with the sampled bit unset but all other bits + // set (the inverse of FlagsSampled). Some systems (e.g. B3) distinguish + // between setting flags to not sample and not making a sampling + // decision, usually called a deferred decision. This byte is used + // internally to represent this unset state. It is not a valid + // representation elsewhere. + FlagsUnset = traceFlagsBitMaskUnset ErrInvalidHexID errorConst = "trace-id and span-id can only contain [0-9a-f] characters, all lowercase" diff --git a/api/trace/span_context_test.go b/api/trace/span_context_test.go index 74a314bd39a..9787e92db9c 100644 --- a/api/trace/span_context_test.go +++ b/api/trace/span_context_test.go @@ -176,7 +176,7 @@ func TestSpanContextIsSampled(t *testing.T) { name: "sampled plus unused", sc: trace.SpanContext{ TraceID: trace.ID([16]byte{1}), - TraceFlags: trace.FlagsSampled | trace.FlagsUnused, + TraceFlags: trace.FlagsSampled | trace.FlagsUnset, }, want: true, }, { diff --git a/api/trace/trace_context_propagator.go b/api/trace/trace_context_propagator.go index d8263745ef8..daac8036cb4 100644 --- a/api/trace/trace_context_propagator.go +++ b/api/trace/trace_context_propagator.go @@ -137,7 +137,7 @@ func (TraceContext) extract(supplier propagation.HTTPSupplier) SpanContext { if err != nil || len(opts) < 1 || (version == 0 && opts[0] > 2) { return EmptySpanContext() } - sc.TraceFlags = opts[0] &^ FlagsUnused + sc.TraceFlags = opts[0] &^ FlagsUnset if !sc.IsValid() { return EmptySpanContext() From a96d02f33e52799cfa633a8c4d5fc19007fc2e6c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:22:52 -0700 Subject: [PATCH 07/33] Update extractSingle to support unset sampling --- api/trace/b3_propagator.go | 6 ++++-- api/trace/b3_propagator_integration_test.go | 1 + api/trace/b3_propagator_test.go | 17 +++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 0f95d738e15..fe6c4fcf6f0 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -291,10 +291,12 @@ func extractSingle(contextHeader string) (SpanContext, error) { return empty, errInvalidTraceIDValue } switch sampling { + case "", "d": + sc.TraceFlags = FlagsUnset case "1": sc.TraceFlags = FlagsSampled - case "", "d", "0": - // Valid but unsupported ("d"), or no-ops ("", "0"). + case "0": + sc.TraceFlags = FlagsNotSampled default: return empty, errInvalidSampledByte } diff --git a/api/trace/b3_propagator_integration_test.go b/api/trace/b3_propagator_integration_test.go index ce8739cf5e1..680d4d56afe 100644 --- a/api/trace/b3_propagator_integration_test.go +++ b/api/trace/b3_propagator_integration_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/api/trace" ) diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go index 66cb3a98893..fecfbfeb676 100644 --- a/api/trace/b3_propagator_test.go +++ b/api/trace/b3_propagator_test.go @@ -195,10 +195,10 @@ func TestExtractSingle(t *testing.T) { expected SpanContext err error }{ - {"0", SpanContext{}, nil}, + {"0", SpanContext{TraceFlags: FlagsNotSampled}, nil}, {"1", SpanContext{TraceFlags: FlagsSampled}, nil}, - // debug flag is valid, but ignored. - {"d", SpanContext{}, nil}, + // debug flag is valid, but ignored. Sampling should be deferred. + {"d", SpanContext{TraceFlags: FlagsUnset}, nil}, {"a", empty, errInvalidSampledByte}, {"3", empty, errInvalidSampledByte}, {"000000000000007b", empty, errInvalidScope}, @@ -207,14 +207,19 @@ func TestExtractSingle(t *testing.T) { { "00000000000001c8-000000000000007b", SpanContext{ - TraceID: ID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x1, 0xc8}, - SpanID: spanID, + TraceID: ID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x1, 0xc8}, + SpanID: spanID, + TraceFlags: FlagsUnset, }, nil, }, { "000000000000007b00000000000001c8-000000000000007b", - SpanContext{TraceID: traceID, SpanID: spanID}, + SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: FlagsUnset, + }, nil, }, { From 6c6a7b29062c778759d1c9813dfb3e18764f7692 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:27:32 -0700 Subject: [PATCH 08/33] Update existing tests to appropriately use FlagsUnset --- .../testtrace/b3_propagator_data_test.go | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index 432ce4995d9..c26bcb46807 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -36,8 +36,9 @@ var extractMultipleHeaders = []extractTest{ trace.B3SpanIDHeader: "00f067aa0ba902b7", }, wantSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, }, }, { @@ -88,13 +89,13 @@ var extractMultipleHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsSampled, + TraceFlags: trace.FlagsUnset, }, }, { - // spec is not clear on the behavior for this case. If debug flag is set - // then sampled state should not be set. From that perspective debug - // takes precedence. Hence, it is sampled. + // spec explictly states "Debug implies an accept decision, so don't + // also send the X-B3-Sampled header", make sure sampling is + // deferred. name: "debug flag set and sampling state is deny", headers: map[string]string{ trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", @@ -105,7 +106,7 @@ var extractMultipleHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsSampled, + TraceFlags: trace.FlagsUnset, }, }, { @@ -136,8 +137,9 @@ var extractMultipleHeaders = []extractTest{ trace.B3SpanIDHeader: "00f067aa0ba902b7", }, wantSc: trace.SpanContext{ - TraceID: traceID64bitPadded, - SpanID: spanID, + TraceID: traceID64bitPadded, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, }, }, } @@ -149,8 +151,9 @@ var extractSingleHeader = []extractTest{ trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", }, wantSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, }, }, { @@ -182,7 +185,7 @@ var extractSingleHeader = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsSampled, + TraceFlags: trace.FlagsUnset, }, }, { @@ -209,8 +212,9 @@ var extractSingleHeader = []extractTest{ trace.B3SingleHeader: "a3ce929d0e0e4736-00f067aa0ba902b7", }, wantSc: trace.SpanContext{ - TraceID: traceID64bitPadded, - SpanID: spanID, + TraceID: traceID64bitPadded, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, }, }, } From 405be1ae68e5132c2b4cb699513ac8b060c4ac52 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:28:32 -0700 Subject: [PATCH 09/33] Remove bogus debug flag test The B3 specification states "Debug is encoded as `X-B3-Flags: 1`. Absent or any other values can be ignored", so testing of other values should not result in an error. --- api/trace/testtrace/b3_propagator_data_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index c26bcb46807..5f95297ff36 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -284,24 +284,6 @@ var extractInvalidB3MultipleHeaders = []extractTest{ trace.B3SampledHeader: "d", }, }, - { - name: "bogus debug flag (string)", - headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", - trace.B3DebugFlagHeader: "d", - }, - }, - { - name: "bogus debug flag (number)", - headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", - trace.B3DebugFlagHeader: "10", - }, - }, { name: "upper case trace ID", headers: map[string]string{ From be56698db8fb0f7eba2658d209e49985a0057f94 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:55:10 -0700 Subject: [PATCH 10/33] B3 Extract now supports parsing both headers Remove test cases that would fail if the fallback header format was expected to not be used. --- .../testtrace/b3_propagator_data_test.go | 58 ------------------- 1 file changed, 58 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index 5f95297ff36..2915d00fcc1 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -330,14 +330,6 @@ var extractInvalidB3MultipleHeaders = []extractTest{ trace.B3SampledHeader: "1", }, }, - { - name: "missing trace ID with valid single header", - headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", - }, - }, { name: "sampled header set to 1 but trace ID and span ID are missing", headers: map[string]string{ @@ -419,23 +411,6 @@ var extractInvalidB3SingleHeader = []extractTest{ trace.B3SingleHeader: "00000000000000000000000000000000-0000000000000000-1", }, }, - { - name: "missing single header with valid separate headers", - headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", - trace.B3SampledHeader: "1", - }, - }, - { - name: "upper case span ID with valid separate headers", - headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-CD00000000000000-1", - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", - trace.B3SampledHeader: "1", - }, - }, { name: "with sampling set to true", headers: map[string]string{ @@ -483,22 +458,6 @@ var injectB3MultipleHeader = []injectTest{ trace.B3ParentSpanIDHeader, }, }, - { - name: "valid spancontext, with unsupported bit set in traceflags", - parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: 0xff, - }, - wantHeaders: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "0000000000000003", - trace.B3SampledHeader: "1", - }, - doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - }, - }, } var injectB3SingleleHeader = []injectTest{ @@ -535,21 +494,4 @@ var injectB3SingleleHeader = []injectTest{ trace.B3ParentSpanIDHeader, }, }, - { - name: "valid spancontext, with unsupported bit set in traceflags", - parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: 0xff, - }, - wantHeaders: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-0000000000000003-1", - }, - doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - }, - }, } From e7d5ed7ed4f294abdc8fccc39969011986c17c09 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 10:58:02 -0700 Subject: [PATCH 11/33] Feedback --- CHANGELOG.md | 2 +- api/trace/b3_propagator.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d4e92a161..4f343a12988 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879) - Use lowercase header names for B3 Multiple Headers. (#881) -- The B3 propagator now extract from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). +- The B3 propagator now extracts from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). This is instead of only extracting based on the propagator's configuration. (#882) - Rename `FlagsUnused` to `FlagsUnset` and clearly defined its purpose to act as a placeholder for systems that have a trinary sampling state (i.e. sample, don't sample, unspecified) to act as the unset state. (#882) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index fe6c4fcf6f0..575a2b1fd88 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -52,8 +52,8 @@ var ( ) // B3 propagator serializes SpanContext to/from B3 Headers. -// This propagator supports both version of B3 headers, -// 1. Single Header : +// This propagator supports both versions of B3 headers, +// 1. Single Header: // b3: {TraceId}-{SpanId}-{SamplingState}-{ParentSpanId} // 2. Multiple Headers: // x-b3-traceid: {TraceId} From 3f1ea36a97d61fddfcc5478a9c088027b9ffc3ee Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 12:41:42 -0700 Subject: [PATCH 12/33] Switch to bitmask inject encoding field Add the B3Encoding and valid HTTP based values. Change the B3 propagator to use these bitmask fields to specify the inject encoding it will propagate. --- CHANGELOG.md | 6 ++- api/trace/b3_propagator.go | 36 ++++++++----- api/trace/b3_propagator_integration_test.go | 8 +-- .../testtrace/b3_propagator_benchmark_test.go | 49 ++++++++--------- .../testtrace/b3_propagator_data_test.go | 2 +- api/trace/testtrace/b3_propagator_test.go | 53 +++++++++---------- api/trace/testtrace/propagator_test.go | 5 +- 7 files changed, 84 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f343a12988..41002d9d038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,14 +10,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- The B3 propagator can now be configured with the `SingleAndMultiple` `bool` field to inject both Single and Multiple B3 Headers into sent HTTP context. (#882) -- The B3 propagator and its extract and inject functionality is now tested. (#882) +- The `B3Encoding` is added to represent the B3 encoding(s) the B3 propagator injects. + A value for HTTP supported encodings (Multiple Header and Single Header) are included. - The `FlagsNotSampled` trace flag to complement the existing `FlagsSampled`. (#882) ### Changed - Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879) - Use lowercase header names for B3 Multiple Headers. (#881) +- The B3 propagator `SingleHeader` field has been replaced with `InjectEncoding`. + This new field can be set to combinations of the `B3Encoding` bitmasks and will inject trace information in these encodings. (#882) - The B3 propagator now extracts from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). This is instead of only extracting based on the propagator's configuration. (#882) - Rename `FlagsUnused` to `FlagsUnset` and clearly defined its purpose to act as a placeholder for systems that have a trinary sampling state (i.e. sample, don't sample, unspecified) to act as the unset state. (#882) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 575a2b1fd88..f81cfc99721 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -51,6 +51,18 @@ var ( errInvalidParentSpanIDValue = errors.New("invalid B3 ParentSpanID value found") ) +// B3Encoding is a bitmask representation of the B3 encoding type. +type B3Encoding uint8 + +const ( + // MultipleHeader is a B3 encoding that uses multiple headers to + // transmit tracing information all prefixed with `x-b3-`. + MultipleHeader B3Encoding = 1 + // SingleHeader is a B3 encoding that uses a single header named `b3 to + // transmit tracing information. + SingleHeader B3Encoding = 2 +) + // B3 propagator serializes SpanContext to/from B3 Headers. // This propagator supports both versions of B3 headers, // 1. Single Header: @@ -62,15 +74,14 @@ var ( // x-b3-sampled: {SamplingState} // x-b3-flags: {DebugFlag} type B3 struct { - // SingleAndMultiHeader specifies if both the single and multiple - // headers should be included in the context injection. If this is - // `true` `SingleHeader` has no effect and the single header will be - // included regardless of its value. - SingleAndMultiHeader bool - - // SingleHeader specifies if the single header should be included in the - // context injection. - SingleHeader bool + // InjectEncoding are the B3 encodings used when injecting trace + // information. If no encoding is specific it defaults to + // `MultipleHeader`. + InjectEncoding B3Encoding +} + +func (b3 B3) supports(e B3Encoding) bool { + return b3.InjectEncoding&e != 0 } var _ propagation.HTTPPropagator = B3{} @@ -81,7 +92,8 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { if !sc.IsValid() { return } - if b3.SingleHeader || b3.SingleAndMultiHeader { + + if b3.supports(SingleHeader) { header := []string{} if sc.TraceID.IsValid() && sc.SpanID.IsValid() { header = append(header, sc.TraceID.String(), sc.SpanID.String()) @@ -98,7 +110,7 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { supplier.Set(B3SingleHeader, strings.Join(header, "-")) } - if !b3.SingleHeader || b3.SingleAndMultiHeader { + if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { if sc.TraceID.IsValid() && sc.SpanID.IsValid() { supplier.Set(B3TraceIDHeader, sc.TraceID.String()) supplier.Set(B3SpanIDHeader, sc.SpanID.String()) @@ -143,7 +155,7 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con } func (b3 B3) GetAllKeys() []string { - if b3.SingleHeader { + if b3.supports(SingleHeader) { return []string{B3SingleHeader} } return []string{B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader} diff --git a/api/trace/b3_propagator_integration_test.go b/api/trace/b3_propagator_integration_test.go index 680d4d56afe..2a92e187fe0 100644 --- a/api/trace/b3_propagator_integration_test.go +++ b/api/trace/b3_propagator_integration_test.go @@ -125,7 +125,7 @@ func TestInject(t *testing.T) { SpanID: spanID, TraceFlags: trace.FlagsSampled, }, - b3: trace.B3{SingleHeader: true}, + b3: trace.B3{InjectEncoding: trace.SingleHeader}, expected: &Supplier{ SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", }, @@ -135,7 +135,7 @@ func TestInject(t *testing.T) { TraceID: traceID, SpanID: spanID, }, - b3: trace.B3{SingleHeader: true}, + b3: trace.B3{InjectEncoding: trace.SingleHeader}, expected: &Supplier{ SingleHeader: "000000000000007b00000000000001c8-000000000000007b-0", }, @@ -146,7 +146,7 @@ func TestInject(t *testing.T) { SpanID: spanID, TraceFlags: trace.FlagsUnset, }, - b3: trace.B3{SingleHeader: true}, + b3: trace.B3{InjectEncoding: trace.SingleHeader}, expected: &Supplier{ SingleHeader: "000000000000007b00000000000001c8-000000000000007b", }, @@ -157,7 +157,7 @@ func TestInject(t *testing.T) { SpanID: spanID, TraceFlags: trace.FlagsSampled, }, - b3: trace.B3{SingleHeader: true, SingleAndMultiHeader: true}, + b3: trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, expected: &Supplier{ TraceIDHeader: traceIDStr, SpanIDHeader: spanIDStr, diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index bbd3e67f05e..83ffa7d06b9 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/api/trace/testtrace/b3_propagator_benchmark_test.go @@ -25,34 +25,32 @@ import ( func BenchmarkExtractB3(b *testing.B) { testGroup := []struct { - singleHeader bool - name string - tests []extractTest + encoding trace.B3Encoding + name string + tests []extractTest }{ { - singleHeader: false, - name: "multiple headers", - tests: extractMultipleHeaders, + name: "multiple headers", + tests: extractMultipleHeaders, }, { - singleHeader: true, - name: "single headers", - tests: extractSingleHeader, + encoding: trace.SingleHeader, + name: "single headers", + tests: extractSingleHeader, }, { - singleHeader: false, - name: "invalid multiple headers", - tests: extractInvalidB3MultipleHeaders, + name: "invalid multiple headers", + tests: extractInvalidB3MultipleHeaders, }, { - singleHeader: true, - name: "invalid single headers", - tests: extractInvalidB3SingleHeader, + encoding: trace.SingleHeader, + name: "invalid single headers", + tests: extractInvalidB3SingleHeader, }, } for _, tg := range testGroup { - propagator := trace.B3{SingleHeader: tg.singleHeader} + propagator := trace.B3{InjectEncoding: tg.encoding} for _, tt := range tg.tests { traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) { ctx := context.Background() @@ -73,19 +71,18 @@ func BenchmarkExtractB3(b *testing.B) { func BenchmarkInjectB3(b *testing.B) { var id uint64 testGroup := []struct { - singleHeader bool - name string - tests []injectTest + encoding trace.B3Encoding + name string + tests []injectTest }{ { - singleHeader: false, - name: "multiple headers", - tests: injectB3MultipleHeader, + name: "multiple headers", + tests: injectB3MultipleHeader, }, { - singleHeader: true, - name: "single headers", - tests: injectB3SingleleHeader, + encoding: trace.SingleHeader, + name: "single headers", + tests: injectB3SingleleHeader, }, } @@ -96,7 +93,7 @@ func BenchmarkInjectB3(b *testing.B) { for _, tg := range testGroup { id = 0 - propagator := trace.B3{SingleHeader: tg.singleHeader} + propagator := trace.B3{InjectEncoding: tg.encoding} for _, tt := range tg.tests { traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) { req, _ := http.NewRequest("GET", "http://example.com", nil) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index 2915d00fcc1..eac0eb5d039 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -93,7 +93,7 @@ var extractMultipleHeaders = []extractTest{ }, }, { - // spec explictly states "Debug implies an accept decision, so don't + // spec explicitly states "Debug implies an accept decision, so don't // also send the X-B3-Sampled header", make sure sampling is // deferred. name: "debug flag set and sampling state is deny", diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 8aba57a6127..1db181d4a61 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -28,34 +28,32 @@ import ( func TestExtractB3(t *testing.T) { testGroup := []struct { - singleHeader bool - name string - tests []extractTest + encoding trace.B3Encoding + name string + tests []extractTest }{ { - singleHeader: false, - name: "multiple headers", - tests: extractMultipleHeaders, + name: "multiple headers", + tests: extractMultipleHeaders, }, { - singleHeader: true, - name: "single headers", - tests: extractSingleHeader, + encoding: trace.SingleHeader, + name: "single headers", + tests: extractSingleHeader, }, { - singleHeader: false, - name: "invalid multiple headers", - tests: extractInvalidB3MultipleHeaders, + name: "invalid multiple headers", + tests: extractInvalidB3MultipleHeaders, }, { - singleHeader: true, - name: "invalid single headers", - tests: extractInvalidB3SingleHeader, + encoding: trace.SingleHeader, + name: "invalid single headers", + tests: extractInvalidB3SingleHeader, }, } for _, tg := range testGroup { - propagator := trace.B3{SingleHeader: tg.singleHeader} + propagator := trace.B3{InjectEncoding: tg.encoding} props := propagation.New(propagation.WithExtractors(propagator)) for _, tt := range tg.tests { @@ -79,19 +77,18 @@ func TestExtractB3(t *testing.T) { func TestInjectB3(t *testing.T) { var id uint64 testGroup := []struct { - singleHeader bool - name string - tests []injectTest + encoding trace.B3Encoding + name string + tests []injectTest }{ { - singleHeader: false, - name: "multiple headers", - tests: injectB3MultipleHeader, + name: "multiple headers", + tests: injectB3MultipleHeader, }, { - singleHeader: true, - name: "single headers", - tests: injectB3SingleleHeader, + encoding: trace.SingleHeader, + name: "single headers", + tests: injectB3SingleleHeader, }, } @@ -102,7 +99,7 @@ func TestInjectB3(t *testing.T) { for _, tg := range testGroup { id = 0 - propagator := trace.B3{SingleHeader: tg.singleHeader} + propagator := trace.B3{InjectEncoding: tg.encoding} props := propagation.New(propagation.WithInjectors(propagator)) for _, tt := range tg.tests { t.Run(tt.name, func(t *testing.T) { @@ -132,7 +129,7 @@ func TestInjectB3(t *testing.T) { } func TestB3Propagator_GetAllKeys(t *testing.T) { - propagator := trace.B3{SingleHeader: false} + propagator := trace.B3{} want := []string{ trace.B3TraceIDHeader, trace.B3SpanIDHeader, @@ -145,7 +142,7 @@ func TestB3Propagator_GetAllKeys(t *testing.T) { } func TestB3PropagatorWithSingleHeader_GetAllKeys(t *testing.T) { - propagator := trace.B3{SingleHeader: true} + propagator := trace.B3{InjectEncoding: trace.SingleHeader} want := []string{ trace.B3SingleHeader, } diff --git a/api/trace/testtrace/propagator_test.go b/api/trace/testtrace/propagator_test.go index 09b75bcc853..43a620bfbb3 100644 --- a/api/trace/testtrace/propagator_test.go +++ b/api/trace/testtrace/propagator_test.go @@ -66,8 +66,9 @@ func TestMultiplePropagators(t *testing.T) { ns := nilSupplier{} testProps := []propagation.HTTPPropagator{ trace.TraceContext{}, - trace.B3{SingleHeader: false}, - trace.B3{SingleHeader: true}, + trace.B3{}, + trace.B3{InjectEncoding: trace.SingleHeader}, + trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, } bg := context.Background() // sanity check of oota propagator, ensuring that it really From 614e3c175c89ca48ab28673c7bb1886661ab1a61 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 12:49:19 -0700 Subject: [PATCH 13/33] Add comments --- api/trace/b3_propagator.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index f81cfc99721..10ff658a841 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -133,11 +133,13 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con err error ) + // Default to Single Header if a valid value exists. if h := supplier.Get(B3SingleHeader); h != "" { sc, err = extractSingle(h) if err == nil && sc.IsValid() { return ContextWithRemoteSpanContext(ctx, sc) } + // The Single Header value was invalid, fallback to Multiple Header. } var ( From 10c345b3bc14f00cceaf0306895ca62fa87b7437 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 17:07:04 -0700 Subject: [PATCH 14/33] Migrate B3 integration tests to existing testtrace --- api/trace/b3_propagator_integration_test.go | 291 ----------- .../testtrace/b3_propagator_benchmark_test.go | 41 +- .../testtrace/b3_propagator_data_test.go | 468 ++++++++++++++---- api/trace/testtrace/b3_propagator_test.go | 43 +- .../trace_context_propagator_test.go | 21 +- 5 files changed, 421 insertions(+), 443 deletions(-) delete mode 100644 api/trace/b3_propagator_integration_test.go diff --git a/api/trace/b3_propagator_integration_test.go b/api/trace/b3_propagator_integration_test.go deleted file mode 100644 index 2a92e187fe0..00000000000 --- a/api/trace/b3_propagator_integration_test.go +++ /dev/null @@ -1,291 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package trace_test - -import ( - "context" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - - "go.opentelemetry.io/otel/api/trace" -) - -var ( - traceID = trace.ID{0, 0, 0, 0, 0, 0, 0, 0x7b, 0, 0, 0, 0, 0, 0, 0x1, 0xc8} - traceIDStr = "000000000000007b00000000000001c8" - spanID = trace.SpanID{0, 0, 0, 0, 0, 0, 0, 0x7b} - spanIDStr = "000000000000007b" -) - -type Supplier struct { - SingleHeader string - DebugFlagHeader string - TraceIDHeader string - SpanIDHeader string - SampledHeader string - ParentSpanIDHeader string -} - -func (s *Supplier) Get(key string) string { - switch key { - case trace.B3SingleHeader: - return s.SingleHeader - case trace.B3DebugFlagHeader: - return s.DebugFlagHeader - case trace.B3TraceIDHeader: - return s.TraceIDHeader - case trace.B3SpanIDHeader: - return s.SpanIDHeader - case trace.B3SampledHeader: - return s.SampledHeader - case trace.B3ParentSpanIDHeader: - return s.ParentSpanIDHeader - } - return "" -} - -func (s *Supplier) Set(key, value string) { - fmt.Println("called", key, value) - switch key { - case trace.B3SingleHeader: - s.SingleHeader = value - case trace.B3DebugFlagHeader: - s.DebugFlagHeader = value - case trace.B3TraceIDHeader: - s.TraceIDHeader = value - case trace.B3SpanIDHeader: - s.SpanIDHeader = value - case trace.B3SampledHeader: - s.SampledHeader = value - case trace.B3ParentSpanIDHeader: - s.ParentSpanIDHeader = value - } -} - -type TestSpan struct { - trace.NoopSpan - sc trace.SpanContext -} - -func (s TestSpan) SpanContext() trace.SpanContext { - return s.sc -} - -func TestInject(t *testing.T) { - tests := []struct { - sc trace.SpanContext - b3 trace.B3 - expected *Supplier - }{ - { - sc: trace.SpanContext{}, - b3: trace.B3{}, - expected: &Supplier{}, - }, - { - sc: trace.SpanContext{ - SpanID: spanID, - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{}, - expected: &Supplier{}, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{}, - expected: &Supplier{}, - }, - { - sc: trace.SpanContext{ - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{}, - expected: &Supplier{}, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{InjectEncoding: trace.SingleHeader}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - }, - b3: trace.B3{InjectEncoding: trace.SingleHeader}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-0", - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsUnset, - }, - b3: trace.B3{InjectEncoding: trace.SingleHeader}, - expected: &Supplier{ - SingleHeader: "000000000000007b00000000000001c8-000000000000007b", - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "1", - SingleHeader: "000000000000007b00000000000001c8-000000000000007b-1", - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsUnset, - }, - b3: trace.B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - }, - b3: trace.B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "0", - }, - }, - { - sc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsSampled, - }, - b3: trace.B3{}, - expected: &Supplier{ - TraceIDHeader: traceIDStr, - SpanIDHeader: spanIDStr, - SampledHeader: "1", - }, - }, - } - - for _, test := range tests { - ctx := trace.ContextWithSpan(context.Background(), TestSpan{sc: test.sc}) - actual := new(Supplier) - test.b3.Inject(ctx, actual) - assert.Equal( - t, - test.expected, - actual, - "B3: %#v, SpanContext: %#v", test.b3, test.sc, - ) - } -} - -func TestExtract(t *testing.T) { - tests := []struct { - traceID string - spanID string - parentSpanID string - sampled string - flags string - single string - expected trace.SpanContext - }{ - { - traceID: "", - spanID: "", - parentSpanID: "", - sampled: "", - flags: "", - single: "", - expected: trace.EmptySpanContext(), - }, - { - traceID: "", - spanID: "", - parentSpanID: "", - sampled: "", - flags: "", - single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", - expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, - }, - { - traceID: traceIDStr, - spanID: spanIDStr, - parentSpanID: "00000000000001c8", - sampled: "1", - flags: "", - single: "", - expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, - }, - { - traceID: traceIDStr, - spanID: spanIDStr, - parentSpanID: "00000000000001c8", - sampled: "1", - flags: "", - single: "000000000000007b00000000000001c8-000000000000007b-1-00000000000001c8", - expected: trace.SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled}, - }, - } - - b3 := trace.B3{} - for _, test := range tests { - ctx := context.Background() - supplier := &Supplier{ - SingleHeader: test.single, - DebugFlagHeader: test.flags, - TraceIDHeader: test.traceID, - SpanIDHeader: test.spanID, - SampledHeader: test.sampled, - ParentSpanIDHeader: test.parentSpanID, - } - info := []interface{}{ - "trace ID: %q, span ID: %q, parent span ID: %q, sampled: %q, flags: %q, single: %q", - test.traceID, - test.spanID, - test.parentSpanID, - test.sampled, - test.flags, - test.single, - } - actual := trace.RemoteSpanContextFromContext(b3.Extract(ctx, supplier)) - assert.Equal(t, test.expected, actual, info...) - } -} diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index 83ffa7d06b9..84eaf170f86 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/api/trace/testtrace/b3_propagator_benchmark_test.go @@ -25,32 +25,21 @@ import ( func BenchmarkExtractB3(b *testing.B) { testGroup := []struct { - encoding trace.B3Encoding - name string - tests []extractTest + name string + tests []extractTest }{ { - name: "multiple headers", - tests: extractMultipleHeaders, + name: "valid headers", + tests: extractHeaders, }, { - encoding: trace.SingleHeader, - name: "single headers", - tests: extractSingleHeader, - }, - { - name: "invalid multiple headers", - tests: extractInvalidB3MultipleHeaders, - }, - { - encoding: trace.SingleHeader, - name: "invalid single headers", - tests: extractInvalidB3SingleHeader, + name: "invalid headers", + tests: extractInvalidHeaders, }, } for _, tg := range testGroup { - propagator := trace.B3{InjectEncoding: tg.encoding} + propagator := trace.B3{} for _, tt := range tg.tests { traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) { ctx := context.Background() @@ -71,18 +60,12 @@ func BenchmarkExtractB3(b *testing.B) { func BenchmarkInjectB3(b *testing.B) { var id uint64 testGroup := []struct { - encoding trace.B3Encoding - name string - tests []injectTest + name string + tests []injectTest }{ { - name: "multiple headers", - tests: injectB3MultipleHeader, - }, - { - encoding: trace.SingleHeader, - name: "single headers", - tests: injectB3SingleleHeader, + name: "valid headers", + tests: injectHeader, }, } @@ -93,8 +76,8 @@ func BenchmarkInjectB3(b *testing.B) { for _, tg := range testGroup { id = 0 - propagator := trace.B3{InjectEncoding: tg.encoding} for _, tt := range tg.tests { + propagator := trace.B3{InjectEncoding: tt.encoding} traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := context.Background() diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index eac0eb5d039..d15a55aa751 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -15,6 +15,8 @@ package testtrace_test import ( + "fmt" + "go.opentelemetry.io/otel/api/trace" ) @@ -28,12 +30,17 @@ var ( traceID64bitPadded = mustTraceIDFromHex("0000000000000000a3ce929d0e0e4736") ) -var extractMultipleHeaders = []extractTest{ +var extractHeaders = []extractTest{ + { + name: "empty", + headers: map[string]string{}, + wantSc: trace.EmptySpanContext(), + }, { - name: "sampling state defer", + name: "multiple: sampling state defer", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -42,22 +49,23 @@ var extractMultipleHeaders = []extractTest{ }, }, { - name: "sampling state deny", + name: "multiple: sampling state deny", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "0", }, wantSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, }, }, { - name: "sampling state accept", + name: "multiple: sampling state accept", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "1", }, wantSc: trace.SpanContext{ @@ -67,10 +75,10 @@ var extractMultipleHeaders = []extractTest{ }, }, { - name: "sampling state as a boolean", + name: "multiple: sampling state as a boolean", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "true", }, wantSc: trace.SpanContext{ @@ -80,10 +88,10 @@ var extractMultipleHeaders = []extractTest{ }, }, { - name: "debug flag set", + name: "multiple: debug flag set", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3DebugFlagHeader: "1", }, wantSc: trace.SpanContext{ @@ -92,14 +100,28 @@ var extractMultipleHeaders = []extractTest{ TraceFlags: trace.FlagsUnset, }, }, + { + name: "multiple: debug flag set to not 1 (ignored)", + headers: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3SampledHeader: "1", + trace.B3DebugFlagHeader: "2", + }, + wantSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + }, { // spec explicitly states "Debug implies an accept decision, so don't // also send the X-B3-Sampled header", make sure sampling is // deferred. - name: "debug flag set and sampling state is deny", + name: "multiple: debug flag set and sampling state is deny", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "0", trace.B3DebugFlagHeader: "1", }, @@ -110,10 +132,10 @@ var extractMultipleHeaders = []extractTest{ }, }, { - name: "with parent span id", + name: "multiple: with parent span id", headers: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "1", trace.B3ParentSpanIDHeader: "00f067aa0ba90200", }, @@ -124,17 +146,17 @@ var extractMultipleHeaders = []extractTest{ }, }, { - name: "with only sampled state header", + name: "multiple: with only sampled state header", headers: map[string]string{ trace.B3SampledHeader: "0", }, wantSc: trace.EmptySpanContext(), }, { - name: "left-padding 64-bit traceID", + name: "multiple: left-padding 64-bit traceID", headers: map[string]string{ trace.B3TraceIDHeader: "a3ce929d0e0e4736", - trace.B3SpanIDHeader: "00f067aa0ba902b7", + trace.B3SpanIDHeader: spanIDStr, }, wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, @@ -142,13 +164,10 @@ var extractMultipleHeaders = []extractTest{ TraceFlags: trace.FlagsUnset, }, }, -} - -var extractSingleHeader = []extractTest{ { - name: "sampling state defer", + name: "single: sampling state defer", headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", + trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -157,9 +176,9 @@ var extractSingleHeader = []extractTest{ }, }, { - name: "sampling state deny", + name: "single: sampling state deny", headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -167,9 +186,9 @@ var extractSingleHeader = []extractTest{ }, }, { - name: "sampling state accept", + name: "single: sampling state accept", headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -178,9 +197,9 @@ var extractSingleHeader = []extractTest{ }, }, { - name: "sampling state debug", + name: "single: sampling state debug", headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-d", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -189,9 +208,9 @@ var extractSingleHeader = []extractTest{ }, }, { - name: "with parent span id", + name: "single: with parent span id", headers: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1-00000000000000cd", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1-00000000000000cd", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -200,16 +219,16 @@ var extractSingleHeader = []extractTest{ }, }, { - name: "with only sampling state deny", + name: "single: with only sampling state deny", headers: map[string]string{ trace.B3SingleHeader: "0", }, wantSc: trace.EmptySpanContext(), }, { - name: "left-padding 64-bit traceID", + name: "single: left-padding 64-bit traceID", headers: map[string]string{ - trace.B3SingleHeader: "a3ce929d0e0e4736-00f067aa0ba902b7", + trace.B3SingleHeader: fmt.Sprintf("a3ce929d0e0e4736-%s", spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, @@ -217,11 +236,55 @@ var extractSingleHeader = []extractTest{ TraceFlags: trace.FlagsUnset, }, }, + { + name: "both single and multiple: single priority", + headers: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3SampledHeader: "0", + }, + wantSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + }, + // An invalid Single Headers should fallback to multiple. + { + name: "both single and multiple: invalid single", + headers: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-%s-", traceIDStr, spanIDStr), + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3SampledHeader: "0", + }, + wantSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, + }, + }, + // Invalid Mult Header should not be noticed as Single takes precedence. + { + name: "both single and multiple: invalid multiple", + headers: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3SampledHeader: "invalid", + }, + wantSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + }, } -var extractInvalidB3MultipleHeaders = []extractTest{ +var extractInvalidHeaders = []extractTest{ { - name: "trace ID length > 32", + name: "multiple: trace ID length > 32", headers: map[string]string{ trace.B3TraceIDHeader: "ab00000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -229,7 +292,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "trace ID length >16 and <32", + name: "multiple: trace ID length >16 and <32", headers: map[string]string{ trace.B3TraceIDHeader: "ab0000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -237,7 +300,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "trace ID length <16", + name: "multiple: trace ID length <16", headers: map[string]string{ trace.B3TraceIDHeader: "ab0000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -245,7 +308,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "wrong span ID length", + name: "multiple: wrong span ID length", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "cd0000000000000000", @@ -253,7 +316,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "wrong sampled flag length", + name: "multiple: wrong sampled flag length", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -261,7 +324,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "bogus trace ID", + name: "multiple: bogus trace ID", headers: map[string]string{ trace.B3TraceIDHeader: "qw000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -269,7 +332,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "bogus span ID", + name: "multiple: bogus span ID", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "qw00000000000000", @@ -277,7 +340,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "bogus sampled flag", + name: "multiple: bogus sampled flag", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -285,7 +348,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "upper case trace ID", + name: "multiple: upper case trace ID", headers: map[string]string{ trace.B3TraceIDHeader: "AB000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -293,7 +356,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "upper case span ID", + name: "multiple: upper case span ID", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "CD00000000000000", @@ -301,7 +364,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "zero trace ID", + name: "multiple: zero trace ID", headers: map[string]string{ trace.B3TraceIDHeader: "00000000000000000000000000000000", trace.B3SpanIDHeader: "cd00000000000000", @@ -309,7 +372,7 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "zero span ID", + name: "multiple: zero span ID", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SpanIDHeader: "0000000000000000", @@ -317,102 +380,99 @@ var extractInvalidB3MultipleHeaders = []extractTest{ }, }, { - name: "missing span ID", + name: "multiple: missing span ID", headers: map[string]string{ trace.B3TraceIDHeader: "ab000000000000000000000000000000", trace.B3SampledHeader: "1", }, }, { - name: "missing trace ID", + name: "multiple: missing trace ID", headers: map[string]string{ trace.B3SpanIDHeader: "cd00000000000000", trace.B3SampledHeader: "1", }, }, { - name: "sampled header set to 1 but trace ID and span ID are missing", + name: "multiple: sampled header set to 1 but trace ID and span ID are missing", headers: map[string]string{ trace.B3SampledHeader: "1", }, }, -} - -var extractInvalidB3SingleHeader = []extractTest{ { - name: "wrong trace ID length", + name: "single: wrong trace ID length", headers: map[string]string{ trace.B3SingleHeader: "ab00000000000000000000000000000000-cd00000000000000-1", }, }, { - name: "wrong span ID length", + name: "single: wrong span ID length", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd0000000000000000-1", }, }, { - name: "wrong sampled state length", + name: "single: wrong sampled state length", headers: map[string]string{ trace.B3SingleHeader: "00-ab000000000000000000000000000000-cd00000000000000-01", }, }, { - name: "wrong parent span ID length", + name: "single: wrong parent span ID length", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-cd0000000000000000", }, }, { - name: "bogus trace ID", + name: "single: bogus trace ID", headers: map[string]string{ trace.B3SingleHeader: "qw000000000000000000000000000000-cd00000000000000-1", }, }, { - name: "bogus span ID", + name: "single: bogus span ID", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-qw00000000000000-1", }, }, { - name: "bogus sampled flag", + name: "single: bogus sampled flag", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-q", }, }, { - name: "bogus parent span ID", + name: "single: bogus parent span ID", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-qw00000000000000", }, }, { - name: "upper case trace ID", + name: "single: upper case trace ID", headers: map[string]string{ trace.B3SingleHeader: "AB000000000000000000000000000000-cd00000000000000-1", }, }, { - name: "upper case span ID", + name: "single: upper case span ID", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-CD00000000000000-1", }, }, { - name: "upper case parent span ID", + name: "single: upper case parent span ID", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-EF00000000000000", }, }, { - name: "zero trace ID and span ID", + name: "single: zero trace ID and span ID", headers: map[string]string{ trace.B3SingleHeader: "00000000000000000000000000000000-0000000000000000-1", }, }, { - name: "with sampling set to true", + name: "single: with sampling set to true", headers: map[string]string{ trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-true", }, @@ -421,77 +481,303 @@ var extractInvalidB3SingleHeader = []extractTest{ type injectTest struct { name string + encoding trace.B3Encoding parentSc trace.SpanContext wantHeaders map[string]string doNotWantHeaders []string } -var injectB3MultipleHeader = []injectTest{ +var injectHeader = []injectTest{ { - name: "valid spancontext, sampled", + name: "none: sampled", parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", + trace.B3TraceIDHeader: traceIDStr, trace.B3SpanIDHeader: "0000000000000001", trace.B3SampledHeader: "1", }, doNotWantHeaders: []string{ trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, }, }, { - name: "valid spancontext, not sampled", + name: "none: not sampled", parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: "4bf92f3577b34da6a3ce929d0e0e4736", + trace.B3TraceIDHeader: traceIDStr, trace.B3SpanIDHeader: "0000000000000002", trace.B3SampledHeader: "0", }, doNotWantHeaders: []string{ trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "none: unset sampled", + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "0000000000000003", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "multiple: sampled", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "0000000000000004", + trace.B3SampledHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "multiple: not sampled", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "0000000000000005", + trace.B3SampledHeader: "0", + }, + doNotWantHeaders: []string{ + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "multiple: unset sampled", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "0000000000000006", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, }, }, -} - -var injectB3SingleleHeader = []injectTest{ { - name: "valid spancontext, sampled", + name: "single: sampled", + encoding: trace.SingleHeader, parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-0000000000000001-1", + trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000007-1", traceIDStr), }, doNotWantHeaders: []string{ trace.B3TraceIDHeader, trace.B3SpanIDHeader, trace.B3SampledHeader, trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, }, }, { - name: "valid spancontext, not sampled", + name: "single: not sampled", + encoding: trace.SingleHeader, parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: "4bf92f3577b34da6a3ce929d0e0e4736-0000000000000002-0", + trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000008-0", traceIDStr), }, doNotWantHeaders: []string{ trace.B3TraceIDHeader, trace.B3SpanIDHeader, trace.B3SampledHeader, trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, }, }, + { + name: "single: unset sampled", + encoding: trace.SingleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, + }, + wantHeaders: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000009", traceIDStr), + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "single+multiple: sampled", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "000000000000000a", + trace.B3SampledHeader: "1", + trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000a-1", traceIDStr), + }, + doNotWantHeaders: []string{ + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "single+multiple: not sampled", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsNotSampled, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "000000000000000b", + trace.B3SampledHeader: "0", + trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000b-0", traceIDStr), + }, + doNotWantHeaders: []string{ + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "single+multiple: unset sampled", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsUnset, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: "000000000000000c", + trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000c", traceIDStr), + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + }, + }, +} + +var injectInvalidHeaderGenerator = []injectTest{ + { + name: "empty", + parentSc: trace.SpanContext{}, + }, + { + name: "missing traceID", + parentSc: trace.SpanContext{ + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, + }, + { + name: "missing spanID", + parentSc: trace.SpanContext{ + TraceID: traceID, + TraceFlags: trace.FlagsSampled, + }, + }, + { + name: "missing traceID and spanID", + parentSc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + }, +} + +var injectInvalidHeader []injectTest + +func init() { + injectInvalidHeader = make([]injectTest, 0, len(injectInvalidHeaderGenerator)*4) + allHeaders := []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + } + // Nothing should be set for any header regardless of encoding. + for _, t := range injectInvalidHeaderGenerator { + injectInvalidHeader = append(injectInvalidHeader, injectTest{ + name: "none: " + t.name, + parentSc: t.parentSc, + doNotWantHeaders: allHeaders, + }) + injectInvalidHeader = append(injectInvalidHeader, injectTest{ + name: "multiple: " + t.name, + encoding: trace.MultipleHeader, + parentSc: t.parentSc, + doNotWantHeaders: allHeaders, + }) + injectInvalidHeader = append(injectInvalidHeader, injectTest{ + name: "single: " + t.name, + encoding: trace.SingleHeader, + parentSc: t.parentSc, + doNotWantHeaders: allHeaders, + }) + injectInvalidHeader = append(injectInvalidHeader, injectTest{ + name: "single+multiple: " + t.name, + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: t.parentSc, + doNotWantHeaders: allHeaders, + }) + } } diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 1db181d4a61..f65da219116 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -28,32 +28,21 @@ import ( func TestExtractB3(t *testing.T) { testGroup := []struct { - encoding trace.B3Encoding - name string - tests []extractTest + name string + tests []extractTest }{ { - name: "multiple headers", - tests: extractMultipleHeaders, + name: "valid headers", + tests: extractHeaders, }, { - encoding: trace.SingleHeader, - name: "single headers", - tests: extractSingleHeader, - }, - { - name: "invalid multiple headers", - tests: extractInvalidB3MultipleHeaders, - }, - { - encoding: trace.SingleHeader, - name: "invalid single headers", - tests: extractInvalidB3SingleHeader, + name: "invalid headers", + tests: extractInvalidHeaders, }, } for _, tg := range testGroup { - propagator := trace.B3{InjectEncoding: tg.encoding} + propagator := trace.B3{} props := propagation.New(propagation.WithExtractors(propagator)) for _, tt := range tg.tests { @@ -77,18 +66,16 @@ func TestExtractB3(t *testing.T) { func TestInjectB3(t *testing.T) { var id uint64 testGroup := []struct { - encoding trace.B3Encoding - name string - tests []injectTest + name string + tests []injectTest }{ { - name: "multiple headers", - tests: injectB3MultipleHeader, + name: "valid headers", + tests: injectHeader, }, { - encoding: trace.SingleHeader, - name: "single headers", - tests: injectB3SingleleHeader, + name: "invalid headers", + tests: injectInvalidHeader, }, } @@ -99,9 +86,9 @@ func TestInjectB3(t *testing.T) { for _, tg := range testGroup { id = 0 - propagator := trace.B3{InjectEncoding: tg.encoding} - props := propagation.New(propagation.WithInjectors(propagator)) for _, tt := range tg.tests { + propagator := trace.B3{InjectEncoding: tt.encoding} + props := propagation.New(propagation.WithInjectors(propagator)) t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := context.Background() diff --git a/api/trace/testtrace/trace_context_propagator_test.go b/api/trace/testtrace/trace_context_propagator_test.go index 2a8fea2fcd0..10a7434e920 100644 --- a/api/trace/testtrace/trace_context_propagator_test.go +++ b/api/trace/testtrace/trace_context_propagator_test.go @@ -26,18 +26,31 @@ import ( mocktrace "go.opentelemetry.io/otel/internal/trace" ) +const ( + traceIDStr = "4bf92f3577b34da6a3ce929d0e0e4736" + spanIDStr = "00f067aa0ba902b7" +) + var ( - traceID = mustTraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") - spanID = mustSpanIDFromHex("00f067aa0ba902b7") + traceID = mustTraceIDFromHex(traceIDStr) + spanID = mustSpanIDFromHex(spanIDStr) ) func mustTraceIDFromHex(s string) (t trace.ID) { - t, _ = trace.IDFromHex(s) + var err error + t, err = trace.IDFromHex(s) + if err != nil { + panic(err) + } return } func mustSpanIDFromHex(s string) (t trace.SpanID) { - t, _ = trace.SpanIDFromHex(s) + var err error + t, err = trace.SpanIDFromHex(s) + if err != nil { + panic(err) + } return } From 727af415a2b0b6f0158cf6d5570d359b2b09763a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 17:14:40 -0700 Subject: [PATCH 15/33] Update comment --- api/trace/span_context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/trace/span_context.go b/api/trace/span_context.go index 8af0298efc5..99b0fec0f1b 100644 --- a/api/trace/span_context.go +++ b/api/trace/span_context.go @@ -32,7 +32,7 @@ const ( // FlagsUnset is a byte with the sampled bit unset but all other bits // set (the inverse of FlagsSampled). Some systems (e.g. B3) distinguish // between setting flags to not sample and not making a sampling - // decision, usually called a deferred decision. This byte is used + // decision, usually called a deferred decision. This byte mask is used // internally to represent this unset state. It is not a valid // representation elsewhere. FlagsUnset = traceFlagsBitMaskUnset From 22c4a48b55a6f7a9d2b9e03021454be759dc9295 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 17:16:51 -0700 Subject: [PATCH 16/33] Benchmark invalid B3 injects as well --- api/trace/testtrace/b3_propagator_benchmark_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index 84eaf170f86..6a7bf961bb8 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/api/trace/testtrace/b3_propagator_benchmark_test.go @@ -67,6 +67,10 @@ func BenchmarkInjectB3(b *testing.B) { name: "valid headers", tests: injectHeader, }, + { + name: "invalid headers", + tests: injectInvalidHeader, + }, } mockTracer := &mocktrace.MockTracer{ From b6a2a145cfc0b3396e4e99d897810bafd8793137 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Jul 2020 20:21:50 -0700 Subject: [PATCH 17/33] Update trace flags Add a FlagsDebug and FlagsDeferred to track the B3 trace state. Add helper methods to the SpanContext to check the debug and deferred bit of the trace flags. Update SpanContext.IsSampled to return if the sampling decision is to sample rather than if the sample bit is set. This means that if the debug bit is also set it will return true. --- api/trace/b3_propagator.go | 30 +++++---- api/trace/b3_propagator_test.go | 30 ++++++--- api/trace/span_context.go | 38 ++++++----- api/trace/span_context_test.go | 2 +- .../testtrace/b3_propagator_data_test.go | 66 ++++++++++--------- api/trace/trace_context_propagator.go | 3 +- 6 files changed, 101 insertions(+), 68 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 10ff658a841..cd9cc515502 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -87,6 +87,8 @@ func (b3 B3) supports(e B3Encoding) bool { var _ propagation.HTTPPropagator = B3{} // Inject injects a context into the supplier as B3 headers. +// The parent span ID is omitted because it is not tracked in the +// SpanContext. func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { sc := SpanFromContext(ctx).SpanContext() if !sc.IsValid() { @@ -99,7 +101,9 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { header = append(header, sc.TraceID.String(), sc.SpanID.String()) } - if sc.TraceFlags&FlagsUnset != FlagsUnset { + if sc.isDebug() { + header = append(header, "d") + } else if !sc.isDeferred() { if sc.IsSampled() { header = append(header, "1") } else { @@ -116,7 +120,10 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { supplier.Set(B3SpanIDHeader, sc.SpanID.String()) } - if sc.TraceFlags&FlagsUnset != FlagsUnset { + if sc.isDebug() { + // Since Debug implies deferred, don't also send "X-B3-Sampled". + supplier.Set(B3DebugFlagHeader, "1") + } else if !sc.isDeferred() { if sc.IsSampled() { supplier.Set(B3SampledHeader, "1") } else { @@ -179,20 +186,19 @@ func extractMultiple(traceID, spanID, parentSpanID, sampled, flags string) (Span // allow "true" and "false" as inputs for interop purposes. switch strings.ToLower(sampled) { case "0", "false": - sc.TraceFlags = FlagsNotSampled + // Zero value for TraceFlags sample bit is unset. case "1", "true": sc.TraceFlags = FlagsSampled case "": - sc.TraceFlags = FlagsUnset + sc.TraceFlags = FlagsDeferred default: return empty, errInvalidSampledHeader } - // The only accepted value for Flags is "1". This will set Debug to true. All - // other values and omission of header will be ignored. + // The only accepted value for Flags is "1". This will set Debug to + // true. All other values and omission of header will be ignored. if flags == "1" { - // We do not track debug status, but the sampling needs to be unset. - sc.TraceFlags = FlagsUnset + sc.TraceFlags |= FlagsDebug } if traceID != "" { @@ -305,12 +311,14 @@ func extractSingle(contextHeader string) (SpanContext, error) { return empty, errInvalidTraceIDValue } switch sampling { - case "", "d": - sc.TraceFlags = FlagsUnset + case "": + sc.TraceFlags = FlagsDeferred + case "d": + sc.TraceFlags = FlagsDebug case "1": sc.TraceFlags = FlagsSampled case "0": - sc.TraceFlags = FlagsNotSampled + // Zero value for TraceFlags sample bit is unset. default: return empty, errInvalidSampledByte } diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go index fecfbfeb676..9dc44cc534a 100644 --- a/api/trace/b3_propagator_test.go +++ b/api/trace/b3_propagator_test.go @@ -44,7 +44,7 @@ func TestExtractMultiple(t *testing.T) { }, { "", "", "", "", "", - SpanContext{TraceFlags: FlagsUnset}, + SpanContext{TraceFlags: FlagsDeferred}, nil, }, { @@ -52,9 +52,24 @@ func TestExtractMultiple(t *testing.T) { SpanContext{TraceFlags: FlagsSampled}, nil, }, + { + "", "", "", "", "1", + SpanContext{TraceFlags: FlagsDeferred | FlagsDebug}, + nil, + }, + { + "", "", "", "0", "1", + SpanContext{TraceFlags: FlagsDebug}, + nil, + }, + { + "", "", "", "1", "1", + SpanContext{TraceFlags: FlagsSampled | FlagsDebug}, + nil, + }, { traceIDStr, spanIDStr, "", "", "", - SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnset}, + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsDeferred}, nil, }, { @@ -86,7 +101,7 @@ func TestExtractMultiple(t *testing.T) { }, { traceIDStr, spanIDStr, "", "1", "1", - SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsUnset}, + SpanContext{TraceID: traceID, SpanID: spanID, TraceFlags: FlagsSampled | FlagsDebug}, nil, }, // Invalid flags are discarded. @@ -195,10 +210,9 @@ func TestExtractSingle(t *testing.T) { expected SpanContext err error }{ - {"0", SpanContext{TraceFlags: FlagsNotSampled}, nil}, + {"0", SpanContext{}, nil}, {"1", SpanContext{TraceFlags: FlagsSampled}, nil}, - // debug flag is valid, but ignored. Sampling should be deferred. - {"d", SpanContext{TraceFlags: FlagsUnset}, nil}, + {"d", SpanContext{TraceFlags: FlagsDebug}, nil}, {"a", empty, errInvalidSampledByte}, {"3", empty, errInvalidSampledByte}, {"000000000000007b", empty, errInvalidScope}, @@ -209,7 +223,7 @@ func TestExtractSingle(t *testing.T) { SpanContext{ TraceID: ID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x1, 0xc8}, SpanID: spanID, - TraceFlags: FlagsUnset, + TraceFlags: FlagsDeferred, }, nil, }, @@ -218,7 +232,7 @@ func TestExtractSingle(t *testing.T) { SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: FlagsUnset, + TraceFlags: FlagsDeferred, }, nil, }, diff --git a/api/trace/span_context.go b/api/trace/span_context.go index 99b0fec0f1b..91398bb3b3c 100644 --- a/api/trace/span_context.go +++ b/api/trace/span_context.go @@ -21,21 +21,15 @@ import ( ) const ( - traceFlagsBitMaskNotSampled = byte(0x00) - traceFlagsBitMaskSampled = byte(0x01) - traceFlagsBitMaskUnset = byte(0xFE) - - // FlagsSampled is a byte with the sampled bit set. - FlagsSampled = traceFlagsBitMaskSampled - // FlagsNotSampled is a byte with the sampled bit unset. - FlagsNotSampled = traceFlagsBitMaskNotSampled - // FlagsUnset is a byte with the sampled bit unset but all other bits - // set (the inverse of FlagsSampled). Some systems (e.g. B3) distinguish - // between setting flags to not sample and not making a sampling - // decision, usually called a deferred decision. This byte mask is used - // internally to represent this unset state. It is not a valid - // representation elsewhere. - FlagsUnset = traceFlagsBitMaskUnset + // FlagsSampled is a bitmask with the sampled bit set. A SpanContext + // with the sampling bit set means the span is sampled. + FlagsSampled = byte(0x01) + // FlagsDeferred is a bitmask with the deferred bit set. A SpanContext + // with the deferred bit set means the sampling decision has been + // defered to the receiver. + FlagsDeferred = byte(0x02) + // FlagsDebug is a bitmask with the debug bit set. + FlagsDebug = byte(0x04) ErrInvalidHexID errorConst = "trace-id and span-id can only contain [0-9a-f] characters, all lowercase" @@ -187,7 +181,17 @@ func (sc SpanContext) HasSpanID() bool { return sc.SpanID.IsValid() } -// IsSampled check if the sampling bit in trace flags is set. +// isDeferred returns if the deferred bit is set in the trace flags. +func (sc SpanContext) isDeferred() bool { + return sc.TraceFlags&FlagsDeferred == FlagsDeferred +} + +// isDebug returns if the debug bit is set in the trace flags. +func (sc SpanContext) isDebug() bool { + return sc.TraceFlags&FlagsDebug == FlagsDebug +} + +// IsSampled returns if the span should be sampled. func (sc SpanContext) IsSampled() bool { - return sc.TraceFlags&traceFlagsBitMaskSampled == traceFlagsBitMaskSampled + return sc.isDebug() || sc.TraceFlags&FlagsSampled == FlagsSampled } diff --git a/api/trace/span_context_test.go b/api/trace/span_context_test.go index 9787e92db9c..bdfcc8460d9 100644 --- a/api/trace/span_context_test.go +++ b/api/trace/span_context_test.go @@ -176,7 +176,7 @@ func TestSpanContextIsSampled(t *testing.T) { name: "sampled plus unused", sc: trace.SpanContext{ TraceID: trace.ID([16]byte{1}), - TraceFlags: trace.FlagsSampled | trace.FlagsUnset, + TraceFlags: trace.FlagsSampled | ^trace.FlagsSampled, }, want: true, }, { diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index d15a55aa751..f8beff9ed2b 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -45,7 +45,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, }, { @@ -56,9 +56,8 @@ var extractHeaders = []extractTest{ trace.B3SampledHeader: "0", }, wantSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, }, { @@ -75,7 +74,7 @@ var extractHeaders = []extractTest{ }, }, { - name: "multiple: sampling state as a boolean", + name: "multiple: sampling state as a boolean: true", headers: map[string]string{ trace.B3TraceIDHeader: traceIDStr, trace.B3SpanIDHeader: spanIDStr, @@ -87,6 +86,18 @@ var extractHeaders = []extractTest{ TraceFlags: trace.FlagsSampled, }, }, + { + name: "multiple: sampling state as a boolean: false", + headers: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3SampledHeader: "false", + }, + wantSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + }, { name: "multiple: debug flag set", headers: map[string]string{ @@ -97,7 +108,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred | trace.FlagsDebug, }, }, { @@ -128,7 +139,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDebug, }, }, { @@ -161,7 +172,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, }, { @@ -172,7 +183,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, }, { @@ -204,7 +215,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDebug, }, }, { @@ -233,7 +244,7 @@ var extractHeaders = []extractTest{ wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, }, { @@ -260,9 +271,8 @@ var extractHeaders = []extractTest{ trace.B3SampledHeader: "0", }, wantSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, }, // Invalid Mult Header should not be noticed as Single takes precedence. @@ -509,9 +519,8 @@ var injectHeader = []injectTest{ { name: "none: not sampled", parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, @@ -529,7 +538,7 @@ var injectHeader = []injectTest{ parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, @@ -565,9 +574,8 @@ var injectHeader = []injectTest{ name: "multiple: not sampled", encoding: trace.MultipleHeader, parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, @@ -586,7 +594,7 @@ var injectHeader = []injectTest{ parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, @@ -622,9 +630,8 @@ var injectHeader = []injectTest{ name: "single: not sampled", encoding: trace.SingleHeader, parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, wantHeaders: map[string]string{ trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000008-0", traceIDStr), @@ -643,7 +650,7 @@ var injectHeader = []injectTest{ parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000009", traceIDStr), @@ -679,9 +686,8 @@ var injectHeader = []injectTest{ name: "single+multiple: not sampled", encoding: trace.SingleHeader | trace.MultipleHeader, parentSc: trace.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsNotSampled, + TraceID: traceID, + SpanID: spanID, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, @@ -700,7 +706,7 @@ var injectHeader = []injectTest{ parentSc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, - TraceFlags: trace.FlagsUnset, + TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, diff --git a/api/trace/trace_context_propagator.go b/api/trace/trace_context_propagator.go index daac8036cb4..5413106f440 100644 --- a/api/trace/trace_context_propagator.go +++ b/api/trace/trace_context_propagator.go @@ -137,7 +137,8 @@ func (TraceContext) extract(supplier propagation.HTTPSupplier) SpanContext { if err != nil || len(opts) < 1 || (version == 0 && opts[0] > 2) { return EmptySpanContext() } - sc.TraceFlags = opts[0] &^ FlagsUnset + // Clear all flags other than the trace-context supported sampling bit. + sc.TraceFlags = opts[0] & FlagsSampled if !sc.IsValid() { return EmptySpanContext() From 712f87970245c2dc78cd7230bf7606221f633f27 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 09:40:56 -0700 Subject: [PATCH 18/33] Revert SpanContext.IsSampled back --- api/trace/span_context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/trace/span_context.go b/api/trace/span_context.go index 91398bb3b3c..abce3f76a65 100644 --- a/api/trace/span_context.go +++ b/api/trace/span_context.go @@ -191,7 +191,7 @@ func (sc SpanContext) isDebug() bool { return sc.TraceFlags&FlagsDebug == FlagsDebug } -// IsSampled returns if the span should be sampled. +// IsSampled returns if the sampling bit is set in the trace flags. func (sc SpanContext) IsSampled() bool { - return sc.isDebug() || sc.TraceFlags&FlagsSampled == FlagsSampled + return sc.TraceFlags&FlagsSampled == FlagsSampled } From 49335f926bced2311596dcc735d4b738d91cbff4 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 09:41:38 -0700 Subject: [PATCH 19/33] Add comment to b3 test data generation --- api/trace/testtrace/b3_propagator_data_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index f8beff9ed2b..ea09730219c 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -751,6 +751,8 @@ var injectInvalidHeaderGenerator = []injectTest{ var injectInvalidHeader []injectTest func init() { + // Preform a test for each invalid injectTest with all combinations of + // encoding values. injectInvalidHeader = make([]injectTest, 0, len(injectInvalidHeaderGenerator)*4) allHeaders := []string{ trace.B3TraceIDHeader, From ee571dea21e17b624887f9e79e90a69468ad6f7e Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 09:41:54 -0700 Subject: [PATCH 20/33] Update Changelog --- CHANGELOG.md | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41002d9d038..ea0a3bd016e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,24 +11,34 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - The `B3Encoding` is added to represent the B3 encoding(s) the B3 propagator injects. - A value for HTTP supported encodings (Multiple Header and Single Header) are included. -- The `FlagsNotSampled` trace flag to complement the existing `FlagsSampled`. (#882) + A value for HTTP supported encodings (Multiple Header: `MultipleHeader`, Single Header: `SingleHeader`) are included. (#882) +- The `FlagsDeferr` trace flag to indicate if the trace sampling decision has been deferred. + This addition is to support the B3 propagation specification, though it might support future trace systems. (#882) +- The `FlagsDebug` trace flag to indicate if the trace is a debug trace. + This addition is to support the B3 propagation specification. (#882) ### Changed - Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879) - Use lowercase header names for B3 Multiple Headers. (#881) - The B3 propagator `SingleHeader` field has been replaced with `InjectEncoding`. - This new field can be set to combinations of the `B3Encoding` bitmasks and will inject trace information in these encodings. (#882) -- The B3 propagator now extracts from either Single or Multiple B3 Headers based on what is contained in the header (with preference for the Single Header). - This is instead of only extracting based on the propagator's configuration. (#882) -- Rename `FlagsUnused` to `FlagsUnset` and clearly defined its purpose to act as a placeholder for systems that have a trinary sampling state (i.e. sample, don't sample, unspecified) to act as the unset state. (#882) + This new field can be set to combinations of the `B3Encoding` bitmasks and will inject trace information in these encodings. + If no encoding is set, the propagator will default to `MultipleHeader` encoding.(#882) +- The B3 propagator now extracts from either HTTP encoding of B3 (Single Header or Multiple Header) based on what is contained in the header. Preference is given to Single Header encoding with Multiple Header being the fallback if Single Header is not found or is invalid. + This behavior change is made to dynamically support all correctly encoded traces received instead of having to guess the expected encoding prior to receiving. (#882) +### Removed + +- The `FlagsUnset` trace flag is removed. + The purpose of this flag was to act as the inverse of `FlagsSampled`, the inverse of `FlagsSampled` is used instead. (#882) ### Fixed - The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881) - The B3 propagator now correctly supports sampling only values for a Single B3 Header. (#882) +- The B3 propagator now propagates the debug flag. + This includes changing the presences of the debug flag into a sampling bit set. + Instead, this now follow the B3 specification and omits the `X-B3-Sampling` header. (#882) - The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and correctly does not set a sampling value in this case when injecting. (#882) ## [0.7.0] - 2020-06-26 From b2f30752a10f420f5210bc362e52d9b53c54c83c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 09:46:32 -0700 Subject: [PATCH 21/33] Fix trace flag name in Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea0a3bd016e..a8aaa1bc09f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `B3Encoding` is added to represent the B3 encoding(s) the B3 propagator injects. A value for HTTP supported encodings (Multiple Header: `MultipleHeader`, Single Header: `SingleHeader`) are included. (#882) -- The `FlagsDeferr` trace flag to indicate if the trace sampling decision has been deferred. +- The `FlagsDeferred` trace flag to indicate if the trace sampling decision has been deferred. This addition is to support the B3 propagation specification, though it might support future trace systems. (#882) - The `FlagsDebug` trace flag to indicate if the trace is a debug trace. This addition is to support the B3 propagation specification. (#882) From 6b44a1027878e723bdeb9cfda0886dfdaaff5992 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 09:50:25 -0700 Subject: [PATCH 22/33] Fix Changelog formatting --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8aaa1bc09f..4edba35531d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use lowercase header names for B3 Multiple Headers. (#881) - The B3 propagator `SingleHeader` field has been replaced with `InjectEncoding`. This new field can be set to combinations of the `B3Encoding` bitmasks and will inject trace information in these encodings. - If no encoding is set, the propagator will default to `MultipleHeader` encoding.(#882) -- The B3 propagator now extracts from either HTTP encoding of B3 (Single Header or Multiple Header) based on what is contained in the header. Preference is given to Single Header encoding with Multiple Header being the fallback if Single Header is not found or is invalid. + If no encoding is set, the propagator will default to `MultipleHeader` encoding. (#882) +- The B3 propagator now extracts from either HTTP encoding of B3 (Single Header or Multiple Header) based on what is contained in the header. + Preference is given to Single Header encoding with Multiple Header being the fallback if Single Header is not found or is invalid. This behavior change is made to dynamically support all correctly encoded traces received instead of having to guess the expected encoding prior to receiving. (#882) ### Removed From 3f92d8c84d46e763b126233003a82455839e1adc Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 10:11:50 -0700 Subject: [PATCH 23/33] Update Changelog --- CHANGELOG.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4edba35531d..0688d98b021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- The `B3Encoding` is added to represent the B3 encoding(s) the B3 propagator injects. +- The `B3Encoding` type to represent the B3 encoding(s) the B3 propagator can inject. A value for HTTP supported encodings (Multiple Header: `MultipleHeader`, Single Header: `SingleHeader`) are included. (#882) -- The `FlagsDeferred` trace flag to indicate if the trace sampling decision has been deferred. - This addition is to support the B3 propagation specification, though it might support future trace systems. (#882) -- The `FlagsDebug` trace flag to indicate if the trace is a debug trace. - This addition is to support the B3 propagation specification. (#882) +- The `FlagsDeferred` trace flag to indicate if the trace sampling decision has been deferred. (#882) +- The `FlagsDebug` trace flag to indicate if the trace is a debug trace. (#882) ### Changed @@ -30,17 +28,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Removed -- The `FlagsUnset` trace flag is removed. +- The `FlagsUnused` trace flag is removed. The purpose of this flag was to act as the inverse of `FlagsSampled`, the inverse of `FlagsSampled` is used instead. (#882) ### Fixed - The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881) -- The B3 propagator now correctly supports sampling only values for a Single B3 Header. (#882) +- The B3 propagator now correctly supports sampling only values (`b3: 0`, `b3: 1`, or `b3: d`) for a Single B3 Header. (#882) - The B3 propagator now propagates the debug flag. - This includes changing the presences of the debug flag into a sampling bit set. + This removes the behavior of changing the debug flag into a set sampling bit. Instead, this now follow the B3 specification and omits the `X-B3-Sampling` header. (#882) -- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and correctly does not set a sampling value in this case when injecting. (#882) +- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and does not set the `X-B3-Sampling` header when injecting. (#882) ## [0.7.0] - 2020-06-26 From b178c20900b243a1ff6e296c13e63358e244a2c3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 11:01:17 -0700 Subject: [PATCH 24/33] Remove valid check at start of B3 injectg This check makes sample only headers not propagate. --- api/trace/b3_propagator.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index cd9cc515502..7faaaeb611b 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -91,9 +91,6 @@ var _ propagation.HTTPPropagator = B3{} // SpanContext. func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { sc := SpanFromContext(ctx).SpanContext() - if !sc.IsValid() { - return - } if b3.supports(SingleHeader) { header := []string{} From e882f6ff480ac2e3dbc6ee0b08a554ec102536e3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 11:02:29 -0700 Subject: [PATCH 25/33] Update B3 inject integration tests Use the passed SpanContext and check directly the span ID. --- .../testtrace/b3_propagator_benchmark_test.go | 17 +++-------- api/trace/testtrace/b3_propagator_test.go | 29 +++++++++---------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index 6a7bf961bb8..b5d1a564df9 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/api/trace/testtrace/b3_propagator_benchmark_test.go @@ -20,7 +20,6 @@ import ( "testing" "go.opentelemetry.io/otel/api/trace" - mocktrace "go.opentelemetry.io/otel/internal/trace" ) func BenchmarkExtractB3(b *testing.B) { @@ -58,7 +57,6 @@ func BenchmarkExtractB3(b *testing.B) { } func BenchmarkInjectB3(b *testing.B) { - var id uint64 testGroup := []struct { name string tests []injectTest @@ -73,22 +71,15 @@ func BenchmarkInjectB3(b *testing.B) { }, } - mockTracer := &mocktrace.MockTracer{ - Sampled: false, - StartSpanID: &id, - } - for _, tg := range testGroup { - id = 0 for _, tt := range tg.tests { propagator := trace.B3{InjectEncoding: tt.encoding} traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) { req, _ := http.NewRequest("GET", "http://example.com", nil) - ctx := context.Background() - if tt.parentSc.IsValid() { - ctx = trace.ContextWithRemoteSpanContext(ctx, tt.parentSc) - } - ctx, _ = mockTracer.Start(ctx, "inject") + ctx := trace.ContextWithSpan( + context.Background(), + testSpan{sc: tt.parentSc}, + ) b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index f65da219116..2687b4e0177 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" - mocktrace "go.opentelemetry.io/otel/internal/trace" ) func TestExtractB3(t *testing.T) { @@ -63,8 +62,16 @@ func TestExtractB3(t *testing.T) { } } +type testSpan struct { + trace.NoopSpan + sc trace.SpanContext +} + +func (s testSpan) SpanContext() trace.SpanContext { + return s.sc +} + func TestInjectB3(t *testing.T) { - var id uint64 testGroup := []struct { name string tests []injectTest @@ -79,24 +86,16 @@ func TestInjectB3(t *testing.T) { }, } - mockTracer := &mocktrace.MockTracer{ - Sampled: false, - StartSpanID: &id, - } - for _, tg := range testGroup { - id = 0 for _, tt := range tg.tests { propagator := trace.B3{InjectEncoding: tt.encoding} - props := propagation.New(propagation.WithInjectors(propagator)) t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) - ctx := context.Background() - if tt.parentSc.IsValid() { - ctx = trace.ContextWithRemoteSpanContext(ctx, tt.parentSc) - } - ctx, _ = mockTracer.Start(ctx, "inject") - propagation.InjectHTTP(ctx, props, req.Header) + ctx := trace.ContextWithSpan( + context.Background(), + testSpan{sc: tt.parentSc}, + ) + propagator.Inject(ctx, req.Header) for h, v := range tt.wantHeaders { got, want := req.Header.Get(h), v From f7d73cd3b7e6c0a827994977e9c58e66acabb363 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 11:03:17 -0700 Subject: [PATCH 26/33] Update B3 integration tests Run update checked SpanID to match sent. Add tests to validate sample only transmissions and debug flag support. --- .../testtrace/b3_propagator_data_test.go | 250 ++++++++++++++++-- 1 file changed, 235 insertions(+), 15 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index ea09730219c..ca458691986 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -507,7 +507,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000001", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "1", }, doNotWantHeaders: []string{ @@ -524,7 +524,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000002", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "0", }, doNotWantHeaders: []string{ @@ -542,7 +542,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000003", + trace.B3SpanIDHeader: spanIDStr, }, doNotWantHeaders: []string{ trace.B3SampledHeader, @@ -551,6 +551,58 @@ var injectHeader = []injectTest{ trace.B3SingleHeader, }, }, + { + name: "none: sampled only", + parentSc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3SampledHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "none: debug", + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, + { + name: "none: debug omitting sample", + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled | trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, { name: "multiple: sampled", encoding: trace.MultipleHeader, @@ -561,7 +613,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000004", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "1", }, doNotWantHeaders: []string{ @@ -579,7 +631,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000005", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "0", }, doNotWantHeaders: []string{ @@ -598,7 +650,7 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "0000000000000006", + trace.B3SpanIDHeader: spanIDStr, }, doNotWantHeaders: []string{ trace.B3SampledHeader, @@ -607,6 +659,61 @@ var injectHeader = []injectTest{ trace.B3SingleHeader, }, }, + { + name: "multiple: sampled only", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3SampledHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "multiple: debug", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, + { + name: "multiple: debug omitting sample", + encoding: trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled | trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, { name: "single: sampled", encoding: trace.SingleHeader, @@ -616,7 +723,7 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000007-1", traceIDStr), + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3TraceIDHeader, @@ -634,7 +741,7 @@ var injectHeader = []injectTest{ SpanID: spanID, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000008-0", traceIDStr), + trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3TraceIDHeader, @@ -653,7 +760,7 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-0000000000000009", traceIDStr), + trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3TraceIDHeader, @@ -663,6 +770,64 @@ var injectHeader = []injectTest{ trace.B3DebugFlagHeader, }, }, + { + name: "single: sampled only", + encoding: trace.SingleHeader, + parentSc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3SingleHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SingleHeader, + }, + }, + { + name: "single: debug", + encoding: trace.SingleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, + { + name: "single: debug omitting sample", + encoding: trace.SingleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled | trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3DebugFlagHeader, + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + trace.B3SingleHeader, + }, + }, { name: "single+multiple: sampled", encoding: trace.SingleHeader | trace.MultipleHeader, @@ -673,9 +838,9 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "000000000000000a", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "1", - trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000a-1", traceIDStr), + trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3ParentSpanIDHeader, @@ -691,9 +856,9 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "000000000000000b", + trace.B3SpanIDHeader: spanIDStr, trace.B3SampledHeader: "0", - trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000b-0", traceIDStr), + trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3ParentSpanIDHeader, @@ -710,8 +875,8 @@ var injectHeader = []injectTest{ }, wantHeaders: map[string]string{ trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: "000000000000000c", - trace.B3SingleHeader: fmt.Sprintf("%s-000000000000000c", traceIDStr), + trace.B3SpanIDHeader: spanIDStr, + trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ trace.B3SampledHeader, @@ -719,6 +884,61 @@ var injectHeader = []injectTest{ trace.B3DebugFlagHeader, }, }, + { + name: "single+multiple: sampled only", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceFlags: trace.FlagsSampled, + }, + wantHeaders: map[string]string{ + trace.B3SingleHeader: "1", + trace.B3SampledHeader: "1", + }, + doNotWantHeaders: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3ParentSpanIDHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "single+multiple: debug", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + }, + }, + { + name: "single+multiple: debug omitting sample", + encoding: trace.SingleHeader | trace.MultipleHeader, + parentSc: trace.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled | trace.FlagsDebug, + }, + wantHeaders: map[string]string{ + trace.B3TraceIDHeader: traceIDStr, + trace.B3SpanIDHeader: spanIDStr, + trace.B3DebugFlagHeader: "1", + trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + }, + doNotWantHeaders: []string{ + trace.B3SampledHeader, + trace.B3ParentSpanIDHeader, + }, + }, } var injectInvalidHeaderGenerator = []injectTest{ From 10b88329a40d53d0f413cf1beeaf1627b4561d82 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 11:13:21 -0700 Subject: [PATCH 27/33] Rename injectTest parentSc to sc This is no longer the parent. --- .../testtrace/b3_propagator_benchmark_test.go | 2 +- .../testtrace/b3_propagator_data_test.go | 68 +++++++++---------- api/trace/testtrace/b3_propagator_test.go | 2 +- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index b5d1a564df9..42870dc8d90 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/api/trace/testtrace/b3_propagator_benchmark_test.go @@ -78,7 +78,7 @@ func BenchmarkInjectB3(b *testing.B) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := trace.ContextWithSpan( context.Background(), - testSpan{sc: tt.parentSc}, + testSpan{sc: tt.sc}, ) b.ReportAllocs() b.ResetTimer() diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index ca458691986..156d04b4384 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -492,7 +492,7 @@ var extractInvalidHeaders = []extractTest{ type injectTest struct { name string encoding trace.B3Encoding - parentSc trace.SpanContext + sc trace.SpanContext wantHeaders map[string]string doNotWantHeaders []string } @@ -500,7 +500,7 @@ type injectTest struct { var injectHeader = []injectTest{ { name: "none: sampled", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, @@ -518,7 +518,7 @@ var injectHeader = []injectTest{ }, { name: "none: not sampled", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, }, @@ -535,7 +535,7 @@ var injectHeader = []injectTest{ }, { name: "none: unset sampled", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDeferred, @@ -553,7 +553,7 @@ var injectHeader = []injectTest{ }, { name: "none: sampled only", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ @@ -569,7 +569,7 @@ var injectHeader = []injectTest{ }, { name: "none: debug", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDebug, @@ -587,7 +587,7 @@ var injectHeader = []injectTest{ }, { name: "none: debug omitting sample", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled | trace.FlagsDebug, @@ -606,7 +606,7 @@ var injectHeader = []injectTest{ { name: "multiple: sampled", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, @@ -625,7 +625,7 @@ var injectHeader = []injectTest{ { name: "multiple: not sampled", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, }, @@ -643,7 +643,7 @@ var injectHeader = []injectTest{ { name: "multiple: unset sampled", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDeferred, @@ -662,7 +662,7 @@ var injectHeader = []injectTest{ { name: "multiple: sampled only", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ @@ -679,7 +679,7 @@ var injectHeader = []injectTest{ { name: "multiple: debug", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDebug, @@ -698,7 +698,7 @@ var injectHeader = []injectTest{ { name: "multiple: debug omitting sample", encoding: trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled | trace.FlagsDebug, @@ -717,7 +717,7 @@ var injectHeader = []injectTest{ { name: "single: sampled", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, @@ -736,7 +736,7 @@ var injectHeader = []injectTest{ { name: "single: not sampled", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, }, @@ -754,7 +754,7 @@ var injectHeader = []injectTest{ { name: "single: unset sampled", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDeferred, @@ -773,7 +773,7 @@ var injectHeader = []injectTest{ { name: "single: sampled only", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ @@ -791,7 +791,7 @@ var injectHeader = []injectTest{ { name: "single: debug", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDebug, @@ -811,7 +811,7 @@ var injectHeader = []injectTest{ { name: "single: debug omitting sample", encoding: trace.SingleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled | trace.FlagsDebug, @@ -831,7 +831,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: sampled", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, @@ -850,7 +850,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: not sampled", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, }, @@ -868,7 +868,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: unset sampled", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDeferred, @@ -887,7 +887,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: sampled only", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ @@ -904,7 +904,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: debug", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsDebug, @@ -923,7 +923,7 @@ var injectHeader = []injectTest{ { name: "single+multiple: debug omitting sample", encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled | trace.FlagsDebug, @@ -943,26 +943,26 @@ var injectHeader = []injectTest{ var injectInvalidHeaderGenerator = []injectTest{ { - name: "empty", - parentSc: trace.SpanContext{}, + name: "empty", + sc: trace.SpanContext{}, }, { name: "missing traceID", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ SpanID: spanID, TraceFlags: trace.FlagsSampled, }, }, { name: "missing spanID", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceID: traceID, TraceFlags: trace.FlagsSampled, }, }, { name: "missing traceID and spanID", - parentSc: trace.SpanContext{ + sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, }, @@ -986,25 +986,25 @@ func init() { for _, t := range injectInvalidHeaderGenerator { injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "none: " + t.name, - parentSc: t.parentSc, + sc: t.sc, doNotWantHeaders: allHeaders, }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "multiple: " + t.name, encoding: trace.MultipleHeader, - parentSc: t.parentSc, + sc: t.sc, doNotWantHeaders: allHeaders, }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "single: " + t.name, encoding: trace.SingleHeader, - parentSc: t.parentSc, + sc: t.sc, doNotWantHeaders: allHeaders, }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "single+multiple: " + t.name, encoding: trace.SingleHeader | trace.MultipleHeader, - parentSc: t.parentSc, + sc: t.sc, doNotWantHeaders: allHeaders, }) } diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 2687b4e0177..07965c46bb8 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -93,7 +93,7 @@ func TestInjectB3(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := trace.ContextWithSpan( context.Background(), - testSpan{sc: tt.parentSc}, + testSpan{sc: tt.sc}, ) propagator.Inject(ctx, req.Header) From 5deb38726e2b4a9272c98d7b4d81d952ab0146e8 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 2 Jul 2020 11:32:32 -0700 Subject: [PATCH 28/33] Update GetAllKeys for B3 --- api/trace/b3_propagator.go | 8 ++- api/trace/testtrace/b3_propagator_test.go | 65 ++++++++++++++++------- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 7faaaeb611b..3621bea3007 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -161,10 +161,14 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con } func (b3 B3) GetAllKeys() []string { + header := []string{} if b3.supports(SingleHeader) { - return []string{B3SingleHeader} + header = append(header, B3SingleHeader) } - return []string{B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader} + if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { + header = append(header, B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader, B3DebugFlagHeader) + } + return header } // extractMultiple reconstructs a SpanContext from header values based on B3 diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 07965c46bb8..872fd23a92a 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -115,25 +115,54 @@ func TestInjectB3(t *testing.T) { } func TestB3Propagator_GetAllKeys(t *testing.T) { - propagator := trace.B3{} - want := []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - } - got := propagator.GetAllKeys() - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("GetAllKeys: -got +want %s", diff) + tests := []struct { + name string + propagator trace.B3 + want []string + }{ + { + name: "no encoding specified", + propagator: trace.B3{}, + want: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3SampledHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "MultipleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.MultipleHeader}, + want: []string{ + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3SampledHeader, + trace.B3DebugFlagHeader, + }, + }, + { + name: "SingleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.SingleHeader}, + want: []string{ + trace.B3SingleHeader, + }, + }, + { + name: "SingleHeader and MultipleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, + want: []string{ + trace.B3SingleHeader, + trace.B3TraceIDHeader, + trace.B3SpanIDHeader, + trace.B3SampledHeader, + trace.B3DebugFlagHeader, + }, + }, } -} -func TestB3PropagatorWithSingleHeader_GetAllKeys(t *testing.T) { - propagator := trace.B3{InjectEncoding: trace.SingleHeader} - want := []string{ - trace.B3SingleHeader, - } - got := propagator.GetAllKeys() - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("GetAllKeys: -got +want %s", diff) + for _, test := range tests { + if diff := cmp.Diff(test.propagator.GetAllKeys(), test.want); diff != "" { + t.Errorf("%s: GetAllKeys: -got +want %s", test.name, diff) + } } } From 8651a693d8b91f2a4a0dc28eee8eb81636fb10eb Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 10:55:24 -0700 Subject: [PATCH 29/33] Un-Export the B3 headers The B3SingleHeader name will conflict with the upcoming change to prefix the SingleHeader encoding with "B3". There are a few options to address this conflict, but in the end we do not need to be exporting these values. They are duplicates of the OpenZipkin package and users should use those. --- CHANGELOG.md | 2 + api/trace/b3_propagator.go | 40 +- .../testtrace/b3_propagator_data_test.go | 525 +++++++++--------- api/trace/testtrace/b3_propagator_test.go | 28 +- 4 files changed, 303 insertions(+), 292 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de1ad0930b5..ac6c5b68757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `FlagsUnused` trace flag is removed. The purpose of this flag was to act as the inverse of `FlagsSampled`, the inverse of `FlagsSampled` is used instead. (#882) +- The B3 header constants (`B3SingleHeader`, `B3DebugFlagHeader`, `B3TraceIDHeader`, `B3SpanIDHeader`, `B3SampledHeader`, `B3ParentSpanIDHeader`) are removed. + If B3 header keys are needed [the authoritative OpenZipkin package constants](https://pkg.go.dev/github.com/openzipkin/zipkin-go@v0.2.2/propagation/b3?tab=doc#pkg-constants) should be used instead. (#882) ### Fixed diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 3621bea3007..f9c10e2a4e8 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -24,12 +24,12 @@ import ( const ( // Default B3 Header names. - B3SingleHeader = "b3" - B3DebugFlagHeader = "x-b3-flags" - B3TraceIDHeader = "x-b3-traceid" - B3SpanIDHeader = "x-b3-spanid" - B3SampledHeader = "x-b3-sampled" - B3ParentSpanIDHeader = "x-b3-parentspanid" + b3ContextHeader = "b3" + b3DebugFlagHeader = "x-b3-flags" + b3TraceIDHeader = "x-b3-traceid" + b3SpanIDHeader = "x-b3-spanid" + b3SampledHeader = "x-b3-sampled" + b3ParentSpanIDHeader = "x-b3-parentspanid" b3TraceIDPadding = "0000000000000000" ) @@ -108,23 +108,23 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { } } - supplier.Set(B3SingleHeader, strings.Join(header, "-")) + supplier.Set(b3ContextHeader, strings.Join(header, "-")) } if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { if sc.TraceID.IsValid() && sc.SpanID.IsValid() { - supplier.Set(B3TraceIDHeader, sc.TraceID.String()) - supplier.Set(B3SpanIDHeader, sc.SpanID.String()) + supplier.Set(b3TraceIDHeader, sc.TraceID.String()) + supplier.Set(b3SpanIDHeader, sc.SpanID.String()) } if sc.isDebug() { // Since Debug implies deferred, don't also send "X-B3-Sampled". - supplier.Set(B3DebugFlagHeader, "1") + supplier.Set(b3DebugFlagHeader, "1") } else if !sc.isDeferred() { if sc.IsSampled() { - supplier.Set(B3SampledHeader, "1") + supplier.Set(b3SampledHeader, "1") } else { - supplier.Set(B3SampledHeader, "0") + supplier.Set(b3SampledHeader, "0") } } } @@ -138,7 +138,7 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con ) // Default to Single Header if a valid value exists. - if h := supplier.Get(B3SingleHeader); h != "" { + if h := supplier.Get(b3ContextHeader); h != "" { sc, err = extractSingle(h) if err == nil && sc.IsValid() { return ContextWithRemoteSpanContext(ctx, sc) @@ -147,11 +147,11 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con } var ( - traceID = supplier.Get(B3TraceIDHeader) - spanID = supplier.Get(B3SpanIDHeader) - parentSpanID = supplier.Get(B3ParentSpanIDHeader) - sampled = supplier.Get(B3SampledHeader) - debugFlag = supplier.Get(B3DebugFlagHeader) + traceID = supplier.Get(b3TraceIDHeader) + spanID = supplier.Get(b3SpanIDHeader) + parentSpanID = supplier.Get(b3ParentSpanIDHeader) + sampled = supplier.Get(b3SampledHeader) + debugFlag = supplier.Get(b3DebugFlagHeader) ) sc, err = extractMultiple(traceID, spanID, parentSpanID, sampled, debugFlag) if err != nil || !sc.IsValid() { @@ -163,10 +163,10 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con func (b3 B3) GetAllKeys() []string { header := []string{} if b3.supports(SingleHeader) { - header = append(header, B3SingleHeader) + header = append(header, b3ContextHeader) } if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { - header = append(header, B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader, B3DebugFlagHeader) + header = append(header, b3TraceIDHeader, b3SpanIDHeader, b3SampledHeader, b3DebugFlagHeader) } return header } diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index 156d04b4384..9ce2a97b435 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -20,6 +20,15 @@ import ( "go.opentelemetry.io/otel/api/trace" ) +const ( + b3Context = "b3" + b3Flags = "x-b3-flags" + b3TraceID = "x-b3-traceid" + b3SpanID = "x-b3-spanid" + b3Sampled = "x-b3-sampled" + b3ParentSpanID = "x-b3-parentspanid" +) + type extractTest struct { name string headers map[string]string @@ -39,8 +48,8 @@ var extractHeaders = []extractTest{ { name: "multiple: sampling state defer", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -51,9 +60,9 @@ var extractHeaders = []extractTest{ { name: "multiple: sampling state deny", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -63,9 +72,9 @@ var extractHeaders = []extractTest{ { name: "multiple: sampling state accept", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -76,9 +85,9 @@ var extractHeaders = []extractTest{ { name: "multiple: sampling state as a boolean: true", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "true", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "true", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -89,9 +98,9 @@ var extractHeaders = []extractTest{ { name: "multiple: sampling state as a boolean: false", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "false", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "false", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -101,9 +110,9 @@ var extractHeaders = []extractTest{ { name: "multiple: debug flag set", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -114,10 +123,10 @@ var extractHeaders = []extractTest{ { name: "multiple: debug flag set to not 1 (ignored)", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", - trace.B3DebugFlagHeader: "2", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", + b3Flags: "2", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -131,10 +140,10 @@ var extractHeaders = []extractTest{ // deferred. name: "multiple: debug flag set and sampling state is deny", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", + b3Flags: "1", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -145,10 +154,10 @@ var extractHeaders = []extractTest{ { name: "multiple: with parent span id", headers: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", - trace.B3ParentSpanIDHeader: "00f067aa0ba90200", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", + b3ParentSpanID: "00f067aa0ba90200", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -159,15 +168,15 @@ var extractHeaders = []extractTest{ { name: "multiple: with only sampled state header", headers: map[string]string{ - trace.B3SampledHeader: "0", + b3Sampled: "0", }, wantSc: trace.EmptySpanContext(), }, { name: "multiple: left-padding 64-bit traceID", headers: map[string]string{ - trace.B3TraceIDHeader: "a3ce929d0e0e4736", - trace.B3SpanIDHeader: spanIDStr, + b3TraceID: "a3ce929d0e0e4736", + b3SpanID: spanIDStr, }, wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, @@ -178,7 +187,7 @@ var extractHeaders = []extractTest{ { name: "single: sampling state defer", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -189,7 +198,7 @@ var extractHeaders = []extractTest{ { name: "single: sampling state deny", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -199,7 +208,7 @@ var extractHeaders = []extractTest{ { name: "single: sampling state accept", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -210,7 +219,7 @@ var extractHeaders = []extractTest{ { name: "single: sampling state debug", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -221,7 +230,7 @@ var extractHeaders = []extractTest{ { name: "single: with parent span id", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1-00000000000000cd", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-1-00000000000000cd", traceIDStr, spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -232,14 +241,14 @@ var extractHeaders = []extractTest{ { name: "single: with only sampling state deny", headers: map[string]string{ - trace.B3SingleHeader: "0", + b3Context: "0", }, wantSc: trace.EmptySpanContext(), }, { name: "single: left-padding 64-bit traceID", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("a3ce929d0e0e4736-%s", spanIDStr), + b3Context: fmt.Sprintf("a3ce929d0e0e4736-%s", spanIDStr), }, wantSc: trace.SpanContext{ TraceID: traceID64bitPadded, @@ -250,10 +259,10 @@ var extractHeaders = []extractTest{ { name: "both single and multiple: single priority", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", + b3Context: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -265,10 +274,10 @@ var extractHeaders = []extractTest{ { name: "both single and multiple: invalid single", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-", traceIDStr, spanIDStr), - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", + b3Context: fmt.Sprintf("%s-%s-", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -279,10 +288,10 @@ var extractHeaders = []extractTest{ { name: "both single and multiple: invalid multiple", headers: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "invalid", + b3Context: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "invalid", }, wantSc: trace.SpanContext{ TraceID: traceID, @@ -296,195 +305,195 @@ var extractInvalidHeaders = []extractTest{ { name: "multiple: trace ID length > 32", headers: map[string]string{ - trace.B3TraceIDHeader: "ab00000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab00000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: trace ID length >16 and <32", headers: map[string]string{ - trace.B3TraceIDHeader: "ab0000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab0000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: trace ID length <16", headers: map[string]string{ - trace.B3TraceIDHeader: "ab0000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab0000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: wrong span ID length", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "cd0000000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "cd0000000000000000", + b3Sampled: "1", }, }, { name: "multiple: wrong sampled flag length", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "10", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "10", }, }, { name: "multiple: bogus trace ID", headers: map[string]string{ - trace.B3TraceIDHeader: "qw000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "qw000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: bogus span ID", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "qw00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "qw00000000000000", + b3Sampled: "1", }, }, { name: "multiple: bogus sampled flag", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "d", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "d", }, }, { name: "multiple: upper case trace ID", headers: map[string]string{ - trace.B3TraceIDHeader: "AB000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "AB000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: upper case span ID", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "CD00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "CD00000000000000", + b3Sampled: "1", }, }, { name: "multiple: zero trace ID", headers: map[string]string{ - trace.B3TraceIDHeader: "00000000000000000000000000000000", - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "00000000000000000000000000000000", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: zero span ID", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SpanIDHeader: "0000000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab000000000000000000000000000000", + b3SpanID: "0000000000000000", + b3Sampled: "1", }, }, { name: "multiple: missing span ID", headers: map[string]string{ - trace.B3TraceIDHeader: "ab000000000000000000000000000000", - trace.B3SampledHeader: "1", + b3TraceID: "ab000000000000000000000000000000", + b3Sampled: "1", }, }, { name: "multiple: missing trace ID", headers: map[string]string{ - trace.B3SpanIDHeader: "cd00000000000000", - trace.B3SampledHeader: "1", + b3SpanID: "cd00000000000000", + b3Sampled: "1", }, }, { name: "multiple: sampled header set to 1 but trace ID and span ID are missing", headers: map[string]string{ - trace.B3SampledHeader: "1", + b3Sampled: "1", }, }, { name: "single: wrong trace ID length", headers: map[string]string{ - trace.B3SingleHeader: "ab00000000000000000000000000000000-cd00000000000000-1", + b3Context: "ab00000000000000000000000000000000-cd00000000000000-1", }, }, { name: "single: wrong span ID length", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd0000000000000000-1", + b3Context: "ab000000000000000000000000000000-cd0000000000000000-1", }, }, { name: "single: wrong sampled state length", headers: map[string]string{ - trace.B3SingleHeader: "00-ab000000000000000000000000000000-cd00000000000000-01", + b3Context: "00-ab000000000000000000000000000000-cd00000000000000-01", }, }, { name: "single: wrong parent span ID length", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-cd0000000000000000", + b3Context: "ab000000000000000000000000000000-cd00000000000000-1-cd0000000000000000", }, }, { name: "single: bogus trace ID", headers: map[string]string{ - trace.B3SingleHeader: "qw000000000000000000000000000000-cd00000000000000-1", + b3Context: "qw000000000000000000000000000000-cd00000000000000-1", }, }, { name: "single: bogus span ID", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-qw00000000000000-1", + b3Context: "ab000000000000000000000000000000-qw00000000000000-1", }, }, { name: "single: bogus sampled flag", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-q", + b3Context: "ab000000000000000000000000000000-cd00000000000000-q", }, }, { name: "single: bogus parent span ID", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-qw00000000000000", + b3Context: "ab000000000000000000000000000000-cd00000000000000-1-qw00000000000000", }, }, { name: "single: upper case trace ID", headers: map[string]string{ - trace.B3SingleHeader: "AB000000000000000000000000000000-cd00000000000000-1", + b3Context: "AB000000000000000000000000000000-cd00000000000000-1", }, }, { name: "single: upper case span ID", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-CD00000000000000-1", + b3Context: "ab000000000000000000000000000000-CD00000000000000-1", }, }, { name: "single: upper case parent span ID", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-1-EF00000000000000", + b3Context: "ab000000000000000000000000000000-cd00000000000000-1-EF00000000000000", }, }, { name: "single: zero trace ID and span ID", headers: map[string]string{ - trace.B3SingleHeader: "00000000000000000000000000000000-0000000000000000-1", + b3Context: "00000000000000000000000000000000-0000000000000000-1", }, }, { name: "single: with sampling set to true", headers: map[string]string{ - trace.B3SingleHeader: "ab000000000000000000000000000000-cd00000000000000-true", + b3Context: "ab000000000000000000000000000000-cd00000000000000-true", }, }, } @@ -506,14 +515,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -523,14 +532,14 @@ var injectHeader = []injectTest{ SpanID: spanID, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -541,14 +550,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -557,14 +566,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SampledHeader: "1", + b3Sampled: "1", }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3TraceID, + b3SpanID, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -575,14 +584,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -593,14 +602,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled | trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -612,14 +621,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -630,14 +639,14 @@ var injectHeader = []injectTest{ SpanID: spanID, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -649,14 +658,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -666,14 +675,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SampledHeader: "1", + b3Sampled: "1", }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3TraceID, + b3SpanID, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -685,14 +694,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -704,14 +713,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled | trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -723,14 +732,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3ParentSpanID, + b3Flags, }, }, { @@ -741,14 +750,14 @@ var injectHeader = []injectTest{ SpanID: spanID, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3ParentSpanID, + b3Flags, }, }, { @@ -760,14 +769,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3ParentSpanID, + b3Flags, }, }, { @@ -777,15 +786,15 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: "1", + b3Context: "1", }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3Sampled, + b3TraceID, + b3SpanID, + b3ParentSpanID, + b3Flags, + b3Context, }, }, { @@ -797,15 +806,15 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3TraceID, + b3SpanID, + b3Flags, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -817,15 +826,15 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled | trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + b3Context: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3SingleHeader, + b3TraceID, + b3SpanID, + b3Flags, + b3Sampled, + b3ParentSpanID, + b3Context, }, }, { @@ -837,14 +846,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "1", - trace.B3SingleHeader: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "1", + b3Context: fmt.Sprintf("%s-%s-1", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3ParentSpanID, + b3Flags, }, }, { @@ -855,14 +864,14 @@ var injectHeader = []injectTest{ SpanID: spanID, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SampledHeader: "0", - trace.B3SingleHeader: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Sampled: "0", + b3Context: fmt.Sprintf("%s-%s-0", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3ParentSpanID, + b3Flags, }, }, { @@ -874,14 +883,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDeferred, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3SingleHeader: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Context: fmt.Sprintf("%s-%s", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3Sampled, + b3ParentSpanID, + b3Flags, }, }, { @@ -891,14 +900,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled, }, wantHeaders: map[string]string{ - trace.B3SingleHeader: "1", - trace.B3SampledHeader: "1", + b3Context: "1", + b3Sampled: "1", }, doNotWantHeaders: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3ParentSpanID, + b3Flags, }, }, { @@ -910,14 +919,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", - trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", + b3Context: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, + b3Sampled, + b3ParentSpanID, }, }, { @@ -929,14 +938,14 @@ var injectHeader = []injectTest{ TraceFlags: trace.FlagsSampled | trace.FlagsDebug, }, wantHeaders: map[string]string{ - trace.B3TraceIDHeader: traceIDStr, - trace.B3SpanIDHeader: spanIDStr, - trace.B3DebugFlagHeader: "1", - trace.B3SingleHeader: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), + b3TraceID: traceIDStr, + b3SpanID: spanIDStr, + b3Flags: "1", + b3Context: fmt.Sprintf("%s-%s-d", traceIDStr, spanIDStr), }, doNotWantHeaders: []string{ - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, + b3Sampled, + b3ParentSpanID, }, }, } @@ -975,12 +984,12 @@ func init() { // encoding values. injectInvalidHeader = make([]injectTest, 0, len(injectInvalidHeaderGenerator)*4) allHeaders := []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3ParentSpanIDHeader, - trace.B3DebugFlagHeader, - trace.B3SingleHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3ParentSpanID, + b3Flags, + b3Context, } // Nothing should be set for any header regardless of encoding. for _, t := range injectInvalidHeaderGenerator { diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 872fd23a92a..b9fc6c83534 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -124,38 +124,38 @@ func TestB3Propagator_GetAllKeys(t *testing.T) { name: "no encoding specified", propagator: trace.B3{}, want: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3Flags, }, }, { name: "MultipleHeader encoding specified", propagator: trace.B3{InjectEncoding: trace.MultipleHeader}, want: []string{ - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3DebugFlagHeader, + b3TraceID, + b3SpanID, + b3Sampled, + b3Flags, }, }, { name: "SingleHeader encoding specified", propagator: trace.B3{InjectEncoding: trace.SingleHeader}, want: []string{ - trace.B3SingleHeader, + b3Context, }, }, { name: "SingleHeader and MultipleHeader encoding specified", propagator: trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, want: []string{ - trace.B3SingleHeader, - trace.B3TraceIDHeader, - trace.B3SpanIDHeader, - trace.B3SampledHeader, - trace.B3DebugFlagHeader, + b3Context, + b3TraceID, + b3SpanID, + b3Sampled, + b3Flags, }, }, } From ee9c26602f87364858627be9de27bc024ee9e723 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 14:39:53 -0700 Subject: [PATCH 30/33] Rename B3 encodings and move support method to B3Encoding Include a `B3` prefix to scope the encoding names. Move the related support method to the B3Encoding itself, instead of the B3 propagator. Add tests to provide a sanity check for encoding bitmasks. --- api/trace/b3_propagator.go | 33 +++++++------- api/trace/b3_propagator_test.go | 43 +++++++++++++++++++ .../testtrace/b3_propagator_data_test.go | 42 +++++++++--------- api/trace/testtrace/b3_propagator_test.go | 20 ++++----- api/trace/testtrace/propagator_test.go | 4 +- 5 files changed, 94 insertions(+), 48 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index f9c10e2a4e8..ff4214f5cac 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -54,13 +54,20 @@ var ( // B3Encoding is a bitmask representation of the B3 encoding type. type B3Encoding uint8 +// supports returns if e has o bit(s) set. +func (e B3Encoding) supports(o B3Encoding) bool { + return e&o == o +} + const ( - // MultipleHeader is a B3 encoding that uses multiple headers to + // B3MultipleHeader is a B3 encoding that uses multiple headers to // transmit tracing information all prefixed with `x-b3-`. - MultipleHeader B3Encoding = 1 - // SingleHeader is a B3 encoding that uses a single header named `b3 to - // transmit tracing information. - SingleHeader B3Encoding = 2 + B3MultipleHeader B3Encoding = 1 << iota + // B3SingleHeader is a B3 encoding that uses a single header named `b3` + // to transmit tracing information. + B3SingleHeader + // B3Unspecified is an unspecified B3 encoding. + B3Unspecified B3Encoding = 0 ) // B3 propagator serializes SpanContext to/from B3 Headers. @@ -75,15 +82,11 @@ const ( // x-b3-flags: {DebugFlag} type B3 struct { // InjectEncoding are the B3 encodings used when injecting trace - // information. If no encoding is specific it defaults to - // `MultipleHeader`. + // information. If no encoding is specific (i.e. `B3Unspecified`) + // `B3MultipleHeader` will be used as the default. InjectEncoding B3Encoding } -func (b3 B3) supports(e B3Encoding) bool { - return b3.InjectEncoding&e != 0 -} - var _ propagation.HTTPPropagator = B3{} // Inject injects a context into the supplier as B3 headers. @@ -92,7 +95,7 @@ var _ propagation.HTTPPropagator = B3{} func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { sc := SpanFromContext(ctx).SpanContext() - if b3.supports(SingleHeader) { + if b3.InjectEncoding.supports(B3SingleHeader) { header := []string{} if sc.TraceID.IsValid() && sc.SpanID.IsValid() { header = append(header, sc.TraceID.String(), sc.SpanID.String()) @@ -111,7 +114,7 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { supplier.Set(b3ContextHeader, strings.Join(header, "-")) } - if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { + if b3.InjectEncoding.supports(B3MultipleHeader) || b3.InjectEncoding == B3Unspecified { if sc.TraceID.IsValid() && sc.SpanID.IsValid() { supplier.Set(b3TraceIDHeader, sc.TraceID.String()) supplier.Set(b3SpanIDHeader, sc.SpanID.String()) @@ -162,10 +165,10 @@ func (b3 B3) Extract(ctx context.Context, supplier propagation.HTTPSupplier) con func (b3 B3) GetAllKeys() []string { header := []string{} - if b3.supports(SingleHeader) { + if b3.InjectEncoding.supports(B3SingleHeader) { header = append(header, b3ContextHeader) } - if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 { + if b3.InjectEncoding.supports(B3MultipleHeader) || b3.InjectEncoding == B3Unspecified { header = append(header, b3TraceIDHeader, b3SpanIDHeader, b3SampledHeader, b3DebugFlagHeader) } return header diff --git a/api/trace/b3_propagator_test.go b/api/trace/b3_propagator_test.go index 9dc44cc534a..b558a76b75e 100644 --- a/api/trace/b3_propagator_test.go +++ b/api/trace/b3_propagator_test.go @@ -279,3 +279,46 @@ func TestExtractSingle(t *testing.T) { assert.Equal(t, test.expected, actual, "header: %s", test.header) } } + +func TestB3EncodingOperations(t *testing.T) { + encodings := []B3Encoding{ + B3MultipleHeader, + B3SingleHeader, + B3Unspecified, + } + + // Test for overflow (or something really unexpected). + for i, e := range encodings { + for j := i + 1; j < i+len(encodings); j++ { + o := encodings[j%len(encodings)] + assert.False(t, e == o, "%v == %v", e, o) + } + } + + // B3Unspecified is a special case, it supports only itself, but is + // supported by everything. + assert.True(t, B3Unspecified.supports(B3Unspecified)) + for _, e := range encodings[:len(encodings)-1] { + assert.False(t, B3Unspecified.supports(e), e) + assert.True(t, e.supports(B3Unspecified), e) + } + + // Skip the special case for B3Unspecified. + for i, e := range encodings[:len(encodings)-1] { + // Everything should support itself. + assert.True(t, e.supports(e)) + for j := i + 1; j < i+len(encodings); j++ { + o := encodings[j%len(encodings)] + // Any "or" combination should be supportive of an operand. + assert.True(t, (e | o).supports(e), "(%[0]v|%[1]v).supports(%[0]v)", e, o) + // Bitmasks should be unique. + assert.False(t, o.supports(e), "%v.supports(%v)", o, e) + } + } + + // B3Encoding.supports should be more inclusive than equality. + all := ^B3Unspecified + for _, e := range encodings { + assert.True(t, all.supports(e)) + } +} diff --git a/api/trace/testtrace/b3_propagator_data_test.go b/api/trace/testtrace/b3_propagator_data_test.go index 9ce2a97b435..262bbe8be6e 100644 --- a/api/trace/testtrace/b3_propagator_data_test.go +++ b/api/trace/testtrace/b3_propagator_data_test.go @@ -614,7 +614,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: sampled", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -633,7 +633,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: not sampled", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -651,7 +651,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: unset sampled", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -670,7 +670,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: sampled only", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, @@ -687,7 +687,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: debug", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -706,7 +706,7 @@ var injectHeader = []injectTest{ }, { name: "multiple: debug omitting sample", - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -725,7 +725,7 @@ var injectHeader = []injectTest{ }, { name: "single: sampled", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -744,7 +744,7 @@ var injectHeader = []injectTest{ }, { name: "single: not sampled", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -762,7 +762,7 @@ var injectHeader = []injectTest{ }, { name: "single: unset sampled", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -781,7 +781,7 @@ var injectHeader = []injectTest{ }, { name: "single: sampled only", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, @@ -799,7 +799,7 @@ var injectHeader = []injectTest{ }, { name: "single: debug", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -819,7 +819,7 @@ var injectHeader = []injectTest{ }, { name: "single: debug omitting sample", - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -839,7 +839,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: sampled", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -858,7 +858,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: not sampled", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -876,7 +876,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: unset sampled", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -895,7 +895,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: sampled only", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceFlags: trace.FlagsSampled, }, @@ -912,7 +912,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: debug", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -931,7 +931,7 @@ var injectHeader = []injectTest{ }, { name: "single+multiple: debug omitting sample", - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -1000,19 +1000,19 @@ func init() { }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "multiple: " + t.name, - encoding: trace.MultipleHeader, + encoding: trace.B3MultipleHeader, sc: t.sc, doNotWantHeaders: allHeaders, }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "single: " + t.name, - encoding: trace.SingleHeader, + encoding: trace.B3SingleHeader, sc: t.sc, doNotWantHeaders: allHeaders, }) injectInvalidHeader = append(injectInvalidHeader, injectTest{ name: "single+multiple: " + t.name, - encoding: trace.SingleHeader | trace.MultipleHeader, + encoding: trace.B3SingleHeader | trace.B3MultipleHeader, sc: t.sc, doNotWantHeaders: allHeaders, }) diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index b9fc6c83534..5384a145557 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/api/trace/testtrace/b3_propagator_test.go @@ -31,11 +31,11 @@ func TestExtractB3(t *testing.T) { tests []extractTest }{ { - name: "valid headers", + name: "valid extract headers", tests: extractHeaders, }, { - name: "invalid headers", + name: "invalid extract headers", tests: extractInvalidHeaders, }, } @@ -77,11 +77,11 @@ func TestInjectB3(t *testing.T) { tests []injectTest }{ { - name: "valid headers", + name: "valid inject headers", tests: injectHeader, }, { - name: "invalid headers", + name: "invalid inject headers", tests: injectInvalidHeader, }, } @@ -131,8 +131,8 @@ func TestB3Propagator_GetAllKeys(t *testing.T) { }, }, { - name: "MultipleHeader encoding specified", - propagator: trace.B3{InjectEncoding: trace.MultipleHeader}, + name: "B3MultipleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.B3MultipleHeader}, want: []string{ b3TraceID, b3SpanID, @@ -141,15 +141,15 @@ func TestB3Propagator_GetAllKeys(t *testing.T) { }, }, { - name: "SingleHeader encoding specified", - propagator: trace.B3{InjectEncoding: trace.SingleHeader}, + name: "B3SingleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.B3SingleHeader}, want: []string{ b3Context, }, }, { - name: "SingleHeader and MultipleHeader encoding specified", - propagator: trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, + name: "B3SingleHeader and B3MultipleHeader encoding specified", + propagator: trace.B3{InjectEncoding: trace.B3SingleHeader | trace.B3MultipleHeader}, want: []string{ b3Context, b3TraceID, diff --git a/api/trace/testtrace/propagator_test.go b/api/trace/testtrace/propagator_test.go index 43a620bfbb3..4cbab89fc9f 100644 --- a/api/trace/testtrace/propagator_test.go +++ b/api/trace/testtrace/propagator_test.go @@ -67,8 +67,8 @@ func TestMultiplePropagators(t *testing.T) { testProps := []propagation.HTTPPropagator{ trace.TraceContext{}, trace.B3{}, - trace.B3{InjectEncoding: trace.SingleHeader}, - trace.B3{InjectEncoding: trace.SingleHeader | trace.MultipleHeader}, + trace.B3{InjectEncoding: trace.B3SingleHeader}, + trace.B3{InjectEncoding: trace.B3SingleHeader | trace.B3MultipleHeader}, } bg := context.Background() // sanity check of oota propagator, ensuring that it really From 68e60e8d155ea1a762e4e0a2d030fd55dea28193 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 14:44:15 -0700 Subject: [PATCH 31/33] Update span_context_test tests Update test name to better describe how unused bits have no affect on the sampling decision. Include the inverse of this test as well: not sampled but has unused bits. --- api/trace/span_context_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/trace/span_context_test.go b/api/trace/span_context_test.go index bdfcc8460d9..39e85439b2a 100644 --- a/api/trace/span_context_test.go +++ b/api/trace/span_context_test.go @@ -173,7 +173,14 @@ func TestSpanContextIsSampled(t *testing.T) { }, want: true, }, { - name: "sampled plus unused", + name: "unused bits are ignored, still not sampled", + sc: trace.SpanContext{ + TraceID: trace.ID([16]byte{1}), + TraceFlags: ^trace.FlagsSampled, + }, + want: false, + }, { + name: "unused bits are ignored, still sampled", sc: trace.SpanContext{ TraceID: trace.ID([16]byte{1}), TraceFlags: trace.FlagsSampled | ^trace.FlagsSampled, From 57c66695df1be6c7916355ae677d592e08b48e97 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 15:37:12 -0700 Subject: [PATCH 32/33] Use named const for Single Header decoding widths --- api/trace/b3_propagator.go | 44 ++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index ff4214f5cac..1eacbb7bc18 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -32,6 +32,14 @@ const ( b3ParentSpanIDHeader = "x-b3-parentspanid" b3TraceIDPadding = "0000000000000000" + + // B3 Single Header encoding widths. + separatorWidth = 1 // Single "-" character. + samplingWidth = 1 // Single hex character. + traceID64BitsWidth = 64 / 4 // 16 hex character Trace ID. + traceID128BitsWidth = 128 / 4 // 32 hex character Trace ID. + spanIDWidth = 16 // 16 hex character ID. + parentSpanIDWidth = 16 // 16 hex character ID. ) var ( @@ -257,21 +265,22 @@ func extractSingle(contextHeader string) (SpanContext, error) { headerLen := len(contextHeader) - if headerLen == 1 { + if headerLen == samplingWidth { sampling = contextHeader - } else if headerLen == 16 || headerLen == 32 { + } else if headerLen == traceID64BitsWidth || headerLen == traceID128BitsWidth { + // Trace ID by itself is invalid. return empty, errInvalidScope - } else if headerLen >= 16+16+1 { + } else if headerLen >= traceID64BitsWidth+spanIDWidth+separatorWidth { pos := 0 var traceID string - if string(contextHeader[16]) == "-" { + if string(contextHeader[traceID64BitsWidth]) == "-" { // traceID must be 64 bits - pos += 16 + 1 // {traceID}- - traceID = b3TraceIDPadding + string(contextHeader[0:16]) + pos += traceID64BitsWidth // {traceID} + traceID = b3TraceIDPadding + string(contextHeader[0:pos]) } else if string(contextHeader[32]) == "-" { // traceID must be 128 bits - pos += 32 + 1 // {traceID}- - traceID = string(contextHeader[0:32]) + pos += traceID128BitsWidth // {traceID} + traceID = string(contextHeader[0:pos]) } else { return empty, errInvalidTraceIDValue } @@ -280,26 +289,29 @@ func extractSingle(contextHeader string) (SpanContext, error) { if err != nil { return empty, errInvalidTraceIDValue } + pos += separatorWidth // {traceID}- - sc.SpanID, err = SpanIDFromHex(contextHeader[pos : pos+16]) + sc.SpanID, err = SpanIDFromHex(contextHeader[pos : pos+spanIDWidth]) if err != nil { return empty, errInvalidSpanIDValue } - pos += 16 // {traceID}-{spanID} + pos += spanIDWidth // {traceID}-{spanID} if headerLen > pos { - if headerLen == pos+1 { + if headerLen == pos+separatorWidth { + // {traceID}-{spanID}- is invalid. return empty, errInvalidSampledByte } - pos++ // {traceID}-{spanID}- + pos += separatorWidth // {traceID}-{spanID}- - if headerLen == pos+1 { + if headerLen == pos+samplingWidth { sampling = string(contextHeader[pos]) - } else if headerLen == pos+16 { + } else if headerLen == pos+parentSpanIDWidth { + // {traceID}-{spanID}-{parentSpanID} is invalid. return empty, errInvalidScopeParentSingle - } else if headerLen == pos+1+16+1 { + } else if headerLen == pos+samplingWidth+separatorWidth+parentSpanIDWidth { sampling = string(contextHeader[pos]) - pos += 1 + 1 // {traceID}-{spanID}-{sampling}- + pos += samplingWidth + separatorWidth // {traceID}-{spanID}-{sampling}- // Validate parent span ID but we do not use it so do not // save it. From 79be79187c1e90a02d1956975ff7b9ea3b373cd7 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 15:43:10 -0700 Subject: [PATCH 33/33] Update api/trace/b3_propagator.go Co-authored-by: Anthony Mirabella --- api/trace/b3_propagator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 1eacbb7bc18..4627618f70b 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -90,7 +90,7 @@ const ( // x-b3-flags: {DebugFlag} type B3 struct { // InjectEncoding are the B3 encodings used when injecting trace - // information. If no encoding is specific (i.e. `B3Unspecified`) + // information. If no encoding is specified (i.e. `B3Unspecified`) // `B3MultipleHeader` will be used as the default. InjectEncoding B3Encoding }