Skip to content

Commit

Permalink
[propagator] Set sample bitmask when sampling decision is debug for B…
Browse files Browse the repository at this point in the history
…3 Propagator. (#369)

According to the spec. Debug sampling decision implies accept(sampled) decision.

But we didn't set sample bitmask. So when trace state is debug, `IsSampled` method will still return false, which is different from spec.

Now we make sure when debug bitmask is set, sample bitmask will also be set.

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
TommyCpp and MrAlias committed Sep 29, 2020
1 parent 9dc5e0c commit 71b6d7f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
9 changes: 4 additions & 5 deletions propagators/b3/b3_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ var extractHeaders = []extractTest{
wantSc: trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceFlags: trace.FlagsDeferred | trace.FlagsDebug,
TraceFlags: trace.FlagsSampled | trace.FlagsDebug,
},
},
{
Expand All @@ -163,8 +163,7 @@ var extractHeaders = []extractTest{
},
{
// spec explicitly states "Debug implies an accept decision, so don't
// also send the X-B3-Sampled header", make sure sampling is
// deferred.
// also send the X-B3-Sampled header", make sure sampling is set in this case.
name: "multiple: debug flag set and sampling state is deny",
headers: map[string]string{
b3TraceID: traceIDStr,
Expand All @@ -175,7 +174,7 @@ var extractHeaders = []extractTest{
wantSc: trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceFlags: trace.FlagsDebug,
TraceFlags: trace.FlagsDebug | trace.FlagsSampled,
},
},
{
Expand Down Expand Up @@ -251,7 +250,7 @@ var extractHeaders = []extractTest{
wantSc: trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceFlags: trace.FlagsDebug,
TraceFlags: trace.FlagsDebug | trace.FlagsSampled,
},
},
{
Expand Down
12 changes: 8 additions & 4 deletions propagators/b3/b3_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,14 @@ func extractMultiple(traceID, spanID, parentSpanID, sampled, flags string) (trac
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 bitmask and
// sampled bitmask to 1 since debug implicitly means sampled. All other
// values and omission of header will be ignored. According to the spec. User
// shouldn't send X-B3-Sampled header along with X-B3-Flags header. Thus we will
// ignore X-B3-Sampled header when X-B3-Flags header is sent and valid.
if flags == "1" {
sc.TraceFlags |= trace.FlagsDebug
sc.TraceFlags |= trace.FlagsDebug | trace.FlagsSampled
sc.TraceFlags &= ^trace.FlagsDeferred
}

if traceID != "" {
Expand Down Expand Up @@ -331,7 +335,7 @@ func extractSingle(contextHeader string) (trace.SpanContext, error) {
case "":
sc.TraceFlags = trace.FlagsDeferred
case "d":
sc.TraceFlags = trace.FlagsDebug
sc.TraceFlags = trace.FlagsDebug | trace.FlagsSampled
case "1":
sc.TraceFlags = trace.FlagsSampled
case "0":
Expand Down
6 changes: 3 additions & 3 deletions propagators/b3/b3_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func TestExtractMultiple(t *testing.T) {
},
{
"", "", "", "", "1",
trace.SpanContext{TraceFlags: trace.FlagsDeferred | trace.FlagsDebug},
trace.SpanContext{TraceFlags: trace.FlagsSampled | trace.FlagsDebug},
nil,
},
{
"", "", "", "0", "1",
trace.SpanContext{TraceFlags: trace.FlagsDebug},
trace.SpanContext{TraceFlags: trace.FlagsDebug | trace.FlagsSampled},
nil,
},
{
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestExtractSingle(t *testing.T) {
}{
{"0", trace.SpanContext{}, nil},
{"1", trace.SpanContext{TraceFlags: trace.FlagsSampled}, nil},
{"d", trace.SpanContext{TraceFlags: trace.FlagsDebug}, nil},
{"d", trace.SpanContext{TraceFlags: trace.FlagsDebug | trace.FlagsSampled}, nil},
{"a", empty, errInvalidSampledByte},
{"3", empty, errInvalidSampledByte},
{"000000000000007b", empty, errInvalidScope},
Expand Down

0 comments on commit 71b6d7f

Please sign in to comment.