From e2fa30f2403d372fb4fa4ca98286ef207a1dc22c Mon Sep 17 00:00:00 2001
From: Pablo Baeyens <pablo.baeyens@datadoghq.com>
Date: Wed, 22 Mar 2023 12:04:47 +0100
Subject: [PATCH] Revert "[chore]: Replace pkg/multierror with standard
errors.Join (#4293)"
This reverts commit 18843defa5edbe149b6a984fdf54951ab4d8ba5f.
Signed-off-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
---
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, 156 insertions(+), 34 deletions(-)
create mode 100644 pkg/multierror/multierror.go
create mode 100644 pkg/multierror/multierror_test.go
diff --git a/cmd/agent/app/reporter/reporter.go b/cmd/agent/app/reporter/reporter.go
index 550a4a2dd9ba..5b300ad50da7 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 errs []error
+ var errors []error
for _, rep := range mr {
if err := rep.EmitZipkinBatch(ctx, spans); err != nil {
- errs = append(errs, err)
+ errors = append(errors, err)
}
}
- return errors.Join(errs...)
+ return multierror.Wrap(errors)
}
// EmitBatch calls each EmitBatch, returning the first error.
func (mr MultiReporter) EmitBatch(ctx context.Context, batch *jaeger.Batch) error {
- var errs []error
+ var errors []error
for _, rep := range mr {
if err := rep.EmitBatch(ctx, batch); err != nil {
- errs = append(errs, err)
+ errors = append(errors, err)
}
}
- return errors.Join(errs...)
+ return multierror.Wrap(errors)
}
diff --git a/cmd/agent/app/reporter/reporter_test.go b/cmd/agent/app/reporter/reporter_test.go
index 7f5b86433f54..5f46c7166315 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\n%s", errMsg, errMsg))
- assert.EqualError(t, e2, fmt.Sprintf("%s\n%s", errMsg, errMsg))
+ assert.EqualError(t, e1, fmt.Sprintf("[%s, %s]", errMsg, errMsg))
+ assert.EqualError(t, e2, fmt.Sprintf("[%s, %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 72c51034983c..a244f7370506 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\ncannot save"}]}`+"\n")
+ 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")
}, querysvc.QueryServiceOptions{ArchiveSpanWriter: mockWriter})
}
diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go
index 3e09895d37bb..49b7805a628c 100644
--- a/cmd/query/app/http_handler.go
+++ b/cmd/query/app/http_handler.go
@@ -35,6 +35,7 @@ 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"
@@ -354,17 +355,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 errs []error
+ var errors []error
if adjust {
var err error
trace, err = aH.queryService.Adjust(trace)
if err != nil {
- errs = append(errs, err)
+ errors = append(errors, err)
}
}
uiTrace := uiconv.FromDomain(trace)
var uiError *structuredError
- if err := errors.Join(errs...); err != nil {
+ if err := multierror.Wrap(errors); 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 7dde2813cae3..a88575d79d43 100644
--- a/cmd/query/app/querysvc/query_service.go
+++ b/cmd/query/app/querysvc/query_service.go
@@ -23,6 +23,7 @@ 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"
@@ -109,7 +110,7 @@ func (qs QueryService) ArchiveTrace(ctx context.Context, traceID model.TraceID)
writeErrors = append(writeErrors, err)
}
}
- return errors.Join(writeErrors...)
+ return multierror.Wrap(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 ee214c33ea28..79e66253be2a 100644
--- a/cmd/query/app/querysvc/query_service_test.go
+++ b/cmd/query/app/querysvc/query_service_test.go
@@ -239,9 +239,10 @@ func TestArchiveTraceWithArchiveWriterError(t *testing.T) {
type contextKey string
ctx := context.Background()
- joinErr := tqs.queryService.ArchiveTrace(context.WithValue(ctx, contextKey("foo"), "bar"), mockTraceID)
+ multiErr := tqs.queryService.ArchiveTrace(context.WithValue(ctx, contextKey("foo"), "bar"), mockTraceID)
+ assert.Len(t, multiErr, 2)
// There are two spans in the mockTrace, ArchiveTrace should return a wrapped error.
- assert.EqualError(t, joinErr, "cannot save\ncannot save")
+ assert.EqualError(t, multiErr, "[cannot save, cannot save]")
}
// Test QueryService.ArchiveTrace() with correctly configured ArchiveSpanWriter.
diff --git a/model/adjuster/adjuster.go b/model/adjuster/adjuster.go
index 5cce6c4a348d..2ac9e619caff 100644
--- a/model/adjuster/adjuster.go
+++ b/model/adjuster/adjuster.go
@@ -16,9 +16,8 @@
package adjuster
import (
- "errors"
-
"github.com/jaegertracing/jaeger/model"
+ "github.com/jaegertracing/jaeger/pkg/multierror"
)
// Adjuster applies certain modifications to a Trace object.
@@ -57,7 +56,7 @@ type sequence struct {
}
func (c sequence) Adjust(trace *model.Trace) (*model.Trace, error) {
- var errs []error
+ var errors []error
for _, adjuster := range c.adjusters {
var err error
trace, err = adjuster.Adjust(trace)
@@ -65,8 +64,8 @@ func (c sequence) Adjust(trace *model.Trace) (*model.Trace, error) {
if c.failFast {
return trace, err
}
- errs = append(errs, err)
+ errors = append(errors, err)
}
}
- return trace, errors.Join(errs...)
+ return trace, multierror.Wrap(errors)
}
diff --git a/model/adjuster/adjuster_test.go b/model/adjuster/adjuster_test.go
index 2c0cefbfbd67..569729a0a931 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\n%s", adjErr, adjErr),
+ err: fmt.Sprintf("[%s, %s]", adjErr, adjErr),
lastSpanID: 2,
},
{
diff --git a/model/converter/thrift/zipkin/to_domain.go b/model/converter/thrift/zipkin/to_domain.go
index 0ee67e5a6ad1..73e68f845761 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 errs []error
+ var errors []error
processes := newProcessHashtable()
trace := &model.Trace{}
for _, zSpan := range zSpans {
jSpans, err := td.ToDomainSpans(zSpan)
if err != nil {
- errs = append(errs, err)
+ errors = append(errors, 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, errors.Join(errs...)
+ return trace, multierror.Wrap(errors)
}
func (td toDomain) ToDomainSpans(zSpan *zipkincore.Span) ([]*model.Span, error) {
diff --git a/pkg/multierror/multierror.go b/pkg/multierror/multierror.go
new file mode 100644
index 000000000000..bb1994490a1b
--- /dev/null
+++ b/pkg/multierror/multierror.go
@@ -0,0 +1,56 @@
+// 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
new file mode 100644
index 000000000000..edd320515b86
--- /dev/null
+++ b/pkg/multierror/multierror_test.go
@@ -0,0 +1,64 @@
+// 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 b2c6b7a92264..84d2aa4e01cb 100644
--- a/plugin/storage/factory.go
+++ b/plugin/storage/factory.go
@@ -16,7 +16,6 @@
package storage
import (
- "errors"
"flag"
"fmt"
"io"
@@ -25,6 +24,7 @@ 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 errors.Join(errs...)
+ return multierror.Wrap(errs)
}
func (f *Factory) publishOpts() {
diff --git a/storage/spanstore/composite.go b/storage/spanstore/composite.go
index 68f9607448c0..5746d09b9251 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 errs []error
+ var errors []error
for _, writer := range c.spanWriters {
if err := writer.WriteSpan(ctx, span); err != nil {
- errs = append(errs, err)
+ errors = append(errors, err)
}
}
- return errors.Join(errs...)
+ return multierror.Wrap(errors)
}
diff --git a/storage/spanstore/composite_test.go b/storage/spanstore/composite_test.go
index e8cb24d6a089..ec7aa8cd661f 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\n%s", errIWillAlwaysFail, errIWillAlwaysFail))
+ assert.EqualError(t, c.WriteSpan(context.Background(), nil), fmt.Sprintf("[%s, %s]", errIWillAlwaysFail, errIWillAlwaysFail))
}
func TestCompositeWriteSpanStoreFirstFailure(t *testing.T) {
c := NewCompositeWriter(&errProneWriteSpanStore{}, &noopWriteSpanStore{})
- assert.EqualError(t, c.WriteSpan(context.Background(), nil), errIWillAlwaysFail.Error())
+ assert.Equal(t, errIWillAlwaysFail, c.WriteSpan(context.Background(), nil))
}