Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[all] Replace multierr by errors.Join #8630

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ linters-settings:
- pkg: github.com/pkg/errors
desc: "Use 'errors' or 'fmt' instead of github.com/pkg/errors"
- pkg: github.com/hashicorp/go-multierror
desc: "Use go.uber.org/multierr instead of github.com/hashicorp/go-multierror"
desc: "Use errors.Join instead of github.com/hashicorp/go-multierror"
- pkg: go.uber.org/multierr
desc: "Use errors.Join instead of go.uber.org/multierr"
# Add a different guard rule so that we can ignore tests.
ignore-in-test:
deny:
Expand Down
2 changes: 1 addition & 1 deletion cmd/builder/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ require (
github.com/spf13/cobra v1.7.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.8.4
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.26.0
)

Expand All @@ -29,6 +28,7 @@ require (
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/sys v0.12.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
3 changes: 1 addition & 2 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"path/filepath"
"strings"

"go.uber.org/multierr"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -83,7 +82,7 @@ func NewDefaultConfig() Config {

// Validate checks whether the current configuration is valid
func (c *Config) Validate() error {
return multierr.Combine(
return errors.Join(
validateModules(c.Extensions),
validateModules(c.Receivers),
validateModules(c.Exporters),
Expand Down
13 changes: 6 additions & 7 deletions component/componenttest/configtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
package componenttest // import "go.opentelemetry.io/collector/component/componenttest"

import (
"errors"
"fmt"
"reflect"
"regexp"
"strings"

"go.uber.org/multierr"
)

// The regular expression for valid config field tag.
Expand Down Expand Up @@ -37,17 +36,17 @@ func CheckConfigStruct(config any) error {
// If the type is a struct it goes to each of its fields to check for the proper
// tags.
func validateConfigDataType(t reflect.Type) error {
var errs error
var errs []error

switch t.Kind() {
case reflect.Ptr:
errs = multierr.Append(errs, validateConfigDataType(t.Elem()))
errs = append(errs, validateConfigDataType(t.Elem()))
case reflect.Struct:
// Reflect on the pointed data and check each of its fields.
nf := t.NumField()
for i := 0; i < nf; i++ {
f := t.Field(i)
errs = multierr.Append(errs, checkStructFieldTags(f))
errs = append(errs, checkStructFieldTags(f))
Comment on lines +39 to +49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter that much here, but this adds lots of allocations. Do we care? Do we want to have a helper type (internal) "MultiError" with an append that does not append to the array if nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. I don't think we care in most places but there are some where we definitely do care; e.g. the fanoutconsumer Consume* calls seems like a place where we should avoid this.

Should we use the helper just for those places? We could also just continue using multierr on those for now and report this on golang/go

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

report this on golang/go

I think we should definitely do that.

Should we use the helper just for those places? We could also just continue using multierr on those for now and

I think we can continue to use multierr for that.

}
default:
// The config object can carry other types but they are not used when
Expand All @@ -56,8 +55,8 @@ func validateConfigDataType(t reflect.Type) error {
// reflect.UnsafePointer.
}

if errs != nil {
return fmt.Errorf("type %q from package %q has invalid config settings: %w", t.Name(), t.PkgPath(), errs)
if err := errors.Join(errs...); err != nil {
return fmt.Errorf("type %q from package %q has invalid config settings: %w", t.Name(), t.PkgPath(), err)
}

return nil
Expand Down
29 changes: 14 additions & 15 deletions component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
package component // import "go.opentelemetry.io/collector/component"

import (
"errors"
"reflect"

"go.uber.org/multierr"

"go.opentelemetry.io/collector/confmap"
)

Expand Down Expand Up @@ -55,33 +54,33 @@ func validate(v reflect.Value) error {
case reflect.Ptr:
return validate(v.Elem())
case reflect.Struct:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
var errs []error
errs = append(errs, callValidateIfPossible(v))
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.NumField(); i++ {
if !v.Type().Field(i).IsExported() {
continue
}
errs = multierr.Append(errs, validate(v.Field(i)))
errs = append(errs, validate(v.Field(i)))
}
return errs
return errors.Join(errs...)
case reflect.Slice, reflect.Array:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
var errs []error
errs = append(errs, callValidateIfPossible(v))
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.Len(); i++ {
errs = multierr.Append(errs, validate(v.Index(i)))
errs = append(errs, validate(v.Index(i)))
}
return errs
return errors.Join(errs...)
case reflect.Map:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
var errs []error
errs = append(errs, callValidateIfPossible(v))
iter := v.MapRange()
for iter.Next() {
errs = multierr.Append(errs, validate(iter.Key()))
errs = multierr.Append(errs, validate(iter.Value()))
errs = append(errs, validate(iter.Key()))
errs = append(errs, validate(iter.Value()))
}
return errs
return errors.Join(errs...)
default:
return callValidateIfPossible(v)
}
Expand Down
6 changes: 5 additions & 1 deletion component/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,11 @@ func TestValidateConfig(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validate(reflect.ValueOf(tt.cfg))
assert.Equal(t, tt.expected, err)
if tt.expected == nil {
assert.Nil(t, err)
} else {
assert.EqualError(t, err, tt.expected.Error())
}
})
}
}
2 changes: 1 addition & 1 deletion component/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
go.opentelemetry.io/collector/pdata v1.0.0-rcv0015
go.opentelemetry.io/otel/metric v1.19.0
go.opentelemetry.io/otel/trace v1.19.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.26.0
)

Expand All @@ -26,6 +25,7 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0015 // indirect
go.opentelemetry.io/otel v1.19.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/net v0.15.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/text v0.13.0 // indirect
Expand Down
1 change: 0 additions & 1 deletion confmap/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/mitchellh/mapstructure v1.5.1-0.20220423185008-bf980b35cac4
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0015
go.uber.org/multierr v1.11.0
gopkg.in/yaml.v3 v3.0.1
)

Expand Down
2 changes: 0 additions & 2 deletions confmap/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
Expand Down
16 changes: 7 additions & 9 deletions confmap/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"regexp"
"strings"

"go.uber.org/multierr"

"go.opentelemetry.io/collector/featuregate"
)

Expand Down Expand Up @@ -175,26 +173,26 @@ func (mr *Resolver) Watch() <-chan error {
func (mr *Resolver) Shutdown(ctx context.Context) error {
close(mr.watcher)

var errs error
errs = multierr.Append(errs, mr.closeIfNeeded(ctx))
var errs []error
errs = append(errs, mr.closeIfNeeded(ctx))
for _, p := range mr.providers {
errs = multierr.Append(errs, p.Shutdown(ctx))
errs = append(errs, p.Shutdown(ctx))
}

return errs
return errors.Join(errs...)
}

func (mr *Resolver) onChange(event *ChangeEvent) {
mr.watcher <- event.Error
}

func (mr *Resolver) closeIfNeeded(ctx context.Context) error {
var err error
var errs []error
for _, ret := range mr.closers {
err = multierr.Append(err, ret(ctx))
errs = append(errs, ret(ctx))
}
mr.closers = nil
return err
return errors.Join(errs...)
}

func (mr *Resolver) retrieveValue(ctx context.Context, uri location) (*Retrieved, error) {
Expand Down
1 change: 0 additions & 1 deletion consumer/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/net v0.15.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/text v0.13.0 // indirect
Expand Down
2 changes: 0 additions & 2 deletions consumer/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
Expand Down
19 changes: 10 additions & 9 deletions exporter/exporterhelper/obsexporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte

import (
"context"
"errors"

"go.opencensus.io/stats"
"go.opencensus.io/tag"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/trace"
"go.uber.org/multierr"
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -82,45 +82,46 @@ func (or *ObsReport) createOtelMetrics(cfg ObsReportSettings) error {
}
meter := cfg.ExporterCreateSettings.MeterProvider.Meter(exporterScope)

var errors, err error
var errs []error
var err error

or.sentSpans, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.SentSpansKey,
metric.WithDescription("Number of spans successfully sent to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

or.failedToSendSpans, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.FailedToSendSpansKey,
metric.WithDescription("Number of spans in failed attempts to send to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

or.sentMetricPoints, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.SentMetricPointsKey,
metric.WithDescription("Number of metric points successfully sent to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

or.failedToSendMetricPoints, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.FailedToSendMetricPointsKey,
metric.WithDescription("Number of metric points in failed attempts to send to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

or.sentLogRecords, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.SentLogRecordsKey,
metric.WithDescription("Number of log record successfully sent to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

or.failedToSendLogRecords, err = meter.Int64Counter(
obsmetrics.ExporterPrefix+obsmetrics.FailedToSendLogRecordsKey,
metric.WithDescription("Number of log records in failed attempts to send to destination."),
metric.WithUnit("1"))
errors = multierr.Append(errors, err)
errs = append(errs, err)

return errors
return errors.Join(errs...)
}

// StartTracesOp is called at the start of an Export operation.
Expand Down
2 changes: 1 addition & 1 deletion exporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
go.opentelemetry.io/otel/metric v1.19.0
go.opentelemetry.io/otel/sdk v1.19.0
go.opentelemetry.io/otel/trace v1.19.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.26.0
golang.org/x/sys v0.12.0
)
Expand Down Expand Up @@ -53,6 +52,7 @@ require (
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0015 // indirect
go.opentelemetry.io/otel/exporters/prometheus v0.42.0 // indirect
go.opentelemetry.io/otel/sdk/metric v1.19.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/net v0.15.0 // indirect
golang.org/x/text v0.13.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 // indirect
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlpexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ require (
)

require (
cloud.google.com/go/compute/metadata v0.2.4-0.20230617002413-005d2dfb6b68 // indirect
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
Expand Down Expand Up @@ -113,3 +112,5 @@ retract (
replace go.opentelemetry.io/collector/connector => ../../connector

replace go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry

replace cloud.google.com/go => cloud.google.com/go v0.110.2
Loading