Simplify trace_context tests (#2108)

* Simplify trace_context tests

Refactor the propagation/trace_context tests to be targeted at testing
the propagator functionality only and remove extraneous dependencies.

Fix the TraceContext Inject method from injecting the tracestate header
if no value exists.

* Add changes to changelog

CHANGELOG.md

@@ -31,6 +31,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - OS type detector now sets the correct `dragonflybsd` value for DragonFly BSD. (#2092)
 - The OTel span status is correctly transformed into the OTLP status in the `go.opentelemetry.io/otel/exporters/otlp/otlptrace` package.
   This fix will by default set the status to `Unset` if it is not explicitly set to `Ok` or `Error`. (#2099 #2102)
+- The `Inject` method for the `"go.opentelemetry.io/otel/propagation".TraceContext` type no longer injects empty `tracestate` values. (#2108)
 
 ### Security
 

propagation/trace_context.go

@@ -50,7 +50,9 @@ func (tc TraceContext) Inject(ctx context.Context, carrier TextMapCarrier) {
 		return
 	}
 
-	carrier.Set(tracestateHeader, sc.TraceState().String())
+	if ts := sc.TraceState().String(); ts != "" {
+		carrier.Set(tracestateHeader, ts)
+	}
 
 	// Clear all flags other than the trace-context supported sampling bit.
 	flags := sc.TraceFlags() & trace.FlagsSampled

propagation/trace_context_test.go

@@ -19,34 +19,49 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
-	"go.opentelemetry.io/otel/attribute"
-	"go.opentelemetry.io/otel/oteltest"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 )
 
-func TestExtractValidTraceContextFromHTTPReq(t *testing.T) {
+var (
-	prop := propagation.TraceContext{}
+	traceparent = http.CanonicalHeaderKey("traceparent")
-	tests := []struct {
+	tracestate  = http.CanonicalHeaderKey("tracestate")
-		name   string
+
-		header string
+	prop = propagation.TraceContext{}
-		wantSc trace.SpanContext
+)
-	}{
+
+type testcase struct {
+	name   string
+	header http.Header
+	sc     trace.SpanContext
+}
+
+func TestExtractValidTraceContext(t *testing.T) {
+	stateStr := "key1=value1,key2=value2"
+	state, err := trace.ParseTraceState(stateStr)
+	require.NoError(t, err)
+
+	tests := []testcase{
 		{
-			name:   "valid w3cHeader",
+			name: "not sampled",
-			header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID: traceID,
 				SpanID:  spanID,
 				Remote:  true,
 			}),
 		},
 		{
-			name:   "valid w3cHeader and sampled",
+			name: "sampled",
-			header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceFlags: trace.FlagsSampled,
@@ -54,38 +69,47 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) {
 			}),
 		},
 		{
-			name:   "future version",
+			name: "valid tracestate",
-			header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
+				tracestate:  []string{stateStr},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
-				TraceFlags: trace.FlagsSampled,
+				TraceState: state,
 				Remote:     true,
 			}),
 		},
 		{
-			name:   "future options with sampled bit set",
+			name: "invalid tracestate perserves traceparent",
-			header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
-				TraceID:    traceID,
+				tracestate:  []string{"invalid$@#=invalid"},
-				SpanID:     spanID,
+			},
-				TraceFlags: trace.FlagsSampled,
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
-				Remote:     true,
-			}),
-		},
-		{
-			name:   "future options with sampled bit cleared",
-			header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-08",
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID: traceID,
 				SpanID:  spanID,
 				Remote:  true,
 			}),
 		},
 		{
-			name:   "future additional data",
+			name: "future version not sampled",
-			header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09-XYZxsf09",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID: traceID,
+				SpanID:  spanID,
+				Remote:  true,
+			}),
+		},
+		{
+			name: "future version sampled",
+			header: http.Header{
+				traceparent: []string{"02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceFlags: trace.FlagsSampled,
@@ -93,9 +117,11 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) {
 			}),
 		},
 		{
-			name:   "valid b3Header ending in dash",
+			name: "future version sample bit set",
-			header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01-",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceFlags: trace.FlagsSampled,
@@ -103,35 +129,61 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) {
 			}),
 		},
 		{
-			name:   "future valid b3Header ending in dash",
+			name: "future version sample bit not set",
-			header: "01-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09-",
+			header: http.Header{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
+				traceparent: []string{"02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-08"},
-				TraceID:    traceID,
+			},
-				SpanID:     spanID,
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
-				TraceFlags: trace.FlagsSampled,
+				TraceID: traceID,
-				Remote:     true,
+				SpanID:  spanID,
+				Remote:  true,
+			}),
+		},
+		{
+			name: "future version additional data",
+			header: http.Header{
+				traceparent: []string{"02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00-XYZxsf09"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID: traceID,
+				SpanID:  spanID,
+				Remote:  true,
+			}),
+		},
+		{
+			name: "B3 format ending in dash",
+			header: http.Header{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00-"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID: traceID,
+				SpanID:  spanID,
+				Remote:  true,
+			}),
+		},
+		{
+			name: "future version B3 format ending in dash",
+			header: http.Header{
+				traceparent: []string{"03-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00-"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID: traceID,
+				SpanID:  spanID,
+				Remote:  true,
 			}),
 		},
 	}
 
