Skip to content

Commit

Permalink
Revert "[chore]: Replace pkg/multierror with standard errors.Join (ja…
Browse files Browse the repository at this point in the history
…egertracing#4293)"

This reverts commit 18843de.

Signed-off-by: Pablo Baeyens <[email protected]>
  • Loading branch information
mx-psi committed Mar 22, 2023
1 parent 8de19fa commit e2fa30f
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 34 deletions.
14 changes: 7 additions & 7 deletions cmd/agent/app/reporter/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions cmd/agent/app/reporter/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/handler_archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
7 changes: 4 additions & 3 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion cmd/query/app/querysvc/query_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions cmd/query/app/querysvc/query_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 4 additions & 5 deletions model/adjuster/adjuster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -57,16 +56,16 @@ 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)
if err != nil {
if c.failFast {
return trace, err
}
errs = append(errs, err)
errors = append(errors, err)
}
}
return trace, errors.Join(errs...)
return trace, multierror.Wrap(errors)
}
2 changes: 1 addition & 1 deletion model/adjuster/adjuster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down
8 changes: 4 additions & 4 deletions model/converter/thrift/zipkin/to_domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -84,21 +84,21 @@ 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
jSpan.Process = processes.add(jSpan.Process)
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) {
Expand Down
56 changes: 56 additions & 0 deletions pkg/multierror/multierror.go
Original file line number Diff line number Diff line change
@@ -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, ", "))
}
64 changes: 64 additions & 0 deletions pkg/multierror/multierror_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
4 changes: 2 additions & 2 deletions plugin/storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package storage

import (
"errors"
"flag"
"fmt"
"io"
Expand All @@ -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"
Expand Down Expand Up @@ -327,7 +327,7 @@ func (f *Factory) Close() error {
}
}
}
return errors.Join(errs...)
return multierror.Wrap(errs)
}

func (f *Factory) publishOpts() {
Expand Down
8 changes: 4 additions & 4 deletions storage/spanstore/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions storage/spanstore/composite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

0 comments on commit e2fa30f

Please sign in to comment.