From 1f43b93ee854c847ce26d08f8433a0b0e3016c4f Mon Sep 17 00:00:00 2001 From: Ian Milligan Date: Thu, 16 Apr 2020 14:37:30 -0700 Subject: [PATCH] Fix a race condition in the distributed tracing unit tests (#420) (#463) Also fix the following bugs which were uncovered as a result: 1. The sampled flag would not be correctly set when creating SpanContext 2. Tracestate parsing would fail and be omitted from SpanContext Signed-off-by: Ian Milligan --- .../distributed_tracing_extension.go | 10 +++++---- .../distributed_tracing_extension_test.go | 21 +++++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/pkg/cloudevents/extensions/distributed_tracing_extension.go b/pkg/cloudevents/extensions/distributed_tracing_extension.go index 8b9d9efa4..2015c5449 100644 --- a/pkg/cloudevents/extensions/distributed_tracing_extension.go +++ b/pkg/cloudevents/extensions/distributed_tracing_extension.go @@ -94,17 +94,19 @@ func (d DistributedTracingExtension) ToSpanContext() (trace.SpanContext, error) SpanID: tp.SpanID, } if tp.Flags.Recorded { - sc.TraceOptions &= 1 + sc.TraceOptions |= 1 } if ts, err := tracestate.ParseString(d.TraceState); err == nil { entries := make([]octs.Entry, 0, len(ts)) for _, member := range ts { var key string - if member.Vendor != "" { - key = member.Tenant + "@" + member.Vendor + if member.Tenant != "" { + // Due to github.com/lightstep/tracecontext.go/issues/6, + // the meaning of Vendor and Tenant are swapped here. + key = member.Vendor + "@" + member.Tenant } else { - key = member.Tenant + key = member.Vendor } entries = append(entries, octs.Entry{Key: key, Value: member.Value}) } diff --git a/pkg/cloudevents/extensions/distributed_tracing_extension_test.go b/pkg/cloudevents/extensions/distributed_tracing_extension_test.go index 852c2e286..ed3fedd3e 100644 --- a/pkg/cloudevents/extensions/distributed_tracing_extension_test.go +++ b/pkg/cloudevents/extensions/distributed_tracing_extension_test.go @@ -252,11 +252,11 @@ func decodeSID(s string) (sid [8]byte, err error) { func TestConvertSpanContext(t *testing.T) { tid, err := decodeTID("4bf92f3577b34da6a3ce929d0e0e4736") if err != nil { - t.Fatalf("failed to decode traceID: %w", err) + t.Fatalf("failed to decode traceID: %v", err) } sid, err := decodeSID("00f067aa0ba902b7") if err != nil { - t.Fatalf("failed to decode spanID: %w", err) + t.Fatalf("failed to decode spanID: %v", err) } ts, err := tracestate.New(nil, tracestate.Entry{ @@ -264,12 +264,12 @@ func TestConvertSpanContext(t *testing.T) { Value: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", }, tracestate.Entry{ - Key: "congo", + Key: "tenant@congo", Value: "lZWRzIHRoNhcm5hbCBwbGVhc3VyZS4", }, ) if err != nil { - t.Fatalf("failed to make tracestate: %w", err) + t.Fatalf("failed to make tracestate: %v", err) } tests := []struct { name string @@ -285,7 +285,7 @@ func TestConvertSpanContext(t *testing.T) { }, ext: extensions.DistributedTracingExtension{ TraceParent: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - TraceState: "rojo=00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01,congo=lZWRzIHRoNhcm5hbCBwbGVhc3VyZS4", + TraceState: "rojo=00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01,tenant@congo=lZWRzIHRoNhcm5hbCBwbGVhc3VyZS4", }, }, { name: "without tracestate", @@ -310,6 +310,7 @@ func TestConvertSpanContext(t *testing.T) { }} for _, tt := range tests { + tt := tt t.Run("FromSpanContext: "+tt.name, func(t *testing.T) { t.Parallel() got := extensions.FromSpanContext(tt.sc) @@ -323,7 +324,15 @@ func TestConvertSpanContext(t *testing.T) { if err != nil { t.Error(err) } - if diff := cmp.Diff(tt.sc, got); diff != "" { + if diff := cmp.Diff( + tt.sc, got, + cmp.Transformer( + "entries", + func(ts tracestate.Tracestate) []tracestate.Entry { + return ts.Entries() + }, + ), + ); diff != "" { t.Errorf("\nunexpected (-want, +got) = %v", diff) } })