-	for _, tt := range tests {
+	for _, tc := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
-			req, _ := http.NewRequest("GET", "http://example.com", nil)
-			req.Header.Set("traceparent", tt.header)
-
 			ctx := context.Background()
-			ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header))
+			ctx = prop.Extract(ctx, propagation.HeaderCarrier(tc.header))
-			gotSc := trace.SpanContextFromContext(ctx)
+			assert.Equal(t, tc.sc, trace.SpanContextFromContext(ctx))
-			if diff := cmp.Diff(gotSc, tt.wantSc, cmp.Comparer(func(sc, other trace.SpanContext) bool { return sc.Equal(other) })); diff != "" {
-				t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff)
-			}
 		})
 	}
 }
 
 func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) {
-	wantSc := trace.SpanContext{}
-	prop := propagation.TraceContext{}
 	tests := []struct {
 		name   string
 		header string
@@ -202,172 +254,100 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) {
 		},
 	}
 
+	empty := trace.SpanContext{}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			req, _ := http.NewRequest("GET", "http://example.com", nil)
+			h := http.Header{traceparent: []string{tt.header}}
-			req.Header.Set("traceparent", tt.header)
-
 			ctx := context.Background()
-			ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header))
+			ctx = prop.Extract(ctx, propagation.HeaderCarrier(h))
-			gotSc := trace.SpanContextFromContext(ctx)
+
-			if diff := cmp.Diff(gotSc, wantSc, cmp.AllowUnexported(trace.TraceState{})); diff != "" {
+			// Failure to extract needs to result in no SpanContext being set.
-				t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff)
+			// This cannot be directly measured, but we can check that an
-			}
+			// zero-value SpanContext is returned from SpanContextFromContext.
+			assert.Equal(t, empty, trace.SpanContextFromContext(ctx))
 		})
 	}
 }
 
-func TestInjectTraceContextToHTTPReq(t *testing.T) {
+func TestInjectValidTraceContext(t *testing.T) {
-	mockTracer := oteltest.DefaultTracer()
+	stateStr := "key1=value1,key2=value2"
-	prop := propagation.TraceContext{}
+	state, err := trace.ParseTraceState(stateStr)
-	tests := []struct {
+	require.NoError(t, err)
-		name       string
+
-		sc         trace.SpanContext
+	tests := []testcase{
-		wantHeader string
-	}{
 		{
-			name: "valid spancontext, sampled",
+			name: "not sampled",
+			header: http.Header{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
+			},
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID: traceID,
+				SpanID:  spanID,
+				Remote:  true,
+			}),
+		},
+		{
+			name: "sampled",
+			header: http.Header{
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"},
+			},
 			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceFlags: trace.FlagsSampled,
+				Remote:     true,
 			}),
-			wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000002-01",
 		},
 		{
-			name: "valid spancontext, not sampled",
+			name: "unsupported trace flag bits dropped",
-			sc: trace.NewSpanContext(trace.SpanContextConfig{
+			header: http.Header{
-				TraceID: traceID,
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"},
-				SpanID:  spanID,
+			},
-			}),
-			wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000003-00",
-		},
-		{
-			name: "valid spancontext, with unsupported bit set in traceflags",
 			sc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceFlags: 0xff,
+				Remote:     true,
 			}),
