From 4b96f77df838f96a9121e2360021d610f4b49f0e Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:25:41 -0500 Subject: [PATCH 01/13] Fix IPAttribute Adjuster Signed-off-by: Mahad Zaryab --- .../app/querysvc/adjuster/ipattribute.go | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/ipattribute.go b/cmd/query/app/querysvc/adjuster/ipattribute.go index 0ddb9b45447..7a82d8fc5e7 100644 --- a/cmd/query/app/querysvc/adjuster/ipattribute.go +++ b/cmd/query/app/querysvc/adjuster/ipattribute.go @@ -20,30 +20,31 @@ var ipAttributesToCorrect = map[string]struct{}{ // IPAttribute returns an adjuster that replaces numeric "ip" attributes, // which usually contain IPv4 packed into uint32, with their string // representation (e.g. "8.8.8.8""). -func IPAttribute() Adjuster { - return Func(func(traces ptrace.Traces) (ptrace.Traces, error) { - adjuster := ipAttributeAdjuster{} - resourceSpans := traces.ResourceSpans() - for i := 0; i < resourceSpans.Len(); i++ { - rs := resourceSpans.At(i) - adjuster.adjust(rs.Resource().Attributes()) - scopeSpans := rs.ScopeSpans() - for j := 0; j < scopeSpans.Len(); j++ { - ss := scopeSpans.At(j) - spans := ss.Spans() - for k := 0; k < spans.Len(); k++ { - span := spans.At(k) - adjuster.adjust(span.Attributes()) - } +func IPAttribute() IPAttributeAdjuster { + return IPAttributeAdjuster{} +} + +type IPAttributeAdjuster struct{} + +func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { + resourceSpans := traces.ResourceSpans() + for i := 0; i < resourceSpans.Len(); i++ { + rs := resourceSpans.At(i) + ia.adjustAttributes(rs.Resource().Attributes()) + scopeSpans := rs.ScopeSpans() + for j := 0; j < scopeSpans.Len(); j++ { + ss := scopeSpans.At(j) + spans := ss.Spans() + for k := 0; k < spans.Len(); k++ { + span := spans.At(k) + ia.adjustAttributes(span.Attributes()) } } - return traces, nil - }) + } + return traces, nil } -type ipAttributeAdjuster struct{} - -func (ipAttributeAdjuster) adjust(attributes pcommon.Map) { +func (IPAttributeAdjuster) adjustAttributes(attributes pcommon.Map) { adjusted := make(map[string]string) attributes.Range(func(k string, v pcommon.Value) bool { if _, ok := ipAttributesToCorrect[k]; !ok { From 5aaba1df3aed9015b2a0dae134c9d589a5ca3327 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:32:38 -0500 Subject: [PATCH 02/13] Fix SpanLinks Adjuster Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/spanlinks.go | 41 ++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/spanlinks.go b/cmd/query/app/querysvc/adjuster/spanlinks.go index bb93dc1f9c3..a72fe7df391 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks.go @@ -9,34 +9,35 @@ import ( // SpanLinks creates an adjuster that removes span links with empty trace IDs. func SpanLinks() Adjuster { - return Func(func(traces ptrace.Traces) (ptrace.Traces, error) { - adjuster := linksAdjuster{} - resourceSpans := traces.ResourceSpans() - for i := 0; i < resourceSpans.Len(); i++ { - rs := resourceSpans.At(i) - scopeSpans := rs.ScopeSpans() - for j := 0; j < scopeSpans.Len(); j++ { - ss := scopeSpans.At(j) - spans := ss.Spans() - for k := 0; k < spans.Len(); k++ { - span := spans.At(k) - adjuster.adjust(span) - } + return LinksAdjuster{} +} + +type LinksAdjuster struct{} + +func (la LinksAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { + resourceSpans := traces.ResourceSpans() + for i := 0; i < resourceSpans.Len(); i++ { + rs := resourceSpans.At(i) + scopeSpans := rs.ScopeSpans() + for j := 0; j < scopeSpans.Len(); j++ { + ss := scopeSpans.At(j) + spans := ss.Spans() + for k := 0; k < spans.Len(); k++ { + span := spans.At(k) + la.adjust(span) } } - return traces, nil - }) + } + return traces, nil } -type linksAdjuster struct{} - // adjust removes invalid links from a span. -func (l linksAdjuster) adjust(span ptrace.Span) { +func (la LinksAdjuster) adjust(span ptrace.Span) { links := span.Links() validLinks := ptrace.NewSpanLinkSlice() for i := 0; i < links.Len(); i++ { link := links.At(i) - if l.valid(link) { + if la.valid(link) { newLink := validLinks.AppendEmpty() link.CopyTo(newLink) } @@ -45,6 +46,6 @@ func (l linksAdjuster) adjust(span ptrace.Span) { } // valid checks if a span link's TraceID is not empty. -func (linksAdjuster) valid(link ptrace.SpanLink) bool { +func (LinksAdjuster) valid(link ptrace.SpanLink) bool { return !link.TraceID().IsEmpty() } From 3e7e300790ef6575d8646d8f76afe3d7bb6e05fb Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:33:00 -0500 Subject: [PATCH 03/13] Remove Func Alias Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/adjuster.go | 12 +++--- .../app/querysvc/adjuster/adjuster_test.go | 37 ++++++++++--------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/adjuster.go b/cmd/query/app/querysvc/adjuster/adjuster.go index e6c4f4039fb..2dfe6def46f 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster.go +++ b/cmd/query/app/querysvc/adjuster/adjuster.go @@ -17,13 +17,13 @@ type Adjuster interface { Adjust(ptrace.Traces) (ptrace.Traces, error) } -// Func is a type alias that wraps a function and makes an Adjuster from it. -type Func func(traces ptrace.Traces) (ptrace.Traces, error) +// // Func is a type alias that wraps a function and makes an Adjuster from it. +// type Func func(traces ptrace.Traces) (ptrace.Traces, error) -// Adjust implements Adjuster interface for the Func alias. -func (f Func) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { - return f(traces) -} +// // Adjust implements Adjuster interface for the Func alias. +// func (f Func) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +// return f(traces) +// } // Sequence creates an adjuster that combines a series of adjusters // applied in order. Errors from each step are accumulated and returned diff --git a/cmd/query/app/querysvc/adjuster/adjuster_test.go b/cmd/query/app/querysvc/adjuster/adjuster_test.go index 96cd4336988..51334965a95 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster_test.go +++ b/cmd/query/app/querysvc/adjuster/adjuster_test.go @@ -4,7 +4,6 @@ package adjuster_test import ( - "errors" "fmt" "testing" @@ -16,21 +15,23 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/adjuster" ) -func TestSequences(t *testing.T) { - // mock adjuster that increments last byte of span ID - adj := adjuster.Func(func(trace ptrace.Traces) (ptrace.Traces, error) { - span := trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) - spanId := span.SpanID() - spanId[7]++ - span.SetSpanID(spanId) - return trace, nil - }) +type mockAdjuster struct{} + +func (mockAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { + span := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) + spanId := span.SpanID() + spanId[7]++ + span.SetSpanID(spanId) + return traces, nil +} - adjErr := errors.New("mock adjuster error") - failingAdj := adjuster.Func(func(trace ptrace.Traces) (ptrace.Traces, error) { - return trace, adjErr - }) +type mockAdjusterError struct{} +func (mockAdjusterError) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { + return traces, assert.AnError +} + +func TestSequences(t *testing.T) { tests := []struct { name string adjuster adjuster.Adjuster @@ -39,14 +40,14 @@ func TestSequences(t *testing.T) { }{ { name: "normal sequence", - adjuster: adjuster.Sequence(adj, failingAdj, adj, failingAdj), - err: fmt.Sprintf("%s\n%s", adjErr, adjErr), + adjuster: adjuster.Sequence(mockAdjuster{}, mockAdjusterError{}, mockAdjuster{}, mockAdjusterError{}), + err: fmt.Sprintf("%s\n%s", assert.AnError, assert.AnError), lastSpanID: [8]byte{0, 0, 0, 0, 0, 0, 0, 2}, }, { name: "fail fast sequence", - adjuster: adjuster.FailFastSequence(adj, failingAdj, adj, failingAdj), - err: adjErr.Error(), + adjuster: adjuster.FailFastSequence(mockAdjuster{}, mockAdjusterError{}, mockAdjuster{}, mockAdjusterError{}), + err: assert.AnError.Error(), lastSpanID: [8]byte{0, 0, 0, 0, 0, 0, 0, 1}, }, } From 43f50c4468627365f9f268308071639a1d7d9968 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:42:55 -0500 Subject: [PATCH 04/13] Change Interface To Not Return Traces Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/adjuster.go | 19 ++++++++-------- .../app/querysvc/adjuster/adjuster_test.go | 14 ++++++------ .../app/querysvc/adjuster/ipattribute.go | 4 ++-- .../app/querysvc/adjuster/ipattribute_test.go | 4 ++-- .../querysvc/adjuster/resourceattributes.go | 4 ++-- .../adjuster/resourceattributes_test.go | 22 +++++++++---------- cmd/query/app/querysvc/adjuster/spanlinks.go | 4 ++-- .../app/querysvc/adjuster/spanlinks_test.go | 8 +++---- 8 files changed, 39 insertions(+), 40 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/adjuster.go b/cmd/query/app/querysvc/adjuster/adjuster.go index 2dfe6def46f..fe384696b07 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster.go +++ b/cmd/query/app/querysvc/adjuster/adjuster.go @@ -9,19 +9,19 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace" ) -// Adjuster defines an interface for modifying a trace object. -// It returns the adjusted trace object, which is also updated in place. +// Adjuster defines an interface for modifying a trace object in place. // If the adjuster encounters an issue that prevents it from applying -// modifications, it should return the original trace object along with an error. +// modifications, it should return an error. The original trace object +// is modified directly and not returned. type Adjuster interface { - Adjust(ptrace.Traces) (ptrace.Traces, error) + Adjust(ptrace.Traces) error } // // Func is a type alias that wraps a function and makes an Adjuster from it. // type Func func(traces ptrace.Traces) (ptrace.Traces, error) // // Adjust implements Adjuster interface for the Func alias. -// func (f Func) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +// func (f Func) Adjust(traces ptrace.Traces) error { // return f(traces) // } @@ -44,17 +44,16 @@ type sequence struct { failFast bool } -func (c sequence) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +func (c sequence) Adjust(traces ptrace.Traces) error { var errs []error for _, adjuster := range c.adjusters { - var err error - traces, err = adjuster.Adjust(traces) + err := adjuster.Adjust(traces) if err != nil { if c.failFast { - return traces, err + return err } errs = append(errs, err) } } - return traces, errors.Join(errs...) + return errors.Join(errs...) } diff --git a/cmd/query/app/querysvc/adjuster/adjuster_test.go b/cmd/query/app/querysvc/adjuster/adjuster_test.go index 51334965a95..fbc5804ed9e 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster_test.go +++ b/cmd/query/app/querysvc/adjuster/adjuster_test.go @@ -17,18 +17,18 @@ import ( type mockAdjuster struct{} -func (mockAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +func (mockAdjuster) Adjust(traces ptrace.Traces) error { span := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) spanId := span.SpanID() spanId[7]++ span.SetSpanID(spanId) - return traces, nil + return nil } type mockAdjusterError struct{} -func (mockAdjusterError) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { - return traces, assert.AnError +func (mockAdjusterError) Adjust(ptrace.Traces) error { + return assert.AnError } func TestSequences(t *testing.T) { @@ -58,12 +58,12 @@ func TestSequences(t *testing.T) { span := trace.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty() span.SetSpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 0}) - adjTrace, err := test.adjuster.Adjust(trace) - adjTraceSpan := adjTrace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) + err := test.adjuster.Adjust(trace) + require.EqualError(t, err, test.err) + adjTraceSpan := trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) assert.Equal(t, span, adjTraceSpan) assert.EqualValues(t, test.lastSpanID, span.SpanID()) - require.EqualError(t, err, test.err) }) } } diff --git a/cmd/query/app/querysvc/adjuster/ipattribute.go b/cmd/query/app/querysvc/adjuster/ipattribute.go index 7a82d8fc5e7..4dcd70bd888 100644 --- a/cmd/query/app/querysvc/adjuster/ipattribute.go +++ b/cmd/query/app/querysvc/adjuster/ipattribute.go @@ -26,7 +26,7 @@ func IPAttribute() IPAttributeAdjuster { type IPAttributeAdjuster struct{} -func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) error { resourceSpans := traces.ResourceSpans() for i := 0; i < resourceSpans.Len(); i++ { rs := resourceSpans.At(i) @@ -41,7 +41,7 @@ func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error } } } - return traces, nil + return nil } func (IPAttributeAdjuster) adjustAttributes(attributes pcommon.Map) { diff --git a/cmd/query/app/querysvc/adjuster/ipattribute_test.go b/cmd/query/app/querysvc/adjuster/ipattribute_test.go index cfd9ddeea26..e55a9dd8bdc 100644 --- a/cmd/query/app/querysvc/adjuster/ipattribute_test.go +++ b/cmd/query/app/querysvc/adjuster/ipattribute_test.go @@ -58,10 +58,10 @@ func TestIPAttributeAdjuster(t *testing.T) { } } - trace, err := IPAttribute().Adjust(traces) + err := IPAttribute().Adjust(traces) require.NoError(t, err) - resourceSpan := trace.ResourceSpans().At(0) + resourceSpan := traces.ResourceSpans().At(0) assert.Equal(t, 3, resourceSpan.Resource().Attributes().Len()) assertAttribute(resourceSpan.Resource().Attributes(), "a", 42) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes.go b/cmd/query/app/querysvc/adjuster/resourceattributes.go index 0c3cd6d512f..b14a0fb1800 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes.go @@ -28,7 +28,7 @@ func ResourceAttributes() ResourceAttributesAdjuster { type ResourceAttributesAdjuster struct{} -func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) error { resourceSpans := traces.ResourceSpans() for i := 0; i < resourceSpans.Len(); i++ { rs := resourceSpans.At(i) @@ -43,7 +43,7 @@ func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, } } } - return traces, nil + return nil } func (ResourceAttributesAdjuster) moveAttributes(span ptrace.Span, resource pcommon.Resource) { diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go index 0d445671ad0..bea01c28e54 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go @@ -25,10 +25,10 @@ func TestResourceAttributesAdjuster_SpanWithLibraryAttributes(t *testing.T) { span.Attributes().PutStr("another_key", "another_value") adjuster := ResourceAttributes() - result, err := adjuster.Adjust(traces) + err := adjuster.Adjust(traces) require.NoError(t, err) - resultSpanAttributes := result.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() + resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 2, resultSpanAttributes.Len()) val, ok := resultSpanAttributes.Get("random_key") require.True(t, ok) @@ -38,7 +38,7 @@ func TestResourceAttributesAdjuster_SpanWithLibraryAttributes(t *testing.T) { require.True(t, ok) require.Equal(t, "another_value", val.Str()) - resultResourceAttributes := result.ResourceSpans().At(0).Resource().Attributes() + resultResourceAttributes := traces.ResourceSpans().At(0).Resource().Attributes() val, ok = resultResourceAttributes.Get(string(otelsemconv.TelemetrySDKLanguageKey)) require.True(t, ok) @@ -68,10 +68,10 @@ func TestResourceAttributesAdjuster_SpanWithoutLibraryAttributes(t *testing.T) { span.Attributes().PutStr("random_key", "random_value") adjuster := ResourceAttributes() - result, err := adjuster.Adjust(traces) + err := adjuster.Adjust(traces) require.NoError(t, err) - resultSpanAttributes := result.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() + resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 1, resultSpanAttributes.Len()) val, ok := resultSpanAttributes.Get("random_key") require.True(t, ok) @@ -87,10 +87,10 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Java") adjuster := ResourceAttributes() - result, err := adjuster.Adjust(traces) + err := adjuster.Adjust(traces) require.NoError(t, err) - resultSpanAttributes := result.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() + resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 3, resultSpanAttributes.Len()) val, ok := resultSpanAttributes.Get("random_key") require.True(t, ok) @@ -107,7 +107,7 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test require.Equal(t, 1, warnings.Len()) require.Equal(t, "conflicting values between Span and Resource for attribute telemetry.sdk.language", warnings.At(0).Str()) - resultResourceAttributes := result.ResourceSpans().At(0).Resource().Attributes() + resultResourceAttributes := traces.ResourceSpans().At(0).Resource().Attributes() val, ok = resultResourceAttributes.Get(string(otelsemconv.TelemetrySDKLanguageKey)) require.True(t, ok) require.Equal(t, "Go", val.Str()) @@ -122,16 +122,16 @@ func TestResourceAttributesAdjuster_SpanWithNonConflictingLibraryAttributes(t *t span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Go") adjuster := ResourceAttributes() - result, err := adjuster.Adjust(traces) + err := adjuster.Adjust(traces) require.NoError(t, err) - resultSpanAttributes := result.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() + resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 1, resultSpanAttributes.Len()) val, ok := resultSpanAttributes.Get("random_key") require.True(t, ok) require.Equal(t, "random_value", val.Str()) - resultResourceAttributes := result.ResourceSpans().At(0).Resource().Attributes() + resultResourceAttributes := traces.ResourceSpans().At(0).Resource().Attributes() val, ok = resultResourceAttributes.Get(string(otelsemconv.TelemetrySDKLanguageKey)) require.True(t, ok) require.Equal(t, "Go", val.Str()) diff --git a/cmd/query/app/querysvc/adjuster/spanlinks.go b/cmd/query/app/querysvc/adjuster/spanlinks.go index a72fe7df391..84dbf781a37 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks.go @@ -14,7 +14,7 @@ func SpanLinks() Adjuster { type LinksAdjuster struct{} -func (la LinksAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { +func (la LinksAdjuster) Adjust(traces ptrace.Traces) error { resourceSpans := traces.ResourceSpans() for i := 0; i < resourceSpans.Len(); i++ { rs := resourceSpans.At(i) @@ -28,7 +28,7 @@ func (la LinksAdjuster) Adjust(traces ptrace.Traces) (ptrace.Traces, error) { } } } - return traces, nil + return nil } // adjust removes invalid links from a span. diff --git a/cmd/query/app/querysvc/adjuster/spanlinks_test.go b/cmd/query/app/querysvc/adjuster/spanlinks_test.go index 9d64b5c8531..af687e50bc4 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks_test.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks_test.go @@ -13,8 +13,8 @@ import ( ) func TestLinksAdjuster(t *testing.T) { - trace := ptrace.NewTraces() - resourceSpans := trace.ResourceSpans().AppendEmpty() + traces := ptrace.NewTraces() + resourceSpans := traces.ResourceSpans().AppendEmpty() scopeSpans := resourceSpans.ScopeSpans().AppendEmpty() // span with no links @@ -30,8 +30,8 @@ func TestLinksAdjuster(t *testing.T) { 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})) - trace, err := SpanLinks().Adjust(trace) - spans := trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans() + 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()) From e47e1314dbc48cf964af7ef9ad75a0abf0bfe930 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:50:24 -0500 Subject: [PATCH 05/13] Move Warning File To Separate Package Signed-off-by: Mahad Zaryab --- cmd/query/app/internal/jotlp/package_test.go | 14 ++++++++++++++ .../adjuster => internal/jotlp}/warning.go | 4 ++-- .../adjuster => internal/jotlp}/warning_test.go | 4 ++-- .../app/querysvc/adjuster/resourceattributes.go | 3 ++- 4 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 cmd/query/app/internal/jotlp/package_test.go rename cmd/query/app/{querysvc/adjuster => internal/jotlp}/warning.go (88%) rename cmd/query/app/{querysvc/adjuster => internal/jotlp}/warning_test.go (96%) diff --git a/cmd/query/app/internal/jotlp/package_test.go b/cmd/query/app/internal/jotlp/package_test.go new file mode 100644 index 00000000000..e7679c77b1a --- /dev/null +++ b/cmd/query/app/internal/jotlp/package_test.go @@ -0,0 +1,14 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package jotlp + +import ( + "testing" + + "github.com/jaegertracing/jaeger/pkg/testutils" +) + +func TestMain(m *testing.M) { + testutils.VerifyGoLeaks(m) +} diff --git a/cmd/query/app/querysvc/adjuster/warning.go b/cmd/query/app/internal/jotlp/warning.go similarity index 88% rename from cmd/query/app/querysvc/adjuster/warning.go rename to cmd/query/app/internal/jotlp/warning.go index e7f1331bfab..60061011522 100644 --- a/cmd/query/app/querysvc/adjuster/warning.go +++ b/cmd/query/app/internal/jotlp/warning.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package adjuster +package jotlp import ( "go.opentelemetry.io/collector/pdata/pcommon" @@ -12,7 +12,7 @@ const ( adjusterWarningAttribute = "jaeger.adjuster.warning" ) -func addWarning(span ptrace.Span, warning string) { +func AddWarning(span ptrace.Span, warning string) { var warnings pcommon.Slice if currWarnings, ok := span.Attributes().Get(adjusterWarningAttribute); ok { warnings = currWarnings.Slice() diff --git a/cmd/query/app/querysvc/adjuster/warning_test.go b/cmd/query/app/internal/jotlp/warning_test.go similarity index 96% rename from cmd/query/app/querysvc/adjuster/warning_test.go rename to cmd/query/app/internal/jotlp/warning_test.go index 209d794e639..9fb4c6956ba 100644 --- a/cmd/query/app/querysvc/adjuster/warning_test.go +++ b/cmd/query/app/internal/jotlp/warning_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package adjuster +package jotlp import ( "testing" @@ -46,7 +46,7 @@ func TestAddWarning(t *testing.T) { warnings.AppendEmpty().SetStr(warn) } } - addWarning(span, test.newWarn) + AddWarning(span, test.newWarn) warnings, ok := attrs.Get("jaeger.adjuster.warning") assert.True(t, ok) assert.Equal(t, len(test.expected), warnings.Slice().Len()) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes.go b/cmd/query/app/querysvc/adjuster/resourceattributes.go index b14a0fb1800..9a7f371fa77 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes.go @@ -7,6 +7,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" + "github.com/jaegertracing/jaeger/cmd/query/app/internal/jotlp" "github.com/jaegertracing/jaeger/pkg/otelsemconv" ) @@ -57,7 +58,7 @@ func (ResourceAttributesAdjuster) moveAttributes(span ptrace.Span, resource pcom for k, v := range replace { existing, ok := resource.Attributes().Get(k) if ok && existing.AsRaw() != v.AsRaw() { - addWarning(span, "conflicting values between Span and Resource for attribute "+k) + jotlp.AddWarning(span, "conflicting values between Span and Resource for attribute "+k) continue } v.CopyTo(resource.Attributes().PutEmpty(k)) From 304c180870bc71dde8f58d4b85c26501af505043 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:52:24 -0500 Subject: [PATCH 06/13] Remove Commented Code Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/adjuster.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/adjuster.go b/cmd/query/app/querysvc/adjuster/adjuster.go index fe384696b07..240b7cfddb9 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster.go +++ b/cmd/query/app/querysvc/adjuster/adjuster.go @@ -17,14 +17,6 @@ type Adjuster interface { Adjust(ptrace.Traces) error } -// // Func is a type alias that wraps a function and makes an Adjuster from it. -// type Func func(traces ptrace.Traces) (ptrace.Traces, error) - -// // Adjust implements Adjuster interface for the Func alias. -// func (f Func) Adjust(traces ptrace.Traces) error { -// return f(traces) -// } - // Sequence creates an adjuster that combines a series of adjusters // applied in order. Errors from each step are accumulated and returned // in the end as a single wrapper error. Errors do not interrupt the From f1331f22e4c0ac69140ee136e55b91337b27ccb3 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 14 Dec 2024 21:57:02 -0500 Subject: [PATCH 07/13] Return Struct Instead of Interface Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/spanlinks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/query/app/querysvc/adjuster/spanlinks.go b/cmd/query/app/querysvc/adjuster/spanlinks.go index 84dbf781a37..d58ad63ab39 100644 --- a/cmd/query/app/querysvc/adjuster/spanlinks.go +++ b/cmd/query/app/querysvc/adjuster/spanlinks.go @@ -8,7 +8,7 @@ import ( ) // SpanLinks creates an adjuster that removes span links with empty trace IDs. -func SpanLinks() Adjuster { +func SpanLinks() LinksAdjuster { return LinksAdjuster{} } From 88c0e6fcdc881da5095281844682c09369e47f62 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 09:26:20 -0500 Subject: [PATCH 08/13] Move jotlp To Root Package Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/resourceattributes.go | 2 +- {cmd/query/app/internal => internal}/jotlp/package_test.go | 0 {cmd/query/app/internal => internal}/jotlp/warning.go | 0 {cmd/query/app/internal => internal}/jotlp/warning_test.go | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename {cmd/query/app/internal => internal}/jotlp/package_test.go (100%) rename {cmd/query/app/internal => internal}/jotlp/warning.go (100%) rename {cmd/query/app/internal => internal}/jotlp/warning_test.go (100%) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes.go b/cmd/query/app/querysvc/adjuster/resourceattributes.go index 9a7f371fa77..87aa8b4c1a7 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes.go @@ -7,7 +7,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/cmd/query/app/internal/jotlp" + "github.com/jaegertracing/jaeger/internal/jotlp" "github.com/jaegertracing/jaeger/pkg/otelsemconv" ) diff --git a/cmd/query/app/internal/jotlp/package_test.go b/internal/jotlp/package_test.go similarity index 100% rename from cmd/query/app/internal/jotlp/package_test.go rename to internal/jotlp/package_test.go diff --git a/cmd/query/app/internal/jotlp/warning.go b/internal/jotlp/warning.go similarity index 100% rename from cmd/query/app/internal/jotlp/warning.go rename to internal/jotlp/warning.go diff --git a/cmd/query/app/internal/jotlp/warning_test.go b/internal/jotlp/warning_test.go similarity index 100% rename from cmd/query/app/internal/jotlp/warning_test.go rename to internal/jotlp/warning_test.go From 1dea7af0a4d8345472249965966fca234fd40919 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 09:28:41 -0500 Subject: [PATCH 09/13] Change Warning Attribute Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/resourceattributes_test.go | 3 ++- internal/jotlp/warning.go | 6 +++--- internal/jotlp/warning_test.go | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go index bea01c28e54..e8d14268b17 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/ptrace" + "github.com/jaegertracing/jaeger/internal/jotlp" "github.com/jaegertracing/jaeger/pkg/otelsemconv" ) @@ -101,7 +102,7 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test require.True(t, ok) require.Equal(t, "Java", val.Str()) - val, ok = resultSpanAttributes.Get("jaeger.adjuster.warning") + val, ok = resultSpanAttributes.Get(jotlp.WarningsAttribute) require.True(t, ok) warnings := val.Slice() require.Equal(t, 1, warnings.Len()) diff --git a/internal/jotlp/warning.go b/internal/jotlp/warning.go index 60061011522..96e52580194 100644 --- a/internal/jotlp/warning.go +++ b/internal/jotlp/warning.go @@ -9,15 +9,15 @@ import ( ) const ( - adjusterWarningAttribute = "jaeger.adjuster.warning" + WarningsAttribute = "jaeger.internal.warnings" ) func AddWarning(span ptrace.Span, warning string) { var warnings pcommon.Slice - if currWarnings, ok := span.Attributes().Get(adjusterWarningAttribute); ok { + if currWarnings, ok := span.Attributes().Get(WarningsAttribute); ok { warnings = currWarnings.Slice() } else { - warnings = span.Attributes().PutEmptySlice(adjusterWarningAttribute) + warnings = span.Attributes().PutEmptySlice(WarningsAttribute) } warnings.AppendEmpty().SetStr(warning) } diff --git a/internal/jotlp/warning_test.go b/internal/jotlp/warning_test.go index 9fb4c6956ba..f9d4a42e40a 100644 --- a/internal/jotlp/warning_test.go +++ b/internal/jotlp/warning_test.go @@ -41,13 +41,13 @@ func TestAddWarning(t *testing.T) { span := ptrace.NewSpan() attrs := span.Attributes() if test.existing != nil { - warnings := attrs.PutEmptySlice("jaeger.adjuster.warning") + warnings := attrs.PutEmptySlice("jaeger.internal.warnings") for _, warn := range test.existing { warnings.AppendEmpty().SetStr(warn) } } AddWarning(span, test.newWarn) - warnings, ok := attrs.Get("jaeger.adjuster.warning") + warnings, ok := attrs.Get("jaeger.internal.warnings") assert.True(t, ok) assert.Equal(t, len(test.expected), warnings.Slice().Len()) for i, expectedWarn := range test.expected { From 84c533243ae10d8a59de97996a3d9eb2074d6558 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 09:30:40 -0500 Subject: [PATCH 10/13] Fix Documentation Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/adjuster.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/adjuster.go b/cmd/query/app/querysvc/adjuster/adjuster.go index 240b7cfddb9..de9a6b347a8 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster.go +++ b/cmd/query/app/querysvc/adjuster/adjuster.go @@ -9,10 +9,11 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace" ) -// Adjuster defines an interface for modifying a trace object in place. -// If the adjuster encounters an issue that prevents it from applying -// modifications, it should return an error. The original trace object -// is modified directly and not returned. +// Adjuster is an interface for modifying a trace object in place. +// If an issue is encountered that prevents modifications, an error should be returned. +// The original trace object is modified directly and not returned. +// The caller must ensure that all spans in the ptrace.Traces argument +// belong to the same trace and represent the complete trace. type Adjuster interface { Adjust(ptrace.Traces) error } From d916c00f094a2ce0c20a434855be19e8c629f547 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 18:12:23 -0500 Subject: [PATCH 11/13] Address Feedback Signed-off-by: Mahad Zaryab --- cmd/query/app/querysvc/adjuster/adjuster.go | 1 - cmd/query/app/querysvc/adjuster/resourceattributes.go | 4 ++-- cmd/query/app/querysvc/adjuster/resourceattributes_test.go | 4 ++-- internal/{jotlp => jptrace}/package_test.go | 2 +- internal/{jotlp => jptrace}/warning.go | 6 +++++- internal/{jotlp => jptrace}/warning_test.go | 2 +- 6 files changed, 11 insertions(+), 8 deletions(-) rename internal/{jotlp => jptrace}/package_test.go (93%) rename internal/{jotlp => jptrace}/warning.go (68%) rename internal/{jotlp => jptrace}/warning_test.go (98%) diff --git a/cmd/query/app/querysvc/adjuster/adjuster.go b/cmd/query/app/querysvc/adjuster/adjuster.go index de9a6b347a8..33b8de79d91 100644 --- a/cmd/query/app/querysvc/adjuster/adjuster.go +++ b/cmd/query/app/querysvc/adjuster/adjuster.go @@ -11,7 +11,6 @@ import ( // Adjuster is an interface for modifying a trace object in place. // If an issue is encountered that prevents modifications, an error should be returned. -// The original trace object is modified directly and not returned. // The caller must ensure that all spans in the ptrace.Traces argument // belong to the same trace and represent the complete trace. type Adjuster interface { diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes.go b/cmd/query/app/querysvc/adjuster/resourceattributes.go index 87aa8b4c1a7..356cd4f800f 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes.go @@ -7,7 +7,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jotlp" + "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/pkg/otelsemconv" ) @@ -58,7 +58,7 @@ func (ResourceAttributesAdjuster) moveAttributes(span ptrace.Span, resource pcom for k, v := range replace { existing, ok := resource.Attributes().Get(k) if ok && existing.AsRaw() != v.AsRaw() { - jotlp.AddWarning(span, "conflicting values between Span and Resource for attribute "+k) + jptrace.AddWarning(span, "conflicting values between Span and Resource for attribute "+k) continue } v.CopyTo(resource.Attributes().PutEmpty(k)) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go index e8d14268b17..59999fcda85 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jotlp" + "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/pkg/otelsemconv" ) @@ -102,7 +102,7 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test require.True(t, ok) require.Equal(t, "Java", val.Str()) - val, ok = resultSpanAttributes.Get(jotlp.WarningsAttribute) + val, ok = resultSpanAttributes.Get(jptrace.WarningsAttribute) require.True(t, ok) warnings := val.Slice() require.Equal(t, 1, warnings.Len()) diff --git a/internal/jotlp/package_test.go b/internal/jptrace/package_test.go similarity index 93% rename from internal/jotlp/package_test.go rename to internal/jptrace/package_test.go index e7679c77b1a..c86c58d3df0 100644 --- a/internal/jotlp/package_test.go +++ b/internal/jptrace/package_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package jotlp +package jptrace import ( "testing" diff --git a/internal/jotlp/warning.go b/internal/jptrace/warning.go similarity index 68% rename from internal/jotlp/warning.go rename to internal/jptrace/warning.go index 96e52580194..df42ae228e3 100644 --- a/internal/jotlp/warning.go +++ b/internal/jptrace/warning.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package jotlp +package jptrace import ( "go.opentelemetry.io/collector/pdata/pcommon" @@ -9,6 +9,10 @@ import ( ) const ( + // WarningsAttribute is the name of the span attribute where we can + // store various warnings produced from transformations, + // such as inbound sanitizers and outbound adjusters. + // The value type of the attribute is a string slice. WarningsAttribute = "jaeger.internal.warnings" ) diff --git a/internal/jotlp/warning_test.go b/internal/jptrace/warning_test.go similarity index 98% rename from internal/jotlp/warning_test.go rename to internal/jptrace/warning_test.go index f9d4a42e40a..f95e22a4586 100644 --- a/internal/jotlp/warning_test.go +++ b/internal/jptrace/warning_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package jotlp +package jptrace import ( "testing" From 107738a02759d525fc8b081d5271e68e2652e7e7 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 21:26:07 -0500 Subject: [PATCH 12/13] Address Feedback Signed-off-by: Mahad Zaryab --- .../adjuster/resourceattributes_test.go | 22 +++++------ internal/jptrace/warning.go | 18 +++++++-- internal/jptrace/warning_test.go | 37 +++++++++++++++++++ 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go index 59999fcda85..5dc5c438133 100644 --- a/cmd/query/app/querysvc/adjuster/resourceattributes_test.go +++ b/cmd/query/app/querysvc/adjuster/resourceattributes_test.go @@ -26,8 +26,7 @@ func TestResourceAttributesAdjuster_SpanWithLibraryAttributes(t *testing.T) { span.Attributes().PutStr("another_key", "another_value") adjuster := ResourceAttributes() - err := adjuster.Adjust(traces) - require.NoError(t, err) + require.NoError(t, adjuster.Adjust(traces)) resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 2, resultSpanAttributes.Len()) @@ -69,8 +68,7 @@ func TestResourceAttributesAdjuster_SpanWithoutLibraryAttributes(t *testing.T) { span.Attributes().PutStr("random_key", "random_value") adjuster := ResourceAttributes() - err := adjuster.Adjust(traces) - require.NoError(t, err) + require.NoError(t, adjuster.Adjust(traces)) resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 1, resultSpanAttributes.Len()) @@ -88,10 +86,10 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Java") adjuster := ResourceAttributes() - err := adjuster.Adjust(traces) - require.NoError(t, err) + require.NoError(t, adjuster.Adjust(traces)) - resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() + resultSpan := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0) + resultSpanAttributes := resultSpan.Attributes() require.Equal(t, 3, resultSpanAttributes.Len()) val, ok := resultSpanAttributes.Get("random_key") require.True(t, ok) @@ -102,11 +100,10 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test require.True(t, ok) require.Equal(t, "Java", val.Str()) - val, ok = resultSpanAttributes.Get(jptrace.WarningsAttribute) + warnings := jptrace.GetWarnings(resultSpan) require.True(t, ok) - warnings := val.Slice() - require.Equal(t, 1, warnings.Len()) - require.Equal(t, "conflicting values between Span and Resource for attribute telemetry.sdk.language", warnings.At(0).Str()) + require.Len(t, warnings, 1) + require.Equal(t, "conflicting values between Span and Resource for attribute telemetry.sdk.language", warnings[0]) resultResourceAttributes := traces.ResourceSpans().At(0).Resource().Attributes() val, ok = resultResourceAttributes.Get(string(otelsemconv.TelemetrySDKLanguageKey)) @@ -123,8 +120,7 @@ func TestResourceAttributesAdjuster_SpanWithNonConflictingLibraryAttributes(t *t span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Go") adjuster := ResourceAttributes() - err := adjuster.Adjust(traces) - require.NoError(t, err) + require.NoError(t, adjuster.Adjust(traces)) resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes() require.Equal(t, 1, resultSpanAttributes.Len()) diff --git a/internal/jptrace/warning.go b/internal/jptrace/warning.go index df42ae228e3..0d96296c214 100644 --- a/internal/jptrace/warning.go +++ b/internal/jptrace/warning.go @@ -13,15 +13,27 @@ const ( // store various warnings produced from transformations, // such as inbound sanitizers and outbound adjusters. // The value type of the attribute is a string slice. - WarningsAttribute = "jaeger.internal.warnings" + warningsAttribute = "jaeger.internal.warnings" ) func AddWarning(span ptrace.Span, warning string) { var warnings pcommon.Slice - if currWarnings, ok := span.Attributes().Get(WarningsAttribute); ok { + if currWarnings, ok := span.Attributes().Get(warningsAttribute); ok { warnings = currWarnings.Slice() } else { - warnings = span.Attributes().PutEmptySlice(WarningsAttribute) + warnings = span.Attributes().PutEmptySlice(warningsAttribute) } warnings.AppendEmpty().SetStr(warning) } + +func GetWarnings(span ptrace.Span) []string { + if w, ok := span.Attributes().Get(warningsAttribute); ok { + warnings := []string{} + ws := w.Slice() + for i := 0; i < ws.Len(); i++ { + warnings = append(warnings, ws.At(i).Str()) + } + return warnings + } + return nil +} diff --git a/internal/jptrace/warning_test.go b/internal/jptrace/warning_test.go index f95e22a4586..a63588aaf55 100644 --- a/internal/jptrace/warning_test.go +++ b/internal/jptrace/warning_test.go @@ -56,3 +56,40 @@ func TestAddWarning(t *testing.T) { }) } } +func TestGetWarnings(t *testing.T) { + tests := []struct { + name string + existing []string + expected []string + }{ + { + name: "get from nil warnings", + existing: nil, + expected: nil, + }, + { + name: "get from empty warnings", + existing: []string{}, + expected: []string{}, + }, + { + name: "get from existing warnings", + existing: []string{"existing warning 1", "existing warning 2"}, + expected: []string{"existing warning 1", "existing warning 2"}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + span := ptrace.NewSpan() + attrs := span.Attributes() + if test.existing != nil { + warnings := attrs.PutEmptySlice("jaeger.internal.warnings") + for _, warn := range test.existing { + warnings.AppendEmpty().SetStr(warn) + } + } + actual := GetWarnings(span) + assert.Equal(t, test.expected, actual) + }) + } +} From e26acdd5f928ae3dff016b7fdf57efbe5a66a2a0 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 15 Dec 2024 21:36:05 -0500 Subject: [PATCH 13/13] Fix Linting Signed-off-by: Mahad Zaryab --- internal/jptrace/warning_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/jptrace/warning_test.go b/internal/jptrace/warning_test.go index a63588aaf55..b8e3c38032c 100644 --- a/internal/jptrace/warning_test.go +++ b/internal/jptrace/warning_test.go @@ -56,6 +56,7 @@ func TestAddWarning(t *testing.T) { }) } } + func TestGetWarnings(t *testing.T) { tests := []struct { name string