address grpc-trace-bin propagator test comments

grpc · Nov 21, 2024 · a3c8693 · a3c8693
1 parent d708a8b
commit a3c8693
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 41 deletions.
diff --git a/stats/opentelemetry/grpc_trace_bin_propagator.go b/stats/opentelemetry/grpc_trace_bin_propagator.go
@@ -38,7 +38,7 @@ type GRPCTraceBinPropagator struct{}
 // Inject sets OpenTelemetry span context from the Context into the carrier as
 // a `grpc-trace-bin` header if span context is valid.
 //
-// If span context is not valid, it ruturns without setting `grpc-trace-bin`
+// If span context is not valid, it returns without setting `grpc-trace-bin`
 // header.
 func (GRPCTraceBinPropagator) Inject(ctx context.Context, carrier otelpropagation.TextMapCarrier) {
 	sc := oteltrace.SpanFromContext(ctx)

diff --git a/stats/opentelemetry/grpc_trace_bin_propagator_test.go b/stats/opentelemetry/grpc_trace_bin_propagator_test.go
@@ -45,25 +45,25 @@ var validSpanContext = oteltrace.SpanContext{}.WithTraceID(
 // header is not set in the carrier's context metadata.
 func (s) TestInject(t *testing.T) {
 	tests := []struct {
-		name     string
-		injectSC oteltrace.SpanContext
-		wantSC   oteltrace.SpanContext
-		md       metadata.MD
-		wantKeys []string
+		name       string
+		injectSC   oteltrace.SpanContext
+		incomingMd metadata.MD
+		outgoingMd metadata.MD
+		wantKeys   []string
 	}{
 		{
-			name:     "valid OpenTelemetry span context",
-			injectSC: validSpanContext,
-			wantSC:   validSpanContext,
-			md:       metadata.MD{"incoming-key": []string{"incoming-value"}, "outgoing-key": []string{"outgoing-value"}},
-			wantKeys: []string{grpcTraceBinHeaderKey, "outgoing-key"},
+			name:       "valid OpenTelemetry span context",
+			injectSC:   validSpanContext,
+			incomingMd: metadata.MD{"incoming-key": []string{"incoming-value"}},
+			outgoingMd: metadata.MD{"outgoing-key": []string{"outgoing-value"}},
+			wantKeys:   []string{grpcTraceBinHeaderKey, "outgoing-key"},
 		},
 		{
-			name:     "invalid OpenTelemetry span context",
-			injectSC: oteltrace.SpanContext{},
-			wantSC:   oteltrace.SpanContext{},
-			md:       metadata.MD{"incoming-key": []string{"incoming-value"}, "outgoing-key": []string{"outgoing-value"}},
-			wantKeys: []string{"outgoing-key"},
+			name:       "invalid OpenTelemetry span context",
+			injectSC:   oteltrace.SpanContext{},
+			incomingMd: metadata.MD{"incoming-key": []string{"incoming-value"}},
+			outgoingMd: metadata.MD{"outgoing-key": []string{"outgoing-value"}},
+			wantKeys:   []string{"outgoing-key"},
 		},
 	}
 
@@ -73,32 +73,32 @@ func (s) TestInject(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 			ctx = oteltrace.ContextWithSpanContext(ctx, test.injectSC)
-			ctx = metadata.NewIncomingContext(ctx, metadata.MD{"incoming-key": test.md["incoming-key"]})
-			ctx = metadata.NewOutgoingContext(ctx, metadata.MD{"outgoing-key": test.md["outgoing-key"]})
+			ctx = metadata.NewIncomingContext(ctx, test.incomingMd)
+			ctx = metadata.NewOutgoingContext(ctx, test.outgoingMd)
 
 			c := itracing.NewOutgoingCarrier(ctx)
 			p.Inject(ctx, c)
 
 			if gotKeys := c.Keys(); !cmp.Equal(test.wantKeys, gotKeys, cmpopts.SortSlices(func(a, b string) bool { return a < b })) {
-				t.Fatalf("c.Keys() = keys %v, want %v", gotKeys, test.wantKeys)
+				t.Errorf("c.Keys() = keys %v, want %v", gotKeys, test.wantKeys)
 			}
 			md, _ := metadata.FromOutgoingContext(c.Context())
 			gotH := md.Get(grpcTraceBinHeaderKey)
-			if !test.wantSC.IsValid() {
+			if !test.injectSC.IsValid() {
 				if len(gotH) > 0 {
-					t.Fatalf("got non-empty value from Carrier's context metadata grpc-trace-bin header, want empty")
+					t.Fatalf("got %v value from Carrier's context metadata grpc-trace-bin header, want empty", gotH)
 				}
 				return
 			}
 			if gotH[len(gotH)-1] == "" {
-				t.Fatalf("got empty value from Carrier's context metadata grpc-trace-bin header, want valid span context: %v", test.wantSC)
+				t.Fatalf("got empty value from Carrier's context metadata grpc-trace-bin header, want valid span context: %v", test.injectSC)
 			}
 			gotSC, ok := fromBinary([]byte(gotH[len(gotH)-1]))
 			if !ok {
-				t.Fatalf("got invalid span context from Carrier's context metadata grpc-trace-bin header, want valid span context: %v", test.wantSC)
+				t.Fatalf("got invalid span context %v from Carrier's context metadata grpc-trace-bin header, want valid span context: %v", gotSC, test.injectSC)
 			}
-			if test.wantSC.TraceID() != gotSC.TraceID() && test.wantSC.SpanID() != gotSC.SpanID() && test.wantSC.TraceFlags() != gotSC.TraceFlags() {
-				t.Fatalf("got span context = %v, want span contexts %v", gotSC, test.wantSC)
+			if test.injectSC.TraceID() != gotSC.TraceID() && test.injectSC.SpanID() != gotSC.SpanID() && test.injectSC.TraceFlags() != gotSC.TraceFlags() {
+				t.Fatalf("got span context = %v, want span contexts %v", gotSC, test.injectSC)
 			}
 		})
 	}
@@ -114,22 +114,25 @@ func (s) TestInject(t *testing.T) {
 // context is not extracted.
 func (s) TestExtract(t *testing.T) {
 	tests := []struct {
-		name     string
-		wantSC   oteltrace.SpanContext // expected span context from carrier
-		md       metadata.MD
-		wantKeys []string
+		name       string
+		wantSC     oteltrace.SpanContext // expected span context from carrier
+		incomingMd metadata.MD
+		outgoingMd metadata.MD
+		wantKeys   []string
 	}{
 		{
-			name:     "valid OpenTelemetry span context",
-			wantSC:   validSpanContext.WithRemote(true),
-			md:       metadata.MD{"incoming-key": []string{"incoming-value"}, "outgoing-key": []string{"outgoing-value"}},
-			wantKeys: []string{grpcTraceBinHeaderKey, "incoming-key"},
+			name:       "valid OpenTelemetry span context",
+			wantSC:     validSpanContext.WithRemote(true),
+			incomingMd: metadata.MD{grpcTraceBinHeaderKey: []string{string(toBinary(validSpanContext.WithRemote(true)))}, "incoming-key": []string{"incoming-value"}},
+			outgoingMd: metadata.MD{"outgoing-key": []string{"outgoing-value"}},
+			wantKeys:   []string{grpcTraceBinHeaderKey, "incoming-key"},
 		},
 		{
-			name:     "invalid OpenTelemetry span context",
-			wantSC:   oteltrace.SpanContext{},
-			md:       metadata.MD{"incoming-key": []string{"incoming-value"}, "outgoing-key": []string{"outgoing-value"}},
-			wantKeys: []string{grpcTraceBinHeaderKey, "incoming-key"},
+			name:       "invalid OpenTelemetry span context",
+			wantSC:     oteltrace.SpanContext{},
+			incomingMd: metadata.MD{grpcTraceBinHeaderKey: []string{string(toBinary(oteltrace.SpanContext{}))}, "incoming-key": []string{"incoming-value"}},
+			outgoingMd: metadata.MD{"outgoing-key": []string{"outgoing-value"}},
+			wantKeys:   []string{grpcTraceBinHeaderKey, "incoming-key"},
 		},
 	}
 
@@ -138,13 +141,13 @@ func (s) TestExtract(t *testing.T) {
 			p := GRPCTraceBinPropagator{}
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
-			ctx = metadata.NewIncomingContext(ctx, metadata.MD{grpcTraceBinHeaderKey: []string{string(toBinary(test.wantSC))}, "incoming-key": test.md["incoming-key"]})
-			ctx = metadata.NewOutgoingContext(ctx, metadata.MD{"outgoing-key": test.md["outgoing-key"]})
+			ctx = metadata.NewIncomingContext(ctx, test.incomingMd)
+			ctx = metadata.NewOutgoingContext(ctx, test.outgoingMd)
 
 			c := itracing.NewIncomingCarrier(ctx)
 
 			if gotKeys := c.Keys(); !cmp.Equal(test.wantKeys, gotKeys, cmpopts.SortSlices(func(a, b string) bool { return a < b })) {
-				t.Fatalf("c.Keys() = keys %v, want %v", gotKeys, test.wantKeys)
+				t.Errorf("c.Keys() = keys %v, want %v", gotKeys, test.wantKeys)
 			}
 			tCtx := p.Extract(ctx, c)
 			got := oteltrace.SpanContextFromContext(tCtx)

diff --git a/stats/opentelemetry/internal/tracing/carrier.go b/stats/opentelemetry/internal/tracing/carrier.go
@@ -99,7 +99,7 @@ func NewOutgoingCarrier(ctx context.Context) *OutgoingCarrier {
 // Get just logs an error and returns an empty string. It implements the
 // `TextMapCarrier` interface but should not be used with `OutgoingCarrier`.
 func (c *OutgoingCarrier) Get(string) string {
-	logger.Error("Get() should not be used with `IncomingCarrier`")
+	logger.Error("Get() should not be used with `OutgoingCarrier`")
 	return ""
 }