-			wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000004-01",
 		},
 		{
-			name:       "invalid spancontext",
+			name: "with tracestate",
-			sc:         trace.SpanContext{},
+			header: http.Header{
-			wantHeader: "",
+				traceparent: []string{"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"},
-		},
+				tracestate:  []string{stateStr},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			req, _ := http.NewRequest("GET", "http://example.com", nil)
-			ctx := context.Background()
-			if tt.sc.IsValid() {
-				ctx = trace.ContextWithRemoteSpanContext(ctx, tt.sc)
-				ctx, _ = mockTracer.Start(ctx, "inject")
-			}
-			prop.Inject(ctx, propagation.HeaderCarrier(req.Header))
-
-			gotHeader := req.Header.Get("traceparent")
-			if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" {
-				t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff)
-			}
-		})
-	}
-}
-
-func TestTraceContextPropagator_GetAllKeys(t *testing.T) {
-	var propagator propagation.TraceContext
-	want := []string{"traceparent", "tracestate"}
-	got := propagator.Fields()
-	if diff := cmp.Diff(got, want); diff != "" {
-		t.Errorf("GetAllKeys: -got +want %s", diff)
-	}
-}
-
-func TestTraceStatePropagation(t *testing.T) {
-	prop := propagation.TraceContext{}
-	stateHeader := "tracestate"
-	parentHeader := "traceparent"
-	state, err := oteltest.TraceStateFromKeyValues(attribute.String("key1", "value1"), attribute.String("key2", "value2"))
-	if err != nil {
-		t.Fatalf("Unable to construct expected TraceState: %s", err.Error())
-	}
-
-	tests := []struct {
-		name    string
-		headers map[string]string
-		valid   bool
-		wantSc  trace.SpanContext
-	}{
-		{
-			name: "valid parent and state",
-			headers: map[string]string{
-				parentHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00",
-				stateHeader:  "key1=value1,key2=value2",
 			},
-			valid: true,
+			sc: trace.NewSpanContext(trace.SpanContextConfig{
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
 				TraceID:    traceID,
 				SpanID:     spanID,
 				TraceState: state,
 				Remote:     true,
 			}),
 		},
-		{
-			name: "valid parent, invalid state",
-			headers: map[string]string{
-				parentHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00",
-				stateHeader:  "key1=value1,invalid$@#=invalid",
-			},
-			valid: false,
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
-				TraceID: traceID,
-				SpanID:  spanID,
-				Remote:  true,
-			}),
-		},
-		{
-			name: "valid parent, malformed state",
-			headers: map[string]string{
-				parentHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00",
-				stateHeader:  "key1=value1,invalid",
-			},
-			valid: false,
-			wantSc: trace.NewSpanContext(trace.SpanContextConfig{
-				TraceID: traceID,
-				SpanID:  spanID,
-				Remote:  true,
-			}),
-		},
 	}
 
-	for _, tt := range tests {
+	for _, tc := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
-			inReq, _ := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			ctx := context.Background()
-			for hk, hv := range tt.headers {
+			ctx = trace.ContextWithRemoteSpanContext(ctx, tc.sc)
-				inReq.Header.Add(hk, hv)
-			}
 
-			ctx := prop.Extract(context.Background(), propagation.HeaderCarrier(inReq.Header))
+			h := http.Header{}
-			if diff := cmp.Diff(
+			prop.Inject(ctx, propagation.HeaderCarrier(h))
-				trace.SpanContextFromContext(ctx),
+			assert.Equal(t, tc.header, h)
-				tt.wantSc,
-				cmp.AllowUnexported(attribute.Value{}),
-				cmp.AllowUnexported(trace.TraceState{}),
-			); diff != "" {
-				t.Errorf("Extracted tracestate: -got +want %s", diff)
-			}
-
-			if tt.valid {
-				mockTracer := oteltest.DefaultTracer()
-				ctx, _ = mockTracer.Start(ctx, "inject")
-				outReq, _ := http.NewRequest(http.MethodGet, "http://www.example.com", nil)
-				prop.Inject(ctx, propagation.HeaderCarrier(outReq.Header))
-
-				if diff := cmp.Diff(outReq.Header.Get(stateHeader), tt.headers[stateHeader]); diff != "" {
-					t.Errorf("Propagated tracestate: -got +want %s", diff)
-				}
-			}
 		})
 	}
 }
+
+func TestInvalidSpanContextDropped(t *testing.T) {
+	invalidSC := trace.SpanContext{}
+	require.False(t, invalidSC.IsValid())
+	ctx := trace.ContextWithRemoteSpanContext(context.Background(), invalidSC)
+
+	header := http.Header{}
+	propagation.TraceContext{}.Inject(ctx, propagation.HeaderCarrier(header))
+	assert.Equal(t, "", header.Get("traceparent"), "injected invalid SpanContext")
+}
+
+func TestTraceContextFields(t *testing.T) {
+	expected := []string{"traceparent", "tracestate"}
+	assert.Equal(t, expected, propagation.TraceContext{}.Fields())
+}