From 1181d83ad380fa612775e6bab67c0c5e7195ad69 Mon Sep 17 00:00:00 2001 From: linfn Date: Fri, 16 Jun 2023 02:44:06 +0800 Subject: [PATCH] [receiver/skywalking]: fix parentSpanId lost when crossing segments (#21799) In Skywalking, parentSpanId == -1 indicates that the span is [the first span within the current segment](https://github.com/apache/skywalking-data-collect-protocol/blob/0da9c8b3e111fb51c9f8854cae16d4519462ecfe/language-agent/Tracing.proto#L119), rather than within the entire trace. Leaving parentSpanId blank can cause the call relationship to be lost when crossing segments. --- ...ix-skywalking-receiver-parent-span-id.yaml | 21 +++++++++++ .../trace/skywalkingproto_to_traces.go | 6 +++- .../trace/skywalkingproto_to_traces_test.go | 35 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 .chloggen/fix-skywalking-receiver-parent-span-id.yaml diff --git a/.chloggen/fix-skywalking-receiver-parent-span-id.yaml b/.chloggen/fix-skywalking-receiver-parent-span-id.yaml new file mode 100644 index 000000000000..eeea68b1129a --- /dev/null +++ b/.chloggen/fix-skywalking-receiver-parent-span-id.yaml @@ -0,0 +1,21 @@ +# Use this changelog template to create an entry for release notes. +# If your change doesn't affect end users, such as a test fix or a tooling change, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: receiver/skywalking + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix the issue where `ParentSpanId` is lost when crossing segments. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [21799] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + diff --git a/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces.go b/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces.go index 1810ffdb3989..9cef435a476b 100644 --- a/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces.go +++ b/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces.go @@ -106,9 +106,13 @@ func swSpanToSpan(traceID string, segmentID string, span *agentV3.SpanObject, de // so use segmentId to convert to an unique otel-span dest.SetSpanID(segmentIDToSpanID(segmentID, uint32(span.GetSpanId()))) - // parent spanid = -1, means(root span) no parent span in skywalking,so just make otlp's parent span id empty. + // parent spanid = -1, means(root span) no parent span in current skywalking segment, so it is necessary to search for the parent segment. if span.ParentSpanId != -1 { dest.SetParentSpanID(segmentIDToSpanID(segmentID, uint32(span.GetParentSpanId()))) + } else if len(span.Refs) == 1 { + // TODO: SegmentReference references usually have only one element, but in batch consumer case, such as in MQ or async batch process, it could be multiple. + // We only handle one element for now. + dest.SetParentSpanID(segmentIDToSpanID(span.Refs[0].GetParentTraceSegmentId(), uint32(span.Refs[0].GetParentSpanId()))) } dest.SetName(span.OperationName) diff --git a/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces_test.go b/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces_test.go index 6c1965940291..bdce5e321164 100644 --- a/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces_test.go +++ b/receiver/skywalkingreceiver/internal/trace/skywalkingproto_to_traces_test.go @@ -301,6 +301,41 @@ func Test_segmentIdToSpanId_Unique(t *testing.T) { assert.NotEqual(t, results[0], results[1]) } +func Test_swSpanToSpan_ParentSpanId(t *testing.T) { + type args struct { + span *agentV3.SpanObject + } + tests := []struct { + name string + args args + want pcommon.SpanID + }{ + { + name: "mock-sw-span-with-parent-segment", + args: args{span: &agentV3.SpanObject{ + ParentSpanId: -1, + Refs: []*agentV3.SegmentReference{{ + ParentTraceSegmentId: "4f2f27748b8e44ecaf18fe0347194e86.33.16560607369950066", + ParentSpanId: 123, + }}}}, + want: [8]byte{233, 196, 85, 168, 37, 66, 48, 106}, + }, + { + name: "mock-sw-span-without-parent-segment", + args: args{span: &agentV3.SpanObject{Refs: []*agentV3.SegmentReference{{ + ParentSpanId: -1, + }}}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dest := ptrace.NewSpan() + swSpanToSpan("de5980b8-fce3-4a37-aab9-b4ac3af7eedd", "", tt.args.span, dest) + assert.Equal(t, tt.want, dest.ParentSpanID()) + }) + } +} + func generateTracesOneEmptyResourceSpans() ptrace.Span { td := ptrace.NewTraces() resourceSpan := td.ResourceSpans().AppendEmpty()