Skip to content

Commit

Permalink
Deprecate exporterhelper.RetrySettings in favor of configretry.BackOf…
Browse files Browse the repository at this point in the history
…fSettings

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed Dec 12, 2023
1 parent 3495332 commit ff1cec1
Show file tree
Hide file tree
Showing 47 changed files with 309 additions and 162 deletions.
13 changes: 13 additions & 0 deletions .chloggen/deprecateretry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: 'deprecation'

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: "exporterhelper"

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Deprecate exporterhelper.RetrySettings in favor of configretry.BackOffSettings"

# One or more tracking issues or pull requests related to the change
issues: [9091]
1 change: 1 addition & 0 deletions cmd/otelcorecol/builder-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ replaces:
- go.opentelemetry.io/collector/config/confighttp => ../../config/confighttp
- go.opentelemetry.io/collector/config/confignet => ../../config/confignet
- go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
- go.opentelemetry.io/collector/config/configretry => ../config/configretry
- go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry
- go.opentelemetry.io/collector/config/configtls => ../../config/configtls
- go.opentelemetry.io/collector/config/internal => ../../config/internal
Expand Down
3 changes: 3 additions & 0 deletions cmd/otelcorecol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ require (
go.opentelemetry.io/collector/config/confighttp v0.91.0 // indirect
go.opentelemetry.io/collector/config/confignet v0.91.0 // indirect
go.opentelemetry.io/collector/config/configopaque v0.91.0 // indirect
go.opentelemetry.io/collector/config/configretry v0.91.0 // indirect
go.opentelemetry.io/collector/config/configtelemetry v0.91.0 // indirect
go.opentelemetry.io/collector/config/configtls v0.91.0 // indirect
go.opentelemetry.io/collector/config/internal v0.91.0 // indirect
Expand Down Expand Up @@ -145,6 +146,8 @@ replace go.opentelemetry.io/collector/config/confignet => ../../config/confignet

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

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

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

replace go.opentelemetry.io/collector/config/configtls => ../../config/configtls
Expand Down
2 changes: 1 addition & 1 deletion component/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Host interface {
// GetExporters can be called by the component anytime after Component.Start() begins and
// until Component.Shutdown() ends.
//
// Deprecated: [0.79.0] This function will be removed in the future.
// Deprecated: `[0.79.0]` This function will be removed in the future.
// Several components in the contrib repository use this function so it cannot be removed
// before those cases are removed. In most cases, use of this function can be replaced by a
// connector. See https://github.com/open-telemetry/opentelemetry-collector/issues/7370 and
Expand Down
6 changes: 4 additions & 2 deletions config/configgrpc/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ replace go.opentelemetry.io/collector/config/configopaque => ../configopaque

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

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

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

replace go.opentelemetry.io/collector/config/internal => ../internal
Expand All @@ -102,12 +104,12 @@ replace go.opentelemetry.io/collector/exporter => ../../exporter

replace go.opentelemetry.io/collector/receiver => ../../receiver

replace go.opentelemetry.io/collector/connector => ../../connector

replace go.opentelemetry.io/collector/featuregate => ../../featuregate

replace go.opentelemetry.io/collector/pdata => ../../pdata

replace go.opentelemetry.io/collector/component => ../../component

replace go.opentelemetry.io/collector/consumer => ../../consumer

replace go.opentelemetry.io/collector/connector => ../../connector
2 changes: 2 additions & 0 deletions config/confighttp/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,5 @@ replace go.opentelemetry.io/collector/pdata => ../../pdata
replace go.opentelemetry.io/collector/component => ../../component

replace go.opentelemetry.io/collector/consumer => ../../consumer

replace go.opentelemetry.io/collector/config/configretry => ../configretry
1 change: 1 addition & 0 deletions config/configretry/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../../Makefile.Common
66 changes: 66 additions & 0 deletions config/configretry/backoff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package configretry

import (
"errors"
"time"

"github.com/cenkalti/backoff/v4"
)

// NewDefaultBackOffSettings returns the default settings for RetrySettings.
func NewDefaultBackOffSettings() BackOffSettings {
return BackOffSettings{
Enabled: true,
InitialInterval: 5 * time.Second,
RandomizationFactor: backoff.DefaultRandomizationFactor,
Multiplier: backoff.DefaultMultiplier,
MaxInterval: 30 * time.Second,
MaxElapsedTime: 5 * time.Minute,
}
}

// BackOffSettings defines configuration for retrying batches in case of export failure.
// The current supported strategy is exponential backoff.
type BackOffSettings struct {
// Enabled indicates whether to not retry sending batches in case of export failure.
Enabled bool `mapstructure:"enabled"`
// InitialInterval the time to wait after the first failure before retrying.
InitialInterval time.Duration `mapstructure:"initial_interval"`
// RandomizationFactor is a random factor used to calculate next backoffs
// Randomized interval = RetryInterval * (1 ± RandomizationFactor)
RandomizationFactor float64 `mapstructure:"randomization_factor"`
// Multiplier is the value multiplied by the backoff interval bounds
Multiplier float64 `mapstructure:"multiplier"`
// MaxInterval is the upper bound on backoff interval. Once this value is reached the delay between
// consecutive retries will always be `MaxInterval`.
MaxInterval time.Duration `mapstructure:"max_interval"`
// MaxElapsedTime is the maximum amount of time (including retries) spent trying to send a request/batch.
// Once this value is reached, the data is discarded.
MaxElapsedTime time.Duration `mapstructure:"max_elapsed_time"`
}

func (bs *BackOffSettings) Validate() error {
if !bs.Enabled {
return nil
}
if bs.InitialInterval < 0 {
return errors.New("'initial_interval' must be non-negative")
}
if bs.RandomizationFactor < 0 || bs.RandomizationFactor > 1 {
return errors.New("'randomization_factor' must be within [0, 1]")
}
if bs.Multiplier <= 0 {
return errors.New("'multiplier' must be positive")
}
if bs.MaxInterval < 0 {
return errors.New("'max_interval' must be non-negative")
}
if bs.MaxElapsedTime < 0 {
return errors.New("'max_elapsed' time must be non-negative")
}
return nil

}
70 changes: 70 additions & 0 deletions config/configretry/backoff_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package configretry

import (
"github.com/stretchr/testify/assert"
"testing"
"time"
)

func TestNewDefaultBackOffSettings(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
assert.Equal(t,
BackOffSettings{
Enabled: true,
InitialInterval: 5 * time.Second,
RandomizationFactor: 0.5,
Multiplier: 1.5,
MaxInterval: 30 * time.Second,
MaxElapsedTime: 5 * time.Minute,
}, cfg)
}

func TestInvalidInitialInterval(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
cfg.InitialInterval = -1
assert.Error(t, cfg.Validate())
}

func TestInvalidRandomizationFactor(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
cfg.RandomizationFactor = -1
assert.Error(t, cfg.Validate())
cfg.RandomizationFactor = 2
assert.Error(t, cfg.Validate())
}

func TestInvalidMultiplier(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
cfg.Multiplier = 0
assert.Error(t, cfg.Validate())
}

func TestInvalidMaxInterval(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
cfg.MaxInterval = -1
assert.Error(t, cfg.Validate())
}

func TestInvalidMaxElapsedTime(t *testing.T) {
cfg := NewDefaultBackOffSettings()
assert.NoError(t, cfg.Validate())
cfg.MaxElapsedTime = -1
assert.Error(t, cfg.Validate())
}

func TestDisabledWithInvalidValues(t *testing.T) {
cfg := BackOffSettings{
Enabled: false,
InitialInterval: -1,
RandomizationFactor: -1,
Multiplier: 0,
MaxInterval: -1,
MaxElapsedTime: -1,
}
assert.NoError(t, cfg.Validate())
}
17 changes: 17 additions & 0 deletions config/configretry/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module go.opentelemetry.io/collector/config/configretry

go 1.20

require (
github.com/cenkalti/backoff/v4 v4.2.1
github.com/stretchr/testify v1.8.4
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.10.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
25 changes: 25 additions & 0 deletions config/configretry/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions connector/forwardconnector/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,5 @@ retract (
)

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

replace go.opentelemetry.io/collector/config/configretry => ../../config/configretry
2 changes: 2 additions & 0 deletions connector/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,5 @@ replace go.opentelemetry.io/collector/processor => ../processor
replace go.opentelemetry.io/collector/receiver => ../receiver

replace go.opentelemetry.io/collector/exporter => ../exporter

replace go.opentelemetry.io/collector/config/configretry => ../config/configretry
2 changes: 2 additions & 0 deletions consumer/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ retract (
replace go.opentelemetry.io/collector/connector => ../connector

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

replace go.opentelemetry.io/collector/config/configretry => ../config/configretry
3 changes: 3 additions & 0 deletions exporter/debugexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/collector v0.91.0 // indirect
go.opentelemetry.io/collector/config/configretry v0.91.0 // indirect
go.opentelemetry.io/collector/consumer v0.91.0 // indirect
go.opentelemetry.io/collector/extension v0.91.0 // indirect
go.opentelemetry.io/collector/featuregate v1.0.0 // indirect
Expand Down Expand Up @@ -70,3 +71,5 @@ replace go.opentelemetry.io/collector/connector => ../../connector
replace go.opentelemetry.io/collector/processor => ../../processor

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

replace go.opentelemetry.io/collector/config/configretry => ../../config/configretry
7 changes: 4 additions & 3 deletions exporter/exporterhelper/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte

import (
"context"
"go.opentelemetry.io/collector/config/configretry"

"go.uber.org/multierr"
"go.uber.org/zap"
Expand Down Expand Up @@ -80,9 +81,9 @@ func WithTimeout(timeoutSettings TimeoutSettings) Option {
}
}

// WithRetry overrides the default RetrySettings for an exporter.
// The default RetrySettings is to disable retries.
func WithRetry(config RetrySettings) Option {
// WithRetry overrides the default configretry.BackOffSettings for an exporter.
// The default configretry.BackOffSettings is to disable retries.
func WithRetry(config configretry.BackOffSettings) Option {
return func(o *baseExporter) {
if !config.Enabled {
o.retrySender = &errorLoggingRequestSender{
Expand Down
7 changes: 4 additions & 3 deletions exporter/exporterhelper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configretry"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/exporter/exportertest"
)
Expand Down Expand Up @@ -73,20 +74,20 @@ func checkStatus(t *testing.T, sd sdktrace.ReadOnlySpan, err error) {

func TestQueueRetryOptionsWithRequestExporter(t *testing.T) {
bs, err := newBaseExporter(exportertest.NewNopCreateSettings(), "", true, nil, nil, newNoopObsrepSender,
WithRetry(NewDefaultRetrySettings()))
WithRetry(configretry.NewDefaultBackOffSettings()))
require.Nil(t, err)
require.True(t, bs.requestExporter)
require.Panics(t, func() {
_, _ = newBaseExporter(exportertest.NewNopCreateSettings(), "", true, nil, nil, newNoopObsrepSender,
WithRetry(NewDefaultRetrySettings()), WithQueue(NewDefaultQueueSettings()))
WithRetry(configretry.NewDefaultBackOffSettings()), WithQueue(NewDefaultQueueSettings()))
})
}

func TestBaseExporterLogging(t *testing.T) {
set := exportertest.NewNopCreateSettings()
logger, observed := observer.New(zap.DebugLevel)
set.Logger = zap.New(logger)
rCfg := NewDefaultRetrySettings()
rCfg := configretry.NewDefaultBackOffSettings()
rCfg.Enabled = false
bs, err := newBaseExporter(set, "", true, nil, nil, newNoopObsrepSender, WithRetry(rCfg))
require.Nil(t, err)
Expand Down
5 changes: 3 additions & 2 deletions exporter/exporterhelper/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configretry"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/consumer/consumertest"
Expand Down Expand Up @@ -159,7 +160,7 @@ func TestLogsExporter_WithPersistentQueue(t *testing.T) {
qCfg := NewDefaultQueueSettings()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID
rCfg := NewDefaultRetrySettings()
rCfg := configretry.NewDefaultBackOffSettings()
ts := consumertest.LogsSink{}
set := exportertest.NewNopCreateSettings()
set.ID = component.NewIDWithName("test_logs", "with_persistent_queue")
Expand Down Expand Up @@ -237,7 +238,7 @@ func TestLogsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := NewDefaultRetrySettings()
rCfg := configretry.NewDefaultBackOffSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
Expand Down
5 changes: 3 additions & 2 deletions exporter/exporterhelper/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configretry"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/consumer/consumertest"
Expand Down Expand Up @@ -160,7 +161,7 @@ func TestMetricsExporter_WithPersistentQueue(t *testing.T) {
qCfg := NewDefaultQueueSettings()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID
rCfg := NewDefaultRetrySettings()
rCfg := configretry.NewDefaultBackOffSettings()
ms := consumertest.MetricsSink{}
set := exportertest.NewNopCreateSettings()
set.ID = component.NewIDWithName("test_metrics", "with_persistent_queue")
Expand Down Expand Up @@ -239,7 +240,7 @@ func TestMetricsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := NewDefaultRetrySettings()
rCfg := configretry.NewDefaultBackOffSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
Expand Down
Loading

0 comments on commit ff1cec1

Please sign in to comment.