From 7fdb6e938df6e3dddf12981e526f839f8d39cdf9 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:37:18 -0500 Subject: [PATCH] [v2][adjuster] Add warning to span links adjuster (#6381) ## Which problem is this PR solving? - Towards #6344 ## Description of the changes - Add a warning to the `SpanLinks` adjuster when there is a bad span link to match the behaviour of the v1 adjuster (https://github.com/jaegertracing/jaeger/blob/main/model/adjuster/bad_span_references.go#L40) ## How was this change tested? - Updated unit tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/spanlinks.go | 8 +++++ .../app/querysvc/adjuster/spanlinks_test.go | 33 ++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/spanlinks.go b/cmd/query/app/querysvc/adjuster/spanlinks.go index d58ad63ab39..b90f41900ac 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks.go @@ -5,6 +5,12 @@ package adjuster import ( "go.opentelemetry.io/collector/pdata/ptrace" + + "github.com/jaegertracing/jaeger/internal/jptrace" +) + +const ( + invalidSpanLinkWarning = "Invalid span link removed" ) // SpanLinks creates an adjuster that removes span links with empty trace IDs. @@ -40,6 +46,8 @@ func (la LinksAdjuster) adjust(span ptrace.Span) { if la.valid(link) { newLink := validLinks.AppendEmpty() link.CopyTo(newLink) + } else { + jptrace.AddWarning(span, invalidSpanLinkWarning) } } validLinks.CopyTo(span.Links()) diff --git a/cmd/query/app/querysvc/adjuster/spanlinks_test.go b/cmd/query/app/querysvc/adjuster/spanlinks_test.go index af687e50bc4..7dfaf0ac6a6 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks_test.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" + + "github.com/jaegertracing/jaeger/internal/jptrace" ) func TestLinksAdjuster(t *testing.T) { @@ -21,19 +23,32 @@ func TestLinksAdjuster(t *testing.T) { scopeSpans.Spans().AppendEmpty() // span with empty traceID link - spanA := scopeSpans.Spans().AppendEmpty() - spanA.Links().AppendEmpty().SetTraceID(pcommon.NewTraceIDEmpty()) + spanB := scopeSpans.Spans().AppendEmpty() + spanB.Links().AppendEmpty().SetTraceID(pcommon.NewTraceIDEmpty()) // span with 2 non-empty traceID links and 1 empty (or zero) traceID link - spanB := scopeSpans.Spans().AppendEmpty() - spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})) - spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0})) - spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})) + spanC := scopeSpans.Spans().AppendEmpty() + spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})) + spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0})) + spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})) err := SpanLinks().Adjust(traces) spans := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans() require.NoError(t, err) - assert.Equal(t, 0, spans.At(0).Links().Len()) - assert.Equal(t, 0, spans.At(1).Links().Len()) - assert.Equal(t, 2, spans.At(2).Links().Len()) + + gotSpansA := spans.At(0) + assert.Equal(t, 0, gotSpansA.Links().Len()) + assert.Empty(t, jptrace.GetWarnings(gotSpansA)) + + gotSpansB := spans.At(1) + assert.Equal(t, 0, gotSpansB.Links().Len()) + spanBWarnings := jptrace.GetWarnings(gotSpansB) + assert.Len(t, spanBWarnings, 1) + assert.Equal(t, "Invalid span link removed", spanBWarnings[0]) + + gotSpansC := spans.At(2) + assert.Equal(t, 2, gotSpansC.Links().Len()) + spanCWarnings := jptrace.GetWarnings(gotSpansC) + assert.Len(t, spanCWarnings, 1) + assert.Equal(t, "Invalid span link removed", spanCWarnings[0]) }