Skip to content

Commit

Permalink
Fix a race condition in the distributed tracing unit tests (#420) (#463)
Browse files Browse the repository at this point in the history
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 <ianmllgn@gmail.com>
  • Loading branch information
ian-mi authored Apr 16, 2020
1 parent a29c9e9 commit 1f43b93
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
10 changes: 6 additions & 4 deletions pkg/cloudevents/extensions/distributed_tracing_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Expand Down
21 changes: 15 additions & 6 deletions pkg/cloudevents/extensions/distributed_tracing_extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,24 +252,24 @@ 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{
Key: "rojo",
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
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -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)
}
})
Expand Down

0 comments on commit 1f43b93

Please sign in to comment.