diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 02ed05c059f..06c97c94dbf 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -254,10 +254,7 @@ func (aH *APIHandler) search(w http.ResponseWriter, r *http.Request) { func (aH *APIHandler) tracesToResponse(traces []*model.Trace, adjust bool, uiErrors []structuredError) *structuredResponse { uiTraces := make([]*ui.Trace, len(traces)) for i, v := range traces { - uiTrace, uiErr := aH.convertModelToUI(v, adjust) - if uiErr != nil { - uiErrors = append(uiErrors, *uiErr) - } + uiTrace := aH.convertModelToUI(v, adjust) uiTraces[i] = uiTrace } @@ -364,24 +361,12 @@ func (aH *APIHandler) metrics(w http.ResponseWriter, r *http.Request, getMetrics aH.writeJSON(w, r, m) } -func (aH *APIHandler) convertModelToUI(trc *model.Trace, adjust bool) (*ui.Trace, *structuredError) { - var errs []error +func (aH *APIHandler) convertModelToUI(trc *model.Trace, adjust bool) *ui.Trace { if adjust { - var err error - trc, err = aH.queryService.Adjust(trc) - if err != nil { - errs = append(errs, err) - } + aH.queryService.Adjust(trc) } uiTrace := uiconv.FromDomain(trc) - var uiError *structuredError - if err := errors.Join(errs...); err != nil { - uiError = &structuredError{ - Msg: err.Error(), - TraceID: uiTrace.TraceID, - } - } - return uiTrace, uiError + return uiTrace } func (*APIHandler) deduplicateDependencies(dependencies []model.DependencyLink) []ui.DependencyLink { diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 479466d0da0..015c8e0fb7a 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -32,7 +32,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/model/adjuster" ui "github.com/jaegertracing/jaeger/model/json" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/tenancy" @@ -59,7 +58,6 @@ func (m *IoReaderMock) Read(b []byte) (int, error) { var ( errStorageMsg = "storage error" errStorage = errors.New(errStorageMsg) - errAdjustment = errors.New("adjustment error") httpClient = &http.Client{ Timeout: 2 * time.Second, @@ -375,25 +373,6 @@ func TestGetTraceNotFound(t *testing.T) { require.EqualError(t, err, parsedError(404, "trace not found")) } -func TestGetTraceAdjustmentFailure(t *testing.T) { - ts := initializeTestServerWithHandler( - t, - querysvc.QueryServiceOptions{ - Adjuster: adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { - return trace, errAdjustment - }), - }, - ) - ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("spanstore.GetTraceParameters")). - Return(mockTrace, nil).Once() - - var response structuredResponse - err := getJSON(ts.server.URL+`/api/traces/123456`, &response) - require.NoError(t, err) - assert.Len(t, response.Errors, 1) - assert.EqualValues(t, errAdjustment.Error(), response.Errors[0].Msg) -} - func TestGetTraceBadTraceID(t *testing.T) { ts := initializeTestServer(t) @@ -564,25 +543,6 @@ func TestSearchByTraceIDFailure(t *testing.T) { require.EqualError(t, err, parsedError(500, whatsamattayou)) } -func TestSearchModelConversionFailure(t *testing.T) { - ts := initializeTestServerWithOptions( - t, - &tenancy.Manager{}, - querysvc.QueryServiceOptions{ - Adjuster: adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { - return trace, errAdjustment - }), - }, - ) - ts.spanReader.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")). - Return([]*model.Trace{mockTrace}, nil).Once() - var response structuredResponse - err := getJSON(ts.server.URL+`/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms`, &response) - require.NoError(t, err) - assert.Len(t, response.Errors, 1) - assert.EqualValues(t, errAdjustment.Error(), response.Errors[0].Msg) -} - func TestSearchDBFailure(t *testing.T) { ts := initializeTestServer(t) ts.spanReader.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")). diff --git a/cmd/query/app/querysvc/query_service.go b/cmd/query/app/querysvc/query_service.go index 724a2536583..3f40b2d44cb 100644 --- a/cmd/query/app/querysvc/query_service.go +++ b/cmd/query/app/querysvc/query_service.go @@ -128,8 +128,8 @@ func (qs QueryService) ArchiveTrace(ctx context.Context, query spanstore.GetTrac } // Adjust applies adjusters to the trace. -func (qs QueryService) Adjust(trace *model.Trace) (*model.Trace, error) { - return qs.options.Adjuster.Adjust(trace) +func (qs QueryService) Adjust(trace *model.Trace) { + qs.options.Adjuster.Adjust(trace) } // GetDependencies implements dependencystore.Reader.GetDependencies diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index 2b1176cf96f..1d9217ade29 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -15,7 +15,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/model/adjuster" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage" @@ -31,8 +30,6 @@ import ( const millisToNanosMultiplier = int64(time.Millisecond / time.Nanosecond) var ( - errAdjustment = errors.New("adjustment error") - defaultDependencyLookbackDuration = time.Hour * 24 mockTraceID = model.NewTraceID(0, 123456) @@ -80,14 +77,6 @@ func withArchiveSpanWriter() testOption { } } -func withAdjuster() testOption { - return func(_ *testQueryService, options *QueryServiceOptions) { - options.Adjuster = adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { - return trace, errAdjustment - }) - } -} - func initializeTestService(optionAppliers ...testOption) *testQueryService { readStorage := &spanstoremocks.Reader{} traceReader := v1adapter.NewTraceReader(readStorage) @@ -307,15 +296,6 @@ func TestArchiveTraceSuccess(t *testing.T) { require.NoError(t, err) } -// Test QueryService.Adjust() -func TestTraceAdjustmentFailure(t *testing.T) { - tqs := initializeTestService(withAdjuster()) - - _, err := tqs.queryService.Adjust(mockTrace) - require.Error(t, err) - assert.EqualValues(t, errAdjustment.Error(), err.Error()) -} - // Test QueryService.GetDependencies() func TestGetDependencies(t *testing.T) { tqs := initializeTestService() diff --git a/model/adjuster/adjuster.go b/model/adjuster/adjuster.go index ac808866326..5f97741398d 100644 --- a/model/adjuster/adjuster.go +++ b/model/adjuster/adjuster.go @@ -5,57 +5,34 @@ package adjuster import ( - "errors" - "github.com/jaegertracing/jaeger/model" ) -// Adjuster applies certain modifications to a Trace object. -// It returns adjusted Trace, which can be the same Trace updated in place. -// If it detects a problem with the trace that prevents it from applying -// adjustments, it must still return the original trace, and the error. +// Adjuster is an interface for modifying a trace object in place. type Adjuster interface { - Adjust(trace *model.Trace) (*model.Trace, error) + Adjust(trace *model.Trace) } // Func wraps a function of appropriate signature and makes an Adjuster from it. -type Func func(trace *model.Trace) (*model.Trace, error) +type Func func(trace *model.Trace) // Adjust implements Adjuster interface. -func (f Func) Adjust(trace *model.Trace) (*model.Trace, error) { - return f(trace) +func (f Func) Adjust(trace *model.Trace) { + f(trace) } // 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 -// sequence of adapters. +// applied in order. 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(trace *model.Trace) (*model.Trace, error) { - var errs []error +func (c sequence) Adjust(trace *model.Trace) { for _, adjuster := range c.adjusters { - var err error - trace, err = adjuster.Adjust(trace) - if err != nil { - if c.failFast { - return trace, err - } - errs = append(errs, err) - } + adjuster.Adjust(trace) } - return trace, errors.Join(errs...) } diff --git a/model/adjuster/adjuster_test.go b/model/adjuster/adjuster_test.go index 0a0960920b5..ddd7b26c744 100644 --- a/model/adjuster/adjuster_test.go +++ b/model/adjuster/adjuster_test.go @@ -5,12 +5,9 @@ package adjuster_test import ( - "errors" - "fmt" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/model/adjuster" @@ -18,41 +15,17 @@ import ( func TestSequences(t *testing.T) { // mock adjuster that increments span ID - adj := adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { + adj := adjuster.Func(func(trace *model.Trace) { trace.Spans[0].SpanID++ - return trace, nil }) - adjErr := errors.New("mock adjuster error") - failingAdj := adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { - return trace, adjErr - }) + seqAdjuster := adjuster.Sequence(adj, adj) + + span := &model.Span{} + trace := model.Trace{Spans: []*model.Span{span}} + + seqAdjuster.Adjust(&trace) - testCases := []struct { - adjuster adjuster.Adjuster - err string - lastSpanID int - }{ - { - adjuster: adjuster.Sequence(adj, failingAdj, adj, failingAdj), - err: fmt.Sprintf("%s\n%s", adjErr, adjErr), - lastSpanID: 2, - }, - { - adjuster: adjuster.FailFastSequence(adj, failingAdj, adj, failingAdj), - err: adjErr.Error(), - lastSpanID: 1, - }, - } - - for _, testCase := range testCases { - span := &model.Span{} - trace := model.Trace{Spans: []*model.Span{span}} - - adjTrace, err := testCase.adjuster.Adjust(&trace) - - assert.Equal(t, span, adjTrace.Spans[0], "same trace & span returned") - assert.EqualValues(t, testCase.lastSpanID, span.SpanID, "expect span ID to be incremented") - require.EqualError(t, err, testCase.err) - } + assert.Equal(t, span, trace.Spans[0], "same trace & span returned") + assert.EqualValues(t, 2, span.SpanID, "expect span ID to be incremented") } diff --git a/model/adjuster/bad_span_references.go b/model/adjuster/bad_span_references.go index 3720be424b3..8f30ec3938b 100644 --- a/model/adjuster/bad_span_references.go +++ b/model/adjuster/bad_span_references.go @@ -12,12 +12,11 @@ import ( // SpanReferences creates an adjuster that removes invalid span references, e.g. with traceID==0 func SpanReferences() Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { adjuster := spanReferenceAdjuster{} for _, span := range trace.Spans { adjuster.adjust(span) } - return trace, nil }) } diff --git a/model/adjuster/bad_span_references_test.go b/model/adjuster/bad_span_references_test.go index 72952c30f40..4c671704e98 100644 --- a/model/adjuster/bad_span_references_test.go +++ b/model/adjuster/bad_span_references_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" ) @@ -29,8 +28,7 @@ func TestSpanReferencesAdjuster(t *testing.T) { }, }, } - trace, err := SpanReferences().Adjust(trace) - require.NoError(t, err) + SpanReferences().Adjust(trace) assert.Empty(t, trace.Spans[0].References) assert.Empty(t, trace.Spans[1].References) assert.Len(t, trace.Spans[2].References, 2) diff --git a/model/adjuster/clockskew.go b/model/adjuster/clockskew.go index 30f922307dc..27cfc0851d5 100644 --- a/model/adjuster/clockskew.go +++ b/model/adjuster/clockskew.go @@ -21,10 +21,9 @@ import ( // The algorithm assumes that all spans have unique IDs, so the trace may need // to go through another adjuster first, such as ZipkinSpanIDUniquifier. // -// This adjuster never returns any errors. Instead it records any issues -// it encounters in Span.Warnings. +// Any issues encountered by the adjuster are recorded in Span.Warnings. func ClockSkew(maxDelta time.Duration) Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { adjuster := &clockSkewAdjuster{ trace: trace, maxDelta: maxDelta, @@ -35,7 +34,6 @@ func ClockSkew(maxDelta time.Duration) Adjuster { skew := clockSkew{hostKey: n.hostKey} adjuster.adjustNode(n, nil, skew) } - return adjuster.trace, nil }) } diff --git a/model/adjuster/clockskew_test.go b/model/adjuster/clockskew_test.go index 5d5023e4d49..1e0d2d8a62a 100644 --- a/model/adjuster/clockskew_test.go +++ b/model/adjuster/clockskew_test.go @@ -187,8 +187,8 @@ func TestClockSkewAdjuster(t *testing.T) { testCase := tt // capture loop var t.Run(testCase.description, func(t *testing.T) { adjuster := ClockSkew(tt.maxAdjust) - trace, err := adjuster.Adjust(makeTrace(testCase.trace)) - require.NoError(t, err) + trace := makeTrace(testCase.trace) + adjuster.Adjust(trace) if testCase.err != "" { var err string for _, span := range trace.Spans { diff --git a/model/adjuster/ip_tag.go b/model/adjuster/ip_tag.go index b3105fe6b5d..2f11e304022 100644 --- a/model/adjuster/ip_tag.go +++ b/model/adjuster/ip_tag.go @@ -49,12 +49,11 @@ func IPTagAdjuster() Adjuster { } } - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { for _, span := range trace.Spans { adjustTags(span.Tags) adjustTags(span.Process.Tags) model.KeyValues(span.Process.Tags).Sort() } - return trace, nil }) } diff --git a/model/adjuster/ip_tag_test.go b/model/adjuster/ip_tag_test.go index 716efb020b5..ae8ec352d38 100644 --- a/model/adjuster/ip_tag_test.go +++ b/model/adjuster/ip_tag_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" ) @@ -36,8 +35,7 @@ func TestIPTagAdjuster(t *testing.T) { }, }, } - trace, err := IPTagAdjuster().Adjust(trace) - require.NoError(t, err) + IPTagAdjuster().Adjust(trace) expectedSpanTags := model.KeyValues{ model.Int64("a", 42), diff --git a/model/adjuster/otel_tag.go b/model/adjuster/otel_tag.go index e579856726d..7811e8ebb19 100644 --- a/model/adjuster/otel_tag.go +++ b/model/adjuster/otel_tag.go @@ -32,11 +32,10 @@ func OTelTagAdjuster() Adjuster { span.Tags = span.Tags[:newI] } - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { for _, span := range trace.Spans { adjustSpanTags(span) model.KeyValues(span.Process.Tags).Sort() } - return trace, nil }) } diff --git a/model/adjuster/otel_tag_test.go b/model/adjuster/otel_tag_test.go index 7e0b802d56c..c87708a43ce 100644 --- a/model/adjuster/otel_tag_test.go +++ b/model/adjuster/otel_tag_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/otelsemconv" @@ -79,8 +78,7 @@ func TestOTelTagAdjuster(t *testing.T) { trace := &model.Trace{ Spans: []*model.Span{testCase.span}, } - trace, err := OTelTagAdjuster().Adjust(trace) - require.NoError(t, err) + OTelTagAdjuster().Adjust(trace) assert.Equal(t, testCase.expected.Tags, trace.Spans[0].Tags) assert.Equal(t, testCase.expected.Process.Tags, trace.Spans[0].Process.Tags) diff --git a/model/adjuster/parent_reference.go b/model/adjuster/parent_reference.go index 00e763d9b2b..253243ad53a 100644 --- a/model/adjuster/parent_reference.go +++ b/model/adjuster/parent_reference.go @@ -11,7 +11,7 @@ import ( // This is necessary to match jaeger-ui expectations: // * https://github.com/jaegertracing/jaeger-ui/issues/966 func ParentReference() Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { for _, span := range trace.Spans { firstChildOfRef := -1 firstOtherRef := -1 @@ -33,7 +33,5 @@ func ParentReference() Adjuster { span.References[swap], span.References[0] = span.References[0], span.References[swap] } } - - return trace, nil }) } diff --git a/model/adjuster/parent_reference_test.go b/model/adjuster/parent_reference_test.go index fc9ba39b50f..58447dc5bf9 100644 --- a/model/adjuster/parent_reference_test.go +++ b/model/adjuster/parent_reference_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" ) @@ -96,8 +95,7 @@ func TestParentReference(t *testing.T) { }, }, } - trace, err := ParentReference().Adjust(trace) - require.NoError(t, err) + ParentReference().Adjust(trace) assert.Equal(t, testCase.expected, trace.Spans[0].References) }) } diff --git a/model/adjuster/sort_tags_and_log_fields.go b/model/adjuster/sort_tags_and_log_fields.go index 0d6028a2d0c..88891e507fb 100644 --- a/model/adjuster/sort_tags_and_log_fields.go +++ b/model/adjuster/sort_tags_and_log_fields.go @@ -20,7 +20,7 @@ import ( // place in the list. This adjuster needs some sort of config describing predefined // field names/types and their relative order. func SortTagsAndLogFields() Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { for _, span := range trace.Spans { model.KeyValues(span.Tags).Sort() if span.Process != nil { @@ -45,6 +45,5 @@ func SortTagsAndLogFields() Adjuster { } } } - return trace, nil }) } diff --git a/model/adjuster/sort_tags_and_log_fields_test.go b/model/adjuster/sort_tags_and_log_fields_test.go index afd1983cac0..e87d3748267 100644 --- a/model/adjuster/sort_tags_and_log_fields_test.go +++ b/model/adjuster/sort_tags_and_log_fields_test.go @@ -80,8 +80,7 @@ func TestSortTagsAndLogFieldsDoesSortFields(t *testing.T) { }, }, } - trace, err := SortTagsAndLogFields().Adjust(trace) - require.NoError(t, err) + SortTagsAndLogFields().Adjust(trace) assert.Equal(t, testCase.expected, model.KeyValues(trace.Spans[0].Logs[0].Fields)) } } @@ -109,11 +108,10 @@ func TestSortTagsAndLogFieldsDoesSortTags(t *testing.T) { }, }, } - sorted, err := SortTagsAndLogFields().Adjust(trace) - require.NoError(t, err) - assert.ElementsMatch(t, tags, sorted.Spans[0].Tags) - adjustedKeys := make([]string, len(sorted.Spans[0].Tags)) - for i, kv := range sorted.Spans[0].Tags { + SortTagsAndLogFields().Adjust(trace) + assert.ElementsMatch(t, tags, trace.Spans[0].Tags) + adjustedKeys := make([]string, len(trace.Spans[0].Tags)) + for i, kv := range trace.Spans[0].Tags { adjustedKeys[i] = kv.Key } assert.IsIncreasing(t, adjustedKeys) @@ -135,11 +133,9 @@ func TestSortTagsAndLogFieldsDoesSortProcessTags(t *testing.T) { }, }, } - sorted, err := SortTagsAndLogFields().Adjust(trace) - require.NoError(t, err) - assert.ElementsMatch(t, trace.Spans[0].Process.Tags, sorted.Spans[0].Process.Tags) - adjustedKeys := make([]string, len(sorted.Spans[0].Process.Tags)) - for i, kv := range sorted.Spans[0].Process.Tags { + SortTagsAndLogFields().Adjust(trace) + adjustedKeys := make([]string, len(trace.Spans[0].Process.Tags)) + for i, kv := range trace.Spans[0].Process.Tags { adjustedKeys[i] = kv.Key } assert.IsIncreasing(t, adjustedKeys) @@ -151,6 +147,10 @@ func TestSortTagsAndLogFieldsHandlesNilProcessTags(t *testing.T) { {}, }, } - _, err := SortTagsAndLogFields().Adjust(trace) - require.NoError(t, err) + SortTagsAndLogFields().Adjust(trace) + require.Equal(t, &model.Trace{ + Spans: []*model.Span{ + {}, + }, + }, trace) } diff --git a/model/adjuster/span_hash_deduper.go b/model/adjuster/span_hash_deduper.go index 5feb79e1769..e0d7a8df6db 100644 --- a/model/adjuster/span_hash_deduper.go +++ b/model/adjuster/span_hash_deduper.go @@ -11,10 +11,9 @@ import ( // This is useful for when spans are duplicated in archival storage, as happens with // ElasticSearch archival. func DedupeBySpanHash() Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { deduper := &spanHashDeduper{trace: trace} deduper.groupSpansByHash() - return deduper.trace, nil }) } diff --git a/model/adjuster/span_hash_deduper_test.go b/model/adjuster/span_hash_deduper_test.go index d8bffd5bf82..3660e9eb253 100644 --- a/model/adjuster/span_hash_deduper_test.go +++ b/model/adjuster/span_hash_deduper_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" ) @@ -74,8 +73,7 @@ func getSpanIDs(spans []*model.Span) []int { func TestDedupeBySpanHashTriggers(t *testing.T) { trc := newDuplicatedSpansTrace() deduper := DedupeBySpanHash() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) assert.Len(t, trc.Spans, 2, "should dedupe spans") @@ -86,8 +84,7 @@ func TestDedupeBySpanHashTriggers(t *testing.T) { func TestDedupeBySpanHashNotTriggered(t *testing.T) { trc := newUniqueSpansTrace() deduper := DedupeBySpanHash() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) assert.Len(t, trc.Spans, 2, "should not dedupe spans") @@ -99,8 +96,7 @@ func TestDedupeBySpanHashNotTriggered(t *testing.T) { func TestDedupeBySpanHashEmpty(t *testing.T) { trc := &model.Trace{} deduper := DedupeBySpanHash() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) assert.Empty(t, trc.Spans, "should be empty") } @@ -117,8 +113,7 @@ func TestDedupeBySpanHashManyManySpans(t *testing.T) { } trc := &model.Trace{Spans: spans} deduper := DedupeBySpanHash() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) assert.Len(t, trc.Spans, distinctSpanIDs, "should dedupe spans") diff --git a/model/adjuster/zipkin_span_id_uniquify.go b/model/adjuster/zipkin_span_id_uniquify.go index 5f8a86a32cc..3de8f20d77f 100644 --- a/model/adjuster/zipkin_span_id_uniquify.go +++ b/model/adjuster/zipkin_span_id_uniquify.go @@ -20,11 +20,10 @@ import ( // This adjuster never returns any errors. Instead it records any issues // it encounters in Span.Warnings. func ZipkinSpanIDUniquifier() Adjuster { - return Func(func(trace *model.Trace) (*model.Trace, error) { + return Func(func(trace *model.Trace) { deduper := &spanIDDeduper{trace: trace} deduper.groupSpansByID() deduper.uniquifyServerSpanIDs() - return deduper.trace, nil }) } diff --git a/model/adjuster/zipkin_span_id_uniquify_test.go b/model/adjuster/zipkin_span_id_uniquify_test.go index c0b92799c21..8b65cad68aa 100644 --- a/model/adjuster/zipkin_span_id_uniquify_test.go +++ b/model/adjuster/zipkin_span_id_uniquify_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/model" ) @@ -53,8 +52,7 @@ func newZipkinTrace() *model.Trace { func TestZipkinSpanIDUniquifierTriggered(t *testing.T) { trc := newZipkinTrace() deduper := ZipkinSpanIDUniquifier() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) clientSpan := trc.Spans[0] assert.Equal(t, clientSpanID, clientSpan.SpanID, "client span ID should not change") @@ -73,8 +71,7 @@ func TestZipkinSpanIDUniquifierNotTriggered(t *testing.T) { trc.Spans = trc.Spans[1:] // remove client span deduper := ZipkinSpanIDUniquifier() - trc, err := deduper.Adjust(trc) - require.NoError(t, err) + deduper.Adjust(trc) serverSpanID := clientSpanID // for better readability serverSpan := trc.Spans[0] diff --git a/plugin/storage/memory/memory.go b/plugin/storage/memory/memory.go index 23ec0bd2d39..02dff26a584 100644 --- a/plugin/storage/memory/memory.go +++ b/plugin/storage/memory/memory.go @@ -89,9 +89,8 @@ func (st *Store) GetDependencies(ctx context.Context, endTs time.Time, lookback defer m.Unlock() deps := map[string]*model.DependencyLink{} startTs := endTs.Add(-1 * lookback) - for _, orig := range m.traces { - // ZipkinSpanIDUniquifier never returns an err - trace, _ := m.deduper.Adjust(orig) + for _, trace := range m.traces { + m.deduper.Adjust(trace) if traceIsBetweenStartAndEnd(startTs, endTs, trace) { for _, s := range trace.Spans { parentSpan := findSpan(trace, s.ParentSpanID())