From 715bf7f512f9513bd1c167a51bf684d552d385f1 Mon Sep 17 00:00:00 2001 From: Clement Date: Sat, 11 Mar 2023 17:33:07 +0800 Subject: [PATCH] Replace pkg/multierror with standard errors.Join Signed-off-by: Clement --- cmd/agent/app/reporter/reporter.go | 14 ++--- cmd/agent/app/reporter/reporter_test.go | 4 +- cmd/query/app/handler_archive_test.go | 2 +- cmd/query/app/http_handler.go | 7 +-- cmd/query/app/querysvc/query_service.go | 3 +- cmd/query/app/querysvc/query_service_test.go | 5 +- model/adjuster/adjuster.go | 9 +-- model/adjuster/adjuster_test.go | 2 +- model/converter/thrift/zipkin/to_domain.go | 8 +-- pkg/multierror/multierror.go | 56 ----------------- pkg/multierror/multierror_test.go | 64 -------------------- plugin/storage/factory.go | 4 +- storage/spanstore/composite.go | 8 +-- storage/spanstore/composite_test.go | 4 +- 14 files changed, 34 insertions(+), 156 deletions(-) delete mode 100644 pkg/multierror/multierror.go delete mode 100644 pkg/multierror/multierror_test.go diff --git a/cmd/agent/app/reporter/reporter.go b/cmd/agent/app/reporter/reporter.go index 5b300ad50da..550a4a2dd9b 100644 --- a/cmd/agent/app/reporter/reporter.go +++ b/cmd/agent/app/reporter/reporter.go @@ -17,8 +17,8 @@ package reporter import ( "context" + "errors" - "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/thrift-gen/jaeger" "github.com/jaegertracing/jaeger/thrift-gen/zipkincore" ) @@ -43,22 +43,22 @@ func NewMultiReporter(reps ...Reporter) MultiReporter { // EmitZipkinBatch calls each EmitZipkinBatch, returning the first error. func (mr MultiReporter) EmitZipkinBatch(ctx context.Context, spans []*zipkincore.Span) error { - var errors []error + var errs []error for _, rep := range mr { if err := rep.EmitZipkinBatch(ctx, spans); err != nil { - errors = append(errors, err) + errs = append(errs, err) } } - return multierror.Wrap(errors) + return errors.Join(errs...) } // EmitBatch calls each EmitBatch, returning the first error. func (mr MultiReporter) EmitBatch(ctx context.Context, batch *jaeger.Batch) error { - var errors []error + var errs []error for _, rep := range mr { if err := rep.EmitBatch(ctx, batch); err != nil { - errors = append(errors, err) + errs = append(errs, err) } } - return multierror.Wrap(errors) + return errors.Join(errs...) } diff --git a/cmd/agent/app/reporter/reporter_test.go b/cmd/agent/app/reporter/reporter_test.go index 5f46c716631..7f5b86433f5 100644 --- a/cmd/agent/app/reporter/reporter_test.go +++ b/cmd/agent/app/reporter/reporter_test.go @@ -60,8 +60,8 @@ func TestMultiReporterErrors(t *testing.T) { {}, }, }) - assert.EqualError(t, e1, fmt.Sprintf("[%s, %s]", errMsg, errMsg)) - assert.EqualError(t, e2, fmt.Sprintf("[%s, %s]", errMsg, errMsg)) + assert.EqualError(t, e1, fmt.Sprintf("%s\n%s", errMsg, errMsg)) + assert.EqualError(t, e2, fmt.Sprintf("%s\n%s", errMsg, errMsg)) } type mockReporter struct { diff --git a/cmd/query/app/handler_archive_test.go b/cmd/query/app/handler_archive_test.go index a244f737050..72c51034983 100644 --- a/cmd/query/app/handler_archive_test.go +++ b/cmd/query/app/handler_archive_test.go @@ -123,6 +123,6 @@ func TestArchiveTrace_WriteErrors(t *testing.T) { Return(mockTrace, nil).Once() var response structuredResponse err := postJSON(ts.server.URL+"/api/archive/"+mockTraceID.String(), []string{}, &response) - assert.EqualError(t, err, `500 error from server: {"data":null,"total":0,"limit":0,"offset":0,"errors":[{"code":500,"msg":"[cannot save, cannot save]"}]}`+"\n") + assert.EqualError(t, err, `500 error from server: {"data":null,"total":0,"limit":0,"offset":0,"errors":[{"code":500,"msg":"cannot save\ncannot save"}]}`+"\n") }, querysvc.QueryServiceOptions{ArchiveSpanWriter: mockWriter}) } diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 49b7805a628..3e09895d37b 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -35,7 +35,6 @@ import ( "github.com/jaegertracing/jaeger/model" uiconv "github.com/jaegertracing/jaeger/model/converter/json" ui "github.com/jaegertracing/jaeger/model/json" - "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics" @@ -355,17 +354,17 @@ func (aH *APIHandler) metrics(w http.ResponseWriter, r *http.Request, getMetrics } func (aH *APIHandler) convertModelToUI(trace *model.Trace, adjust bool) (*ui.Trace, *structuredError) { - var errors []error + var errs []error if adjust { var err error trace, err = aH.queryService.Adjust(trace) if err != nil { - errors = append(errors, err) + errs = append(errs, err) } } uiTrace := uiconv.FromDomain(trace) var uiError *structuredError - if err := multierror.Wrap(errors); err != nil { + if err := errors.Join(errs...); err != nil { uiError = &structuredError{ Msg: err.Error(), TraceID: uiTrace.TraceID, diff --git a/cmd/query/app/querysvc/query_service.go b/cmd/query/app/querysvc/query_service.go index a88575d79d4..7dde2813cae 100644 --- a/cmd/query/app/querysvc/query_service.go +++ b/cmd/query/app/querysvc/query_service.go @@ -23,7 +23,6 @@ import ( "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/model/adjuster" - "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/spanstore" @@ -110,7 +109,7 @@ func (qs QueryService) ArchiveTrace(ctx context.Context, traceID model.TraceID) writeErrors = append(writeErrors, err) } } - return multierror.Wrap(writeErrors) + return errors.Join(writeErrors...) } // Adjust applies adjusters to the trace. diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index 79e66253be2..ee214c33ea2 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -239,10 +239,9 @@ func TestArchiveTraceWithArchiveWriterError(t *testing.T) { type contextKey string ctx := context.Background() - multiErr := tqs.queryService.ArchiveTrace(context.WithValue(ctx, contextKey("foo"), "bar"), mockTraceID) - assert.Len(t, multiErr, 2) + joinErr := tqs.queryService.ArchiveTrace(context.WithValue(ctx, contextKey("foo"), "bar"), mockTraceID) // There are two spans in the mockTrace, ArchiveTrace should return a wrapped error. - assert.EqualError(t, multiErr, "[cannot save, cannot save]") + assert.EqualError(t, joinErr, "cannot save\ncannot save") } // Test QueryService.ArchiveTrace() with correctly configured ArchiveSpanWriter. diff --git a/model/adjuster/adjuster.go b/model/adjuster/adjuster.go index 2ac9e619caf..5cce6c4a348 100644 --- a/model/adjuster/adjuster.go +++ b/model/adjuster/adjuster.go @@ -16,8 +16,9 @@ package adjuster import ( + "errors" + "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/pkg/multierror" ) // Adjuster applies certain modifications to a Trace object. @@ -56,7 +57,7 @@ type sequence struct { } func (c sequence) Adjust(trace *model.Trace) (*model.Trace, error) { - var errors []error + var errs []error for _, adjuster := range c.adjusters { var err error trace, err = adjuster.Adjust(trace) @@ -64,8 +65,8 @@ func (c sequence) Adjust(trace *model.Trace) (*model.Trace, error) { if c.failFast { return trace, err } - errors = append(errors, err) + errs = append(errs, err) } } - return trace, multierror.Wrap(errors) + return trace, errors.Join(errs...) } diff --git a/model/adjuster/adjuster_test.go b/model/adjuster/adjuster_test.go index 569729a0a93..2c0cefbfbd6 100644 --- a/model/adjuster/adjuster_test.go +++ b/model/adjuster/adjuster_test.go @@ -45,7 +45,7 @@ func TestSequences(t *testing.T) { }{ { adjuster: adjuster.Sequence(adj, failingAdj, adj, failingAdj), - err: fmt.Sprintf("[%s, %s]", adjErr, adjErr), + err: fmt.Sprintf("%s\n%s", adjErr, adjErr), lastSpanID: 2, }, { diff --git a/model/converter/thrift/zipkin/to_domain.go b/model/converter/thrift/zipkin/to_domain.go index 73e68f84576..0ee67e5a6ad 100644 --- a/model/converter/thrift/zipkin/to_domain.go +++ b/model/converter/thrift/zipkin/to_domain.go @@ -20,12 +20,12 @@ import ( "encoding/base64" "encoding/binary" "encoding/json" + "errors" "fmt" "github.com/opentracing/opentracing-go/ext" "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/thrift-gen/zipkincore" ) @@ -84,13 +84,13 @@ func ToDomainSpan(zSpan *zipkincore.Span) ([]*model.Span, error) { type toDomain struct{} func (td toDomain) ToDomain(zSpans []*zipkincore.Span) (*model.Trace, error) { - var errors []error + var errs []error processes := newProcessHashtable() trace := &model.Trace{} for _, zSpan := range zSpans { jSpans, err := td.ToDomainSpans(zSpan) if err != nil { - errors = append(errors, err) + errs = append(errs, err) } for _, jSpan := range jSpans { // remove duplicate Process instances @@ -98,7 +98,7 @@ func (td toDomain) ToDomain(zSpans []*zipkincore.Span) (*model.Trace, error) { trace.Spans = append(trace.Spans, jSpan) } } - return trace, multierror.Wrap(errors) + return trace, errors.Join(errs...) } func (td toDomain) ToDomainSpans(zSpan *zipkincore.Span) ([]*model.Span, error) { diff --git a/pkg/multierror/multierror.go b/pkg/multierror/multierror.go deleted file mode 100644 index bb1994490a1..00000000000 --- a/pkg/multierror/multierror.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) 2019 The Jaeger Authors. -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package multierror - -import ( - "fmt" - "strings" -) - -// Wrap takes a slice of errors and returns a single error that encapsulates -// those underlying errors. If the slice is nil or empty it returns nil. -// If the slice only contains a single element, that error is returned directly. -// When more than one error is wrapped, the Error() string is a concatenation -// of the Error() values of all underlying errors. -func Wrap(errs []error) error { - return multiError(errs).flatten() -} - -// multiError bundles several errors together into a single error. -type multiError []error - -// flatten returns either: nil, the only error, or the multiError instance itself -// if there are 0, 1, or more errors in the slice respectively. -func (errors multiError) flatten() error { - switch len(errors) { - case 0: - return nil - case 1: - return errors[0] - default: - return errors - } -} - -// Error returns a string like "[e1, e2, ...]" where each eN is the Error() of -// each error in the slice. -func (errors multiError) Error() string { - parts := make([]string, len(errors)) - for i, err := range errors { - parts[i] = err.Error() - } - return fmt.Sprintf("[%s]", strings.Join(parts, ", ")) -} diff --git a/pkg/multierror/multierror_test.go b/pkg/multierror/multierror_test.go deleted file mode 100644 index edd320515b8..00000000000 --- a/pkg/multierror/multierror_test.go +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (c) 2019 The Jaeger Authors. -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package multierror - -import ( - "errors" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func ExampleWrap() { - someFunc := func() error { - return errors.New("doh") - } - - var errs []error - for i := 0; i < 2; i++ { - if err := someFunc(); err != nil { - errs = append(errs, err) - } - fmt.Println(Wrap(errs).Error()) - } - // Output: doh - // [doh, doh] -} - -func TestWrapEmptySlice(t *testing.T) { - var errors []error - e1 := Wrap(errors) - assert.Nil(t, e1) - e2 := Wrap([]error{}) - assert.Nil(t, e2) -} - -func TestWrapSingleError(t *testing.T) { - err := errors.New("doh") - e1 := Wrap([]error{err}) - assert.Error(t, e1) - assert.Equal(t, err, e1) - assert.Equal(t, "doh", e1.Error()) -} - -func TestWrapManyErrors(t *testing.T) { - err1 := errors.New("ay") - err2 := errors.New("caramba") - e1 := Wrap([]error{err1, err2}) - assert.Error(t, e1) - assert.Equal(t, "[ay, caramba]", e1.Error()) -} diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 84d2aa4e01c..b2c6b7a9226 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -16,6 +16,7 @@ package storage import ( + "errors" "flag" "fmt" "io" @@ -24,7 +25,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/metrics" - "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/storage/badger" "github.com/jaegertracing/jaeger/plugin/storage/cassandra" @@ -327,7 +327,7 @@ func (f *Factory) Close() error { } } } - return multierror.Wrap(errs) + return errors.Join(errs...) } func (f *Factory) publishOpts() { diff --git a/storage/spanstore/composite.go b/storage/spanstore/composite.go index 5746d09b925..68f9607448c 100644 --- a/storage/spanstore/composite.go +++ b/storage/spanstore/composite.go @@ -17,9 +17,9 @@ package spanstore import ( "context" + "errors" "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/pkg/multierror" ) // CompositeWriter is a span Writer that tries to save spans into several underlying span Writers @@ -36,11 +36,11 @@ func NewCompositeWriter(spanWriters ...Writer) *CompositeWriter { // WriteSpan calls WriteSpan on each span writer. It will sum up failures, it is not transactional func (c *CompositeWriter) WriteSpan(ctx context.Context, span *model.Span) error { - var errors []error + var errs []error for _, writer := range c.spanWriters { if err := writer.WriteSpan(ctx, span); err != nil { - errors = append(errors, err) + errs = append(errs, err) } } - return multierror.Wrap(errors) + return errors.Join(errs...) } diff --git a/storage/spanstore/composite_test.go b/storage/spanstore/composite_test.go index ec7aa8cd661..e8cb24d6a08 100644 --- a/storage/spanstore/composite_test.go +++ b/storage/spanstore/composite_test.go @@ -48,10 +48,10 @@ func TestCompositeWriteSpanStoreSuccess(t *testing.T) { func TestCompositeWriteSpanStoreSecondFailure(t *testing.T) { c := NewCompositeWriter(&errProneWriteSpanStore{}, &errProneWriteSpanStore{}) - assert.EqualError(t, c.WriteSpan(context.Background(), nil), fmt.Sprintf("[%s, %s]", errIWillAlwaysFail, errIWillAlwaysFail)) + assert.EqualError(t, c.WriteSpan(context.Background(), nil), fmt.Sprintf("%s\n%s", errIWillAlwaysFail, errIWillAlwaysFail)) } func TestCompositeWriteSpanStoreFirstFailure(t *testing.T) { c := NewCompositeWriter(&errProneWriteSpanStore{}, &noopWriteSpanStore{}) - assert.Equal(t, errIWillAlwaysFail, c.WriteSpan(context.Background(), nil)) + assert.EqualError(t, c.WriteSpan(context.Background(), nil), errIWillAlwaysFail.Error()) }