From 9c4f8405c4b2ebe179a1553204ad75585d59f35a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 3 Apr 2020 13:49:18 -0700 Subject: [PATCH 1/2] Update OTLP SpanData transform The ParentSpanId needs to be empty for root spans according to the OTLP [docs](https://github.com/open-telemetry/opentelemetry-proto/blob/6c2a86ed2f74ca46f979f22c77f0aad449fb9629/gen/go/trace/v1/trace.pb.go#L284-L286). This updates the SpanData transform function to not add the ParentSpanID if it is not a valid span ID (which includes if it is the nil span ID used for an unset ID). Additionally, this adds a test to prevent regression. --- exporters/otlp/internal/transform/span.go | 10 +++- .../otlp/internal/transform/span_test.go | 58 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index 38a48bf1d9c..ca67527cbe3 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -65,10 +65,10 @@ func span(sd *export.SpanData) *tracepb.Span { if sd == nil { return nil } - return &tracepb.Span{ + + s := &tracepb.Span{ TraceId: sd.SpanContext.TraceID[:], SpanId: sd.SpanContext.SpanID[:], - ParentSpanId: sd.ParentSpanID[:], Status: status(sd.StatusCode, sd.StatusMessage), StartTimeUnixNano: uint64(sd.StartTime.UnixNano()), EndTimeUnixNano: uint64(sd.EndTime.UnixNano()), @@ -82,6 +82,12 @@ func span(sd *export.SpanData) *tracepb.Span { DroppedEventsCount: uint32(sd.DroppedMessageEventCount), DroppedLinksCount: uint32(sd.DroppedLinkCount), } + + if sd.ParentSpanID.IsValid() { + s.ParentSpanId = sd.ParentSpanID[:] + } + + return s } // status transform a span code and message into an OTLP span status. diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index eec4b3c2f27..8622094550b 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -360,3 +360,61 @@ func TestSpanData(t *testing.T) { t.Fatalf("transformed span differs %v\n", diff) } } + +func TestRootSpanData(t *testing.T) { + // Root span should have an empty ParentSpanId field. + + // March 31, 2020 5:01:26 1234nanos (UTC) + startTime := time.Unix(1585674086, 1234) + endTime := startTime.Add(10 * time.Second) + + sds := []*export.SpanData{ + { + SpanContext: core.SpanContext{ + TraceID: core.TraceID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanID: core.SpanID{0, 0, 0, 0, 0, 0, 0, 1}, + }, + Name: "invalid nil parent span ID", + StartTime: startTime, + EndTime: endTime, + }, + { + SpanContext: core.SpanContext{ + TraceID: core.TraceID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanID: core.SpanID{0, 0, 0, 0, 0, 0, 0, 1}, + }, + Name: "empty parent span ID", + StartTime: startTime, + EndTime: endTime, + }, + } + want := []*tracepb.InstrumentationLibrarySpans{ + { + Spans: []*tracepb.Span{ + { + TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, + ParentSpanId: []byte{}, // Empty means root span. + Name: "invalid nil parent span ID", + StartTimeUnixNano: uint64(1585674086000001234), + EndTimeUnixNano: uint64(1585674096000001234), + Status: &tracepb.Status{}, + }, + { + TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, + ParentSpanId: []byte{}, // Empty means root span. + Name: "empty parent span ID", + StartTimeUnixNano: uint64(1585674086000001234), + EndTimeUnixNano: uint64(1585674096000001234), + Status: &tracepb.Status{}, + }, + }, + }, + } + + rss := SpanData(sds)[0] + if diff := cmp.Diff(want, rss.GetInstrumentationLibrarySpans(), cmp.Comparer(proto.Equal)); diff != "" { + t.Fatalf("transformed complete SpanData incorrect: %v\n", diff) + } +} From 96c48d8e191a7a41870c3504110cbce20ecbc70a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 6 Apr 2020 09:22:19 -0700 Subject: [PATCH 2/2] Simplify test to just check parent span ID transform --- .../otlp/internal/transform/span_test.go | 59 ++----------------- 1 file changed, 5 insertions(+), 54 deletions(-) diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index 8622094550b..9525e3e2569 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -361,60 +361,11 @@ func TestSpanData(t *testing.T) { } } +// Empty parent span ID should be treated as root span. func TestRootSpanData(t *testing.T) { - // Root span should have an empty ParentSpanId field. + rs := SpanData([]*export.SpanData{{}})[0] + got := rs.GetInstrumentationLibrarySpans()[0].GetSpans()[0].GetParentSpanId() - // March 31, 2020 5:01:26 1234nanos (UTC) - startTime := time.Unix(1585674086, 1234) - endTime := startTime.Add(10 * time.Second) - - sds := []*export.SpanData{ - { - SpanContext: core.SpanContext{ - TraceID: core.TraceID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, - SpanID: core.SpanID{0, 0, 0, 0, 0, 0, 0, 1}, - }, - Name: "invalid nil parent span ID", - StartTime: startTime, - EndTime: endTime, - }, - { - SpanContext: core.SpanContext{ - TraceID: core.TraceID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, - SpanID: core.SpanID{0, 0, 0, 0, 0, 0, 0, 1}, - }, - Name: "empty parent span ID", - StartTime: startTime, - EndTime: endTime, - }, - } - want := []*tracepb.InstrumentationLibrarySpans{ - { - Spans: []*tracepb.Span{ - { - TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, - SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, - ParentSpanId: []byte{}, // Empty means root span. - Name: "invalid nil parent span ID", - StartTimeUnixNano: uint64(1585674086000001234), - EndTimeUnixNano: uint64(1585674096000001234), - Status: &tracepb.Status{}, - }, - { - TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, - SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, - ParentSpanId: []byte{}, // Empty means root span. - Name: "empty parent span ID", - StartTimeUnixNano: uint64(1585674086000001234), - EndTimeUnixNano: uint64(1585674096000001234), - Status: &tracepb.Status{}, - }, - }, - }, - } - - rss := SpanData(sds)[0] - if diff := cmp.Diff(want, rss.GetInstrumentationLibrarySpans(), cmp.Comparer(proto.Equal)); diff != "" { - t.Fatalf("transformed complete SpanData incorrect: %v\n", diff) - } + // Empty means root span. + assert.Nil(t, got, "incorrect transform of root parent span ID") }