From 71b6d7fc42c235ea005fde85c23fb84df36d3e60 Mon Sep 17 00:00:00 2001
From: Zhongyang Wu <zhongyang.wu@outlook.com>
Date: Tue, 29 Sep 2020 10:48:48 -0400
Subject: [PATCH] [propagator] Set sample bitmask when sampling decision is
 debug for B3 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>
---
 propagators/b3/b3_data_test.go       |  9 ++++-----
 propagators/b3/b3_propagator.go      | 12 ++++++++----
 propagators/b3/b3_propagator_test.go |  6 +++---
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/propagators/b3/b3_data_test.go b/propagators/b3/b3_data_test.go
index 21a4029cecc..814554939d0 100644
--- a/propagators/b3/b3_data_test.go
+++ b/propagators/b3/b3_data_test.go
@@ -144,7 +144,7 @@ var extractHeaders = []extractTest{
 		wantSc: trace.SpanContext{
 			TraceID:    traceID,
 			SpanID:     spanID,
-			TraceFlags: trace.FlagsDeferred | trace.FlagsDebug,
+			TraceFlags: trace.FlagsSampled | trace.FlagsDebug,
 		},
 	},
 	{
@@ -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,
@@ -175,7 +174,7 @@ var extractHeaders = []extractTest{
 		wantSc: trace.SpanContext{
 			TraceID:    traceID,
 			SpanID:     spanID,
-			TraceFlags: trace.FlagsDebug,
+			TraceFlags: trace.FlagsDebug | trace.FlagsSampled,
 		},
 	},
 	{
@@ -251,7 +250,7 @@ var extractHeaders = []extractTest{
 		wantSc: trace.SpanContext{
 			TraceID:    traceID,
 			SpanID:     spanID,
-			TraceFlags: trace.FlagsDebug,
+			TraceFlags: trace.FlagsDebug | trace.FlagsSampled,
 		},
 	},
 	{
diff --git a/propagators/b3/b3_propagator.go b/propagators/b3/b3_propagator.go
index 12682c3aac1..50aebf78114 100644
--- a/propagators/b3/b3_propagator.go
+++ b/propagators/b3/b3_propagator.go
@@ -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 != "" {
@@ -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":
diff --git a/propagators/b3/b3_propagator_test.go b/propagators/b3/b3_propagator_test.go
index e31f5818996..3ffa22585c0 100644
--- a/propagators/b3/b3_propagator_test.go
+++ b/propagators/b3/b3_propagator_test.go
@@ -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,
 		},
 		{
@@ -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},