Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix B3 propagator and add tests #882

Merged
merged 38 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5efea1c
Correct B3 propagators and add tests
MrAlias Jul 1, 2020
5b14cc4
Break up external integration and internal unit tests
MrAlias Jul 1, 2020
08e9a40
Merge remote-tracking branch 'upstream/master' into b3-header-tests
MrAlias Jul 1, 2020
0a7a3fa
Add changes to Changelog.
MrAlias Jul 1, 2020
a89f8e6
Update Changelog with PR number
MrAlias Jul 1, 2020
bda1e3e
Fix lint issues
MrAlias Jul 1, 2020
f16dc0c
Update trace flags
MrAlias Jul 1, 2020
a96d02f
Update extractSingle to support unset sampling
MrAlias Jul 1, 2020
6c6a7b2
Update existing tests to appropriately use FlagsUnset
MrAlias Jul 1, 2020
405be1a
Remove bogus debug flag test
MrAlias Jul 1, 2020
be56698
B3 Extract now supports parsing both headers
MrAlias Jul 1, 2020
e7d5ed7
Feedback
MrAlias Jul 1, 2020
16e4bc1
Merge remote-tracking branch 'upstream/master' into b3-header-tests
MrAlias Jul 1, 2020
3f1ea36
Switch to bitmask inject encoding field
MrAlias Jul 1, 2020
614e3c1
Add comments
MrAlias Jul 1, 2020
10c345b
Migrate B3 integration tests to existing testtrace
MrAlias Jul 2, 2020
2c37f31
Merge branch 'master' into b3-header-tests
MrAlias Jul 2, 2020
727af41
Update comment
MrAlias Jul 2, 2020
22c4a48
Benchmark invalid B3 injects as well
MrAlias Jul 2, 2020
b6a2a14
Update trace flags
MrAlias Jul 2, 2020
712f879
Revert SpanContext.IsSampled back
MrAlias Jul 2, 2020
49335f9
Add comment to b3 test data generation
MrAlias Jul 2, 2020
ee571de
Update Changelog
MrAlias Jul 2, 2020
b2f3075
Fix trace flag name in Changelog
MrAlias Jul 2, 2020
6b44a10
Fix Changelog formatting
MrAlias Jul 2, 2020
3f92d8c
Update Changelog
MrAlias Jul 2, 2020
b178c20
Remove valid check at start of B3 injectg
MrAlias Jul 2, 2020
e882f6f
Update B3 inject integration tests
MrAlias Jul 2, 2020
f7d73cd
Update B3 integration tests
MrAlias Jul 2, 2020
10b8832
Rename injectTest parentSc to sc
MrAlias Jul 2, 2020
5deb387
Update GetAllKeys for B3
MrAlias Jul 2, 2020
e2868bd
Merge remote-tracking branch 'upstream/master' into b3-header-tests
MrAlias Jul 7, 2020
8651a69
Un-Export the B3 headers
MrAlias Jul 7, 2020
ee9c266
Rename B3 encodings and move support method to B3Encoding
MrAlias Jul 7, 2020
68e60e8
Update span_context_test tests
MrAlias Jul 7, 2020
57c6669
Use named const for Single Header decoding widths
MrAlias Jul 7, 2020
79be791
Update api/trace/b3_propagator.go
MrAlias Jul 7, 2020
cc5dae2
Merge branch 'master' into b3-header-tests
MrAlias Jul 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
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. (#882)
- The `FlagsDebug` trace flag to indicate if the trace is a debug trace. (#882)
- Add `peer.service` semantic attribute. (#898)
- Add database-specific semantic attributes. (#899)
- Add semantic convention for `faas.coldstart` and `container.id`. (#909)

### Changed

Expand All @@ -30,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

Expand All @@ -39,6 +44,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
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 does not set the `X-B3-Sampling` header when injecting. (#882)
- Ensure span status is not set to `Unknown` when no HTTP status code is provided as it is assumed to be `200 OK`. (#908)
- Ensure `httptrace.clientTracer` closes `http.headers` span. (#912)

## [0.7.0] - 2020-06-26

Expand Down
73 changes: 38 additions & 35 deletions api/trace/b3_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.
Expand All @@ -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`)
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
// `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.
Expand All @@ -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())
Expand All @@ -108,23 +111,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 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())
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")
}
}
}
Expand All @@ -138,7 +141,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)
Expand All @@ -147,11 +150,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() {
Expand All @@ -162,11 +165,11 @@ 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)
if b3.InjectEncoding.supports(B3SingleHeader) {
header = append(header, b3ContextHeader)
}
if b3.supports(MultipleHeader) || b3.InjectEncoding == 0 {
header = append(header, B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader, B3DebugFlagHeader)
if b3.InjectEncoding.supports(B3MultipleHeader) || b3.InjectEncoding == B3Unspecified {
header = append(header, b3TraceIDHeader, b3SpanIDHeader, b3SampledHeader, b3DebugFlagHeader)
}
return header
}
Expand Down
43 changes: 43 additions & 0 deletions api/trace/b3_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
9 changes: 8 additions & 1 deletion api/trace/span_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading