Deprecate exporterhelper.RetrySettings in favor of configretry.BackOf…

…fConfig

Signed-off-by: Bogdan Drutu <[email protected]>

.chloggen/deprecateretry.yaml

@@ -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.BackOffConfig"
+
+# One or more tracking issues or pull requests related to the change
+issues: [9091]

cmd/otelcorecol/builder-config.yaml

@@ -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

cmd/otelcorecol/go.mod

@@ -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
@@ -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

component/host.go

@@ -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

config/configgrpc/go.mod

@@ -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
@@ -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

config/confighttp/go.mod

@@ -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

config/configretry/Makefile

@@ -0,0 +1 @@
+include ../../Makefile.Common

config/configretry/backoff.go

@@ -0,0 +1,65 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package configretry
+
+import (
+	"errors"
+	"time"
+
+	"github.com/cenkalti/backoff/v4"
+)
+
+// NewDefaultBackOffConfig returns the default settings for RetrySettings.
+func NewDefaultBackOffConfig() BackOffConfig {
+	return BackOffConfig{
+		Enabled:             true,
+		InitialInterval:     5 * time.Second,
+		RandomizationFactor: backoff.DefaultRandomizationFactor,
+		Multiplier:          backoff.DefaultMultiplier,
+		MaxInterval:         30 * time.Second,
+		MaxElapsedTime:      5 * time.Minute,
+	}
+}
+
+// BackOffConfig defines configuration for retrying batches in case of export failure.
+// The current supported strategy is exponential backoff.
+type BackOffConfig 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 *BackOffConfig) 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
+}

config/configretry/backoff_test.go

@@ -0,0 +1,70 @@
+package configretry
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+	"time"
+)
+
+func TestNewDefaultBackOffSettings(t *testing.T) {
+	cfg := NewDefaultBackOffConfig()
+	assert.NoError(t, cfg.Validate())
+	assert.Equal(t,
+		BackOffConfig{
+			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 := NewDefaultBackOffConfig()
+	assert.NoError(t, cfg.Validate())
+	cfg.InitialInterval = -1
+	assert.Error(t, cfg.Validate())
+}
+
+func TestInvalidRandomizationFactor(t *testing.T) {
+	cfg := NewDefaultBackOffConfig()
+	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 := NewDefaultBackOffConfig()
+	assert.NoError(t, cfg.Validate())
+	cfg.Multiplier = 0
+	assert.Error(t, cfg.Validate())
+}
+
+func TestInvalidMaxInterval(t *testing.T) {
+	cfg := NewDefaultBackOffConfig()
+	assert.NoError(t, cfg.Validate())
+	cfg.MaxInterval = -1
+	assert.Error(t, cfg.Validate())
+}
+
+func TestInvalidMaxElapsedTime(t *testing.T) {
+	cfg := NewDefaultBackOffConfig()
+	assert.NoError(t, cfg.Validate())
+	cfg.MaxElapsedTime = -1
+	assert.Error(t, cfg.Validate())
+}
+
+func TestDisabledWithInvalidValues(t *testing.T) {
+	cfg := BackOffConfig{
+		Enabled:             false,
+		InitialInterval:     -1,
+		RandomizationFactor: -1,
+		Multiplier:          0,
+		MaxInterval:         -1,
+		MaxElapsedTime:      -1,
+	}
+	assert.NoError(t, cfg.Validate())
+}

config/configretry/go.mod

@@ -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
+)

connector/forwardconnector/go.mod

@@ -71,3 +71,5 @@ retract (
 )
 
 replace go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry
+
+replace go.opentelemetry.io/collector/config/configretry => ../../config/configretry

connector/go.mod

@@ -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

consumer/go.mod

@@ -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

exporter/debugexporter/go.mod

@@ -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
@@ -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

exporter/exporterhelper/common.go

@@ -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"
@@ -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.BackOffConfig for an exporter.
+// The default configretry.BackOffConfig is to disable retries.
+func WithRetry(config configretry.BackOffConfig) Option {
 	return func(o *baseExporter) {
 		if !config.Enabled {
 			o.retrySender = &errorLoggingRequestSender{

exporter/exporterhelper/common_test.go

@@ -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"
 )
@@ -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.NewDefaultBackOffConfig()))
 	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.NewDefaultBackOffConfig()), 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.NewDefaultBackOffConfig()
 	rCfg.Enabled = false
 	bs, err := newBaseExporter(set, "", true, nil, nil, newNoopObsrepSender, WithRetry(rCfg))
 	require.Nil(t, err)

exporter/exporterhelper/logs_test.go

@@ -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"
@@ -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.NewDefaultBackOffConfig()
 	ts := consumertest.LogsSink{}
 	set := exportertest.NewNopCreateSettings()
 	set.ID = component.NewIDWithName("test_logs", "with_persistent_queue")
@@ -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.NewDefaultBackOffConfig()
 	qCfg := NewDefaultQueueSettings()
 	qCfg.NumConsumers = 1
 	qCfg.QueueSize = 2

exporter/exporterhelper/metrics_test.go

@@ -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"
@@ -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.NewDefaultBackOffConfig()
 	ms := consumertest.MetricsSink{}
 	set := exportertest.NewNopCreateSettings()
 	set.ID = component.NewIDWithName("test_metrics", "with_persistent_queue")
@@ -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.NewDefaultBackOffConfig()
 	qCfg := NewDefaultQueueSettings()
 	qCfg.NumConsumers = 1
 	qCfg.QueueSize = 2