Skip to content

Commit

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

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed Dec 13, 2023
1 parent defc1bb commit 29311e8
Show file tree
Hide file tree
Showing 47 changed files with 314 additions and 161 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.BackOffConfig"

# One or more tracking issues or pull requests related to the change
issues: [9091]
1 change: 1 addition & 0 deletions cmd/builder/test/core.builder.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ replaces:
- go.opentelemetry.io/collector/config/confighttp => ${WORKSPACE_DIR}/config/confighttp
- go.opentelemetry.io/collector/config/confignet => ${WORKSPACE_DIR}/config/confignet
- go.opentelemetry.io/collector/config/configopaque => ${WORKSPACE_DIR}/config/configopaque
- go.opentelemetry.io/collector/config/configretry => ${WORKSPACE_DIR}/config/configretry
- go.opentelemetry.io/collector/config/configtelemetry => ${WORKSPACE_DIR}/config/configtelemetry
- go.opentelemetry.io/collector/config/configtls => ${WORKSPACE_DIR}/config/configtls
- go.opentelemetry.io/collector/config/internal => ${WORKSPACE_DIR}/config/internal
Expand Down
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
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
65 changes: 65 additions & 0 deletions config/configretry/backoff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package configretry // import "go.opentelemetry.io/collector/config/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
}
74 changes: 74 additions & 0 deletions config/configretry/backoff_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package configretry

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
)

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())
}
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 @@ -10,6 +10,7 @@ import (
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configretry"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/exporter"
)
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.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{
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.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)
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.NewDefaultBackOffConfig()
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.NewDefaultBackOffConfig()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
Expand Down
Loading

0 comments on commit 29311e8

Please sign in to comment.