Repository: grpc-ecosystem/go-grpc-middleware
Pull request #338 (2020-09-17 03:50:49)

Fix the special case for jaeger format traceid extraction
PR #262 introduces a regression where special parsing for the jaeger format traceid extraction is overridden with the unparsed value later in the same file.

Where a header matches the traceHeaderName it is preferred over the other matching types using partial string matches.

nvx: I looked at doing that but couldn't spot a way to do it end-to-end, unless you're happy with just a test that calls the function directly?
johanbrandhorst: I think that'd be alright, just something to protect us from regressions and to show a case that was broken.
nvx: Added. Any chance of having a look at PR #327 while you're at it? That one's been bugging me for a while, has a unit test already too.

Patch:
diff --git a/tracing/opentracing/id_extract.go b/tracing/opentracing/id_extract.go
index c92b527..bc7302e 100644
--- a/tracing/opentracing/id_extract.go
+++ b/tracing/opentracing/id_extract.go
@@ -40,20 +40,6 @@ type tagsCarrier struct {
 
 func (t *tagsCarrier) Set(key, val string) {
 	key = strings.ToLower(key)
-	if strings.Contains(key, "traceid") {
-		t.Tags.Set(TagTraceId, val) // this will most likely be base-16 (hex) encoded
-	}
-
-	if strings.Contains(key, "spanid") && !strings.Contains(strings.ToLower(key), "parent") {
-		t.Tags.Set(TagSpanId, val) // this will most likely be base-16 (hex) encoded
-	}
-
-	if strings.Contains(key, "sampled") {
-		switch val {
-		case "true", "false":
-			t.Tags.Set(TagSampled, val)
-		}
-	}
 
 	if key == t.traceHeaderName {
 		parts := strings.Split(val, ":")
@@ -66,6 +52,23 @@ func (t *tagsCarrier) Set(key, val string) {
 			} else {
 				t.Tags.Set(TagSampled, "false")
 			}
+
+			return
+		}
+	}
+
+	if strings.Contains(key, "traceid") {
+		t.Tags.Set(TagTraceId, val) // this will most likely be base-16 (hex) encoded
+	}
+
+	if strings.Contains(key, "spanid") && !strings.Contains(strings.ToLower(key), "parent") {
+		t.Tags.Set(TagSpanId, val) // this will most likely be base-16 (hex) encoded
+	}
+
+	if strings.Contains(key, "sampled") {
+		switch val {
+		case "true", "false":
+			t.Tags.Set(TagSampled, val)
 		}
 	}
 


Test patch:
diff --git a/tracing/opentracing/id_extract_test.go b/tracing/opentracing/id_extract_test.go
new file mode 100644
index 0000000..72a8200
--- /dev/null
+++ b/tracing/opentracing/id_extract_test.go
@@ -0,0 +1,32 @@
+package grpc_opentracing
+
+import (
+	"fmt"
+	grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestTagsCarrier_Set_JaegerTraceFormat(t *testing.T) {
+	var (
+		fakeTraceSampled   = 1
+		fakeInboundTraceId = "deadbeef"
+		fakeInboundSpanId  = "c0decafe"
+		traceHeaderName    = "uber-trace-id"
+	)
+
+	traceHeaderValue := fmt.Sprintf("%s:%s:%s:%d", fakeInboundTraceId, fakeInboundSpanId, fakeInboundSpanId, fakeTraceSampled)
+
+	c := &tagsCarrier{
+		Tags:            grpc_ctxtags.NewTags(),
+		traceHeaderName: traceHeaderName,
+	}
+
+	c.Set(traceHeaderName, traceHeaderValue)
+
+	assert.EqualValues(t, map[string]interface{}{
+		TagTraceId: fakeInboundTraceId,
+		TagSpanId:  fakeInboundSpanId,
+		TagSampled: "true",
+	}, c.Tags.Values())
+}