Skip to content

Commit

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

## Which problem is this PR solving?
Resolves jaegertracing#4292

Signed-off-by: Clement <ClementBarnes@mail.uk>
  • Loading branch information
clement2026 authored Mar 11, 2023
1 parent a172bcc commit 18843de
Show file tree
Hide file tree
Showing 14 changed files with 34 additions and 156 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 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...)
}
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, %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 {
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, 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})
}
7 changes: 3 additions & 4 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions cmd/query/app/querysvc/query_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions cmd/query/app/querysvc/query_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions model/adjuster/adjuster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -56,16 +57,16 @@ 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)
if err != nil {
if c.failFast {
return trace, err
}
errors = append(errors, err)
errs = append(errs, err)
}
}
return trace, multierror.Wrap(errors)
return trace, errors.Join(errs...)
}
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, %s]", adjErr, adjErr),
err: fmt.Sprintf("%s\n%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 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
jSpan.Process = processes.add(jSpan.Process)
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) {
Expand Down
56 changes: 0 additions & 56 deletions pkg/multierror/multierror.go

This file was deleted.

64 changes: 0 additions & 64 deletions pkg/multierror/multierror_test.go

This file was deleted.

4 changes: 2 additions & 2 deletions plugin/storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package storage

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

0 comments on commit 18843de

Please sign in to comment.