[v2][adjuster] Remove error return from adjuster interface (#6383)

## Which problem is this PR solving?
- Towards #6344

## Description of the changes
- This PR removes the `error` return value from the v2 adjuster
interface as none of the adjusters return any errors and errors instead
recorded on the spans as warnings.

## How was this change tested?
- CI

## 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 <[email protected]>

cmd/query/app/querysvc/adjuster/adjuster.go

@@ -4,8 +4,6 @@
 package adjuster
 
 import (
-	"errors"
-
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
 
@@ -14,15 +12,15 @@ import (
 // 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
+	Adjust(ptrace.Traces)
 }
 
 // Func is a type alias that wraps a function and makes an Adjuster from it.
-type Func func(traces ptrace.Traces) error
+type Func func(traces ptrace.Traces)
 
 // Adjust implements Adjuster interface for the Func alias.
-func (f Func) Adjust(traces ptrace.Traces) error {
-	return f(traces)
+func (f Func) Adjust(traces ptrace.Traces) {
+	f(traces)
 }
 
 // Sequence creates an adjuster that combines a series of adjusters
@@ -33,27 +31,12 @@ func Sequence(adjusters ...Adjuster) Adjuster {
 	return sequence{adjusters: adjusters}
 }
 
-// FailFastSequence is similar to Sequence() but returns immediately
-// if any adjuster returns an error.
-func FailFastSequence(adjusters ...Adjuster) Adjuster {
-	return sequence{adjusters: adjusters, failFast: true}
-}
-
 type sequence struct {
 	adjusters []Adjuster
-	failFast  bool
 }
 
-func (c sequence) Adjust(traces ptrace.Traces) error {
-	var errs []error
+func (c sequence) Adjust(traces ptrace.Traces) {
 	for _, adjuster := range c.adjusters {
-		err := adjuster.Adjust(traces)
-		if err != nil {
-			if c.failFast {
-				return err
-			}
-			errs = append(errs, err)
-		}
+		adjuster.Adjust(traces)
 	}
-	return errors.Join(errs...)
 }

cmd/query/app/querysvc/adjuster/adjuster_test.go

@@ -4,66 +4,33 @@
 package adjuster_test
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/adjuster"
 )
 
 type mockAdjuster struct{}
 
-func (mockAdjuster) Adjust(traces ptrace.Traces) error {
+func (mockAdjuster) Adjust(traces ptrace.Traces) {
 	span := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
 	spanId := span.SpanID()
 	spanId[7]++
 	span.SetSpanID(spanId)
-	return nil
-}
-
-type mockAdjusterError struct{}
-
-func (mockAdjusterError) Adjust(ptrace.Traces) error {
-	return assert.AnError
 }
 
 func TestSequences(t *testing.T) {
-	tests := []struct {
-		name       string
-		adjuster   adjuster.Adjuster
-		err        string
-		lastSpanID pcommon.SpanID
-	}{
-		{
-			name:       "normal sequence",
-			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(mockAdjuster{}, mockAdjusterError{}, mockAdjuster{}, mockAdjusterError{}),
-			err:        assert.AnError.Error(),
-			lastSpanID: [8]byte{0, 0, 0, 0, 0, 0, 0, 1},
-		},
-	}
+	trace := ptrace.NewTraces()
+	span := trace.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty()
+	span.SetSpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 0})
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			trace := ptrace.NewTraces()
-			span := trace.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty()
-			span.SetSpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 0})
+	a := adjuster.Sequence(mockAdjuster{}, mockAdjuster{})
 
-			err := test.adjuster.Adjust(trace)
-			require.EqualError(t, err, test.err)
+	a.Adjust(trace)
 
-			adjTraceSpan := trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
-			assert.Equal(t, span, adjTraceSpan)
-			assert.EqualValues(t, test.lastSpanID, span.SpanID())
-		})
-	}
+	adjTraceSpan := trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
+	assert.Equal(t, span, adjTraceSpan)
+	assert.EqualValues(t, [8]byte{0, 0, 0, 0, 0, 0, 0, 2}, span.SpanID())
 }

cmd/query/app/querysvc/adjuster/ipattribute.go

@@ -28,7 +28,7 @@ func IPAttribute() IPAttributeAdjuster {
 
 type IPAttributeAdjuster struct{}
 
-func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) error {
+func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) {
 	resourceSpans := traces.ResourceSpans()
 	for i := 0; i < resourceSpans.Len(); i++ {
 		rs := resourceSpans.At(i)
@@ -43,7 +43,6 @@ func (ia IPAttributeAdjuster) Adjust(traces ptrace.Traces) error {
 			}
 		}
 	}
-	return nil
 }
 
 func (IPAttributeAdjuster) adjustAttributes(attributes pcommon.Map) {

cmd/query/app/querysvc/adjuster/ipattribute_test.go

@@ -58,8 +58,7 @@ func TestIPAttributeAdjuster(t *testing.T) {
 		}
 	}
 
-	err := IPAttribute().Adjust(traces)
-	require.NoError(t, err)
+	IPAttribute().Adjust(traces)
 
 	resourceSpan := traces.ResourceSpans().At(0)
 	assert.Equal(t, 3, resourceSpan.Resource().Attributes().Len())

cmd/query/app/querysvc/adjuster/resourceattributes.go

@@ -31,7 +31,7 @@ func ResourceAttributes() ResourceAttributesAdjuster {
 
 type ResourceAttributesAdjuster struct{}
 
-func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) error {
+func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) {
 	resourceSpans := traces.ResourceSpans()
 	for i := 0; i < resourceSpans.Len(); i++ {
 		rs := resourceSpans.At(i)
@@ -46,7 +46,6 @@ func (o ResourceAttributesAdjuster) Adjust(traces ptrace.Traces) error {
 			}
 		}
 	}
-	return nil
 }
 
 func (ResourceAttributesAdjuster) moveAttributes(span ptrace.Span, resource pcommon.Resource) {

cmd/query/app/querysvc/adjuster/resourceattributes_test.go

@@ -26,7 +26,7 @@ func TestResourceAttributesAdjuster_SpanWithLibraryAttributes(t *testing.T) {
 	span.Attributes().PutStr("another_key", "another_value")
 
 	adjuster := ResourceAttributes()
-	require.NoError(t, adjuster.Adjust(traces))
+	adjuster.Adjust(traces)
 
 	resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes()
 	require.Equal(t, 2, resultSpanAttributes.Len())
@@ -68,7 +68,7 @@ func TestResourceAttributesAdjuster_SpanWithoutLibraryAttributes(t *testing.T) {
 	span.Attributes().PutStr("random_key", "random_value")
 
 	adjuster := ResourceAttributes()
-	require.NoError(t, adjuster.Adjust(traces))
+	adjuster.Adjust(traces)
 
 	resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes()
 	require.Equal(t, 1, resultSpanAttributes.Len())
@@ -86,7 +86,7 @@ func TestResourceAttributesAdjuster_SpanWithConflictingLibraryAttributes(t *test
 	span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Java")
 
 	adjuster := ResourceAttributes()
-	require.NoError(t, adjuster.Adjust(traces))
+	adjuster.Adjust(traces)
 
 	resultSpan := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
 	resultSpanAttributes := resultSpan.Attributes()
@@ -120,7 +120,7 @@ func TestResourceAttributesAdjuster_SpanWithNonConflictingLibraryAttributes(t *t
 	span.Attributes().PutStr(string(otelsemconv.TelemetrySDKLanguageKey), "Go")
 
 	adjuster := ResourceAttributes()
-	require.NoError(t, adjuster.Adjust(traces))
+	adjuster.Adjust(traces)
 
 	resultSpanAttributes := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes()
 	require.Equal(t, 1, resultSpanAttributes.Len())

cmd/query/app/querysvc/adjuster/spaniduniquifier.go

@@ -21,15 +21,15 @@ var errTooManySpans = errors.New("cannot assign unique span ID, too many spans i
 // Zipkin-style clients that reuse the same span ID for both client and server
 // side of an RPC call. Jaeger UI expects all spans to have unique IDs.
 //
-// This adjuster never returns any errors. Instead it records any issues
-// it encounters in Span.Warnings.
+// Any issues encountered during adjustment are recorded as warnings in the
+// span.
 func SpanIDUniquifier() Adjuster {
-	return Func(func(traces ptrace.Traces) error {
+	return Func(func(traces ptrace.Traces) {
 		adjuster := spanIDDeduper{
 			spansByID: make(map[pcommon.SpanID][]ptrace.Span),
 			maxUsedID: pcommon.NewSpanIDEmpty(),
 		}
-		return adjuster.adjust(traces)
+		adjuster.adjust(traces)
 	})
 }
 
@@ -38,10 +38,9 @@ type spanIDDeduper struct {
 	maxUsedID pcommon.SpanID
 }
 
-func (d *spanIDDeduper) adjust(traces ptrace.Traces) error {
+func (d *spanIDDeduper) adjust(traces ptrace.Traces) {
 	d.groupSpansByID(traces)
 	d.uniquifyServerSpanIDs(traces)
-	return nil
 }
 
 // groupSpansByID groups spans with the same ID returning a map id -> []Span

cmd/query/app/querysvc/adjuster/spaniduniquifier_test.go

@@ -47,7 +47,7 @@ func makeTraces() ptrace.Traces {
 func TestSpanIDUniquifierTriggered(t *testing.T) {
 	traces := makeTraces()
 	deduper := SpanIDUniquifier()
-	require.NoError(t, deduper.Adjust(traces))
+	deduper.Adjust(traces)
 
 	spans := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans()
 
@@ -74,7 +74,7 @@ func TestSpanIDUniquifierNotTriggered(t *testing.T) {
 	newSpans.CopyTo(spans)
 
 	deduper := SpanIDUniquifier()
-	require.NoError(t, deduper.Adjust(traces))
+	deduper.Adjust(traces)
 
 	gotSpans := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans()
 
@@ -96,7 +96,7 @@ func TestSpanIDUniquifierError(t *testing.T) {
 		// instead of 0 start at the last possible value to cause an error
 		maxUsedID: maxID,
 	}
-	require.NoError(t, deduper.adjust(traces))
+	deduper.adjust(traces)
 
 	span := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(1)
 	warnings := jptrace.GetWarnings(span)

cmd/query/app/querysvc/adjuster/spanlinks.go

@@ -22,7 +22,7 @@ func SpanLinks() LinksAdjuster {
 
 type LinksAdjuster struct{}
 
-func (la LinksAdjuster) Adjust(traces ptrace.Traces) error {
+func (la LinksAdjuster) Adjust(traces ptrace.Traces) {
 	resourceSpans := traces.ResourceSpans()
 	for i := 0; i < resourceSpans.Len(); i++ {
 		rs := resourceSpans.At(i)
@@ -36,7 +36,6 @@ func (la LinksAdjuster) Adjust(traces ptrace.Traces) error {
 			}
 		}
 	}
-	return nil
 }
 
 // adjust removes invalid links from a span.

cmd/query/app/querysvc/adjuster/spanlinks_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 
@@ -32,9 +31,8 @@ func TestLinksAdjuster(t *testing.T) {
 	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)
+	SpanLinks().Adjust(traces)
 	spans := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans()
-	require.NoError(t, err)
 
 	gotSpansA := spans.At(0)
 	assert.Equal(t, 0, gotSpansA.Links().Len())