Skip to content

Commit

Permalink
Updating methods to reflect the contribution guide's recommendations
Browse files Browse the repository at this point in the history
  • Loading branch information
MovieStoreGuy committed Feb 18, 2022
1 parent 2a2a722 commit 9b07d5f
Show file tree
Hide file tree
Showing 22 changed files with 94 additions and 67 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### 🛑 Breaking changes 🛑

- Deprecated methods `config.DefaultConfig`, `confighttp.DefaultHTTPSettings`, `exporterhelper.DefaultTimeoutSettings`,
`exporthelper.DefaultQueueSettings`, `exporterhelper.DefaultRetrySettings`, `testcomponents.DefaultFactories`, and
`scraperhelper.DefaultScraperControllerSettings` in favour for their `NewDefault` method to adhere to contribution guidelines (#4865)

### 💡 Enhancements 💡

- Add validation to check at least one endpoint is specified in otlphttpexporter's configuration (#4860)
Expand Down
7 changes: 5 additions & 2 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ type Module struct {
Path string `mapstructure:"path"` // an optional path to the local version of this module
}

// DefaultConfig creates a new config, with default values
func DefaultConfig() Config {
// Deprecated: [v0.46.0] Use NewDefaultConfig instead
var DefaultConfig = NewDefaultConfig

// NewDefaultConfig creates a new config, with default values
func NewDefaultConfig() Config {
log, err := zap.NewDevelopment()
if err != nil {
panic(fmt.Sprintf("failed to obtain a logger instance: %v", err))
Expand Down
2 changes: 1 addition & 1 deletion cmd/builder/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

var (
cfgFile string
cfg = builder.DefaultConfig()
cfg = builder.NewDefaultConfig()
)

// Command is the main entrypoint for this application
Expand Down
7 changes: 5 additions & 2 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,14 @@ type HTTPClientSettings struct {
IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"`
}

// DefaultHTTPClientSettings returns HTTPClientSettings type object with
// Deprecated: [v0.46.0] Use NewDefaultHTTPClientSettings instead.
var DefaultHTTPClientSettings = NewDefaultHTTPClientSettings

// NewDefaultHTTPClientSettings returns HTTPClientSettings type object with
// the default values of 'MaxIdleConns' and 'IdleConnTimeout'.
// Other config options are not added as they are initialized with 'zero value' by GoLang as default.
// We encourage to use this function to create an object of HTTPClientSettings.
func DefaultHTTPClientSettings() HTTPClientSettings {
func NewDefaultHTTPClientSettings() HTTPClientSettings {
// The default values are taken from the values of 'DefaultTransport' of 'http' package.
maxIdleConns := 100
idleConnTimeout := 90 * time.Second
Expand Down
2 changes: 1 addition & 1 deletion config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestPartialHTTPClientSettings(t *testing.T) {
}

func TestDefaultHTTPClientSettings(t *testing.T) {
httpClientSettings := DefaultHTTPClientSettings()
httpClientSettings := NewDefaultHTTPClientSettings()
assert.EqualValues(t, 100, *httpClientSettings.MaxIdleConns)
assert.EqualValues(t, 90*time.Second, *httpClientSettings.IdleConnTimeout)
}
Expand Down
9 changes: 6 additions & 3 deletions exporter/exporterhelper/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ type TimeoutSettings struct {
Timeout time.Duration `mapstructure:"timeout"`
}

// DefaultTimeoutSettings returns the default settings for TimeoutSettings.
func DefaultTimeoutSettings() TimeoutSettings {
// Deprecated: [v0.46.0] use NewDefaultTimeoutSettings instead.
var DefaultTimeoutSettings = NewDefaultTimeoutSettings

// NewDefaultTimeoutSettings returns the default settings for TimeoutSettings.
func NewDefaultTimeoutSettings() TimeoutSettings {
return TimeoutSettings{
Timeout: 5 * time.Second,
}
Expand Down Expand Up @@ -100,7 +103,7 @@ type baseSettings struct {
func fromOptions(options ...Option) *baseSettings {
// Start from the default options:
opts := &baseSettings{
TimeoutSettings: DefaultTimeoutSettings(),
TimeoutSettings: NewDefaultTimeoutSettings(),
// TODO: Enable queuing by default (call DefaultQueueSettings)
QueueSettings: QueueSettings{Enabled: false},
// TODO: Enable retry by default (call DefaultRetrySettings)
Expand Down
2 changes: 1 addition & 1 deletion exporter/exporterhelper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestBaseExporterWithOptions(t *testing.T) {
fromOptions(
WithStart(func(ctx context.Context, host component.Host) error { return want }),
WithShutdown(func(ctx context.Context) error { return want }),
WithTimeout(DefaultTimeoutSettings())),
WithTimeout(NewDefaultTimeoutSettings())),
"",
nopRequestUnmarshaler(),
)
Expand Down
4 changes: 2 additions & 2 deletions exporter/exporterhelper/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ func TestLogsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := DefaultRetrySettings()
qCfg := DefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
wantErr := errors.New("some-error")
Expand Down
4 changes: 2 additions & 2 deletions exporter/exporterhelper/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ func TestMetricsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := DefaultRetrySettings()
qCfg := DefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
wantErr := errors.New("some-error")
Expand Down
7 changes: 5 additions & 2 deletions exporter/exporterhelper/queued_retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ type RetrySettings struct {
MaxElapsedTime time.Duration `mapstructure:"max_elapsed_time"`
}

// DefaultRetrySettings returns the default settings for RetrySettings.
func DefaultRetrySettings() RetrySettings {
// Deprecated: [v0.46.0] use NewDefaultRetrySettings instead.
var DefaultRetrySettings = NewDefaultRetrySettings

// NewDefaultRetrySettings returns the default settings for RetrySettings.
func NewDefaultRetrySettings() RetrySettings {
return RetrySettings{
Enabled: true,
InitialInterval: 5 * time.Second,
Expand Down
7 changes: 5 additions & 2 deletions exporter/exporterhelper/queued_retry_experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ type QueueSettings struct {
PersistentStorageEnabled bool `mapstructure:"persistent_storage_enabled"`
}

// DefaultQueueSettings returns the default settings for QueueSettings.
func DefaultQueueSettings() QueueSettings {
// Deprecated: [v0.46.0] use NewDefaultQueueSettings instead.
var DefaultQueueSettings = NewDefaultQueueSettings

// NewDefaultQueueSettings returns the default settings for QueueSettings.
func NewDefaultQueueSettings() QueueSettings {
return QueueSettings{
Enabled: true,
NumConsumers: 10,
Expand Down
7 changes: 5 additions & 2 deletions exporter/exporterhelper/queued_retry_inmemory.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ type QueueSettings struct {
QueueSize int `mapstructure:"queue_size"`
}

// DefaultQueueSettings returns the default settings for QueueSettings.
func DefaultQueueSettings() QueueSettings {
// Deprecated: [v0.46.0] use NewDefaultQueueSettings instead.
var DefaultQueueSettings = NewDefaultQueueSettings

// NewDefaultQueueSettings returns the default settings for QueueSettings.
func NewDefaultQueueSettings() QueueSettings {
return QueueSettings{
Enabled: true,
NumConsumers: 10,
Expand Down
46 changes: 23 additions & 23 deletions exporter/exporterhelper/queued_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func mockRequestUnmarshaler(mr *mockRequest) internal.RequestUnmarshaler {
}

func TestQueuedRetry_DropOnPermanentError(t *testing.T) {
qCfg := DefaultQueueSettings()
rCfg := DefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
mockR := newMockRequest(context.Background(), 2, consumererror.NewPermanent(errors.New("bad data")))
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", mockRequestUnmarshaler(mockR))
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
Expand All @@ -68,8 +68,8 @@ func TestQueuedRetry_DropOnPermanentError(t *testing.T) {
}

func TestQueuedRetry_DropOnNoRetry(t *testing.T) {
qCfg := DefaultQueueSettings()
rCfg := DefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
rCfg.Enabled = false
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
Expand All @@ -92,9 +92,9 @@ func TestQueuedRetry_DropOnNoRetry(t *testing.T) {
}

func TestQueuedRetry_OnError(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
rCfg.InitialInterval = 0
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
Expand All @@ -119,9 +119,9 @@ func TestQueuedRetry_OnError(t *testing.T) {
}

func TestQueuedRetry_StopWhileWaiting(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
be.qrSender.consumerSender = ocs
Expand Down Expand Up @@ -152,9 +152,9 @@ func TestQueuedRetry_StopWhileWaiting(t *testing.T) {
}

func TestQueuedRetry_DoNotPreserveCancellation(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
be.qrSender.consumerSender = ocs
Expand All @@ -179,9 +179,9 @@ func TestQueuedRetry_DoNotPreserveCancellation(t *testing.T) {
}

func TestQueuedRetry_MaxElapsedTime(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
rCfg.InitialInterval = time.Millisecond
rCfg.MaxElapsedTime = 100 * time.Millisecond
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
Expand Down Expand Up @@ -226,9 +226,9 @@ func (e wrappedError) Unwrap() error {
}

func TestQueuedRetry_ThrottleError(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
rCfg.InitialInterval = 10 * time.Millisecond
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
Expand Down Expand Up @@ -257,10 +257,10 @@ func TestQueuedRetry_ThrottleError(t *testing.T) {
}

func TestQueuedRetry_RetryOnError(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 1
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
rCfg.InitialInterval = 0
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
Expand All @@ -285,9 +285,9 @@ func TestQueuedRetry_RetryOnError(t *testing.T) {
}

func TestQueuedRetry_DropOnFull(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.QueueSize = 0
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
be.qrSender.consumerSender = ocs
Expand All @@ -304,8 +304,8 @@ func TestQueuedRetryHappyPath(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

qCfg := DefaultQueueSettings()
rCfg := DefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
be := newBaseExporter(&defaultExporterCfg, tt.ToExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
ocs := newObservabilityConsumerSender(be.qrSender.consumerSender)
be.qrSender.consumerSender = ocs
Expand Down Expand Up @@ -337,9 +337,9 @@ func TestQueuedRetryHappyPath(t *testing.T) {
}

func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 0 // to make every request go straight to the queue
rCfg := DefaultRetrySettings()
rCfg := NewDefaultRetrySettings()
be := newBaseExporter(&defaultExporterCfg, componenttest.NewNopExporterCreateSettings(), fromOptions(WithRetry(rCfg), WithQueue(qCfg)), "", nopRequestUnmarshaler())
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))

Expand Down Expand Up @@ -369,7 +369,7 @@ func TestNoCancellationContext(t *testing.T) {
}

func TestQueueSettings_Validate(t *testing.T) {
qCfg := DefaultQueueSettings()
qCfg := NewDefaultQueueSettings()
assert.NoError(t, qCfg.Validate())

qCfg.QueueSize = 0
Expand Down
4 changes: 2 additions & 2 deletions exporter/exporterhelper/traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ func TestTracesExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := DefaultRetrySettings()
qCfg := DefaultQueueSettings()
rCfg := NewDefaultRetrySettings()
qCfg := NewDefaultQueueSettings()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
wantErr := errors.New("some-error")
Expand Down
6 changes: 3 additions & 3 deletions exporter/otlpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ func NewFactory() component.ExporterFactory {
func createDefaultConfig() config.Exporter {
return &Config{
ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
TimeoutSettings: exporterhelper.DefaultTimeoutSettings(),
RetrySettings: exporterhelper.DefaultRetrySettings(),
QueueSettings: exporterhelper.DefaultQueueSettings(),
TimeoutSettings: exporterhelper.NewDefaultTimeoutSettings(),
RetrySettings: exporterhelper.NewDefaultRetrySettings(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),
GRPCClientSettings: configgrpc.GRPCClientSettings{
Headers: map[string]string{},
// Default to gzip compression
Expand Down
6 changes: 3 additions & 3 deletions exporter/otlpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ func TestCreateDefaultConfig(t *testing.T) {
assert.NoError(t, configtest.CheckConfigStruct(cfg))
ocfg, ok := factory.CreateDefaultConfig().(*Config)
assert.True(t, ok)
assert.Equal(t, ocfg.RetrySettings, exporterhelper.DefaultRetrySettings())
assert.Equal(t, ocfg.QueueSettings, exporterhelper.DefaultQueueSettings())
assert.Equal(t, ocfg.TimeoutSettings, exporterhelper.DefaultTimeoutSettings())
assert.Equal(t, ocfg.RetrySettings, exporterhelper.NewDefaultRetrySettings())
assert.Equal(t, ocfg.QueueSettings, exporterhelper.NewDefaultQueueSettings())
assert.Equal(t, ocfg.TimeoutSettings, exporterhelper.NewDefaultTimeoutSettings())
assert.Equal(t, ocfg.Compression, configcompression.Gzip)
}

Expand Down
4 changes: 2 additions & 2 deletions exporter/otlphttpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func NewFactory() component.ExporterFactory {
func createDefaultConfig() config.Exporter {
return &Config{
ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
RetrySettings: exporterhelper.DefaultRetrySettings(),
QueueSettings: exporterhelper.DefaultQueueSettings(),
RetrySettings: exporterhelper.NewDefaultRetrySettings(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "",
Timeout: 30 * time.Second,
Expand Down
10 changes: 5 additions & 5 deletions internal/testcomponents/example_factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ func ExampleComponents() (
return
}

// DefaultFactories returns the set of components in "testdata/otelcol-config.yaml". This is only used by tests.
func DefaultFactories() (
component.Factories,
error,
) {
// Deprecated: [v0.46.0] use NewDefaultFactories instead.
var DefaultFactories = NewDefaultFactories

// NewDefaultFactories returns the set of components in "testdata/otelcol-config.yaml". This is only used by tests.
func NewDefaultFactories() (component.Factories, error) {
var errs error

extensions, err := component.MakeExtensionFactoryMap(
Expand Down
7 changes: 5 additions & 2 deletions receiver/scraperhelper/scrapercontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ type ScraperControllerSettings struct {
CollectionInterval time.Duration `mapstructure:"collection_interval"`
}

// DefaultScraperControllerSettings returns default scraper controller
// Deprecated: [v0.46.0] use NewDefaultScraperControllerSettings instead.
var DefaultScraperControllerSettings = NewDefaultScraperControllerSettings

// NewDefaultScraperControllerSettings returns default scraper controller
// settings with a collection interval of one minute.
func DefaultScraperControllerSettings(cfgType config.Type) ScraperControllerSettings {
func NewDefaultScraperControllerSettings(cfgType config.Type) ScraperControllerSettings {
return ScraperControllerSettings{
ReceiverSettings: config.NewReceiverSettings(config.NewComponentID(cfgType)),
CollectionInterval: time.Minute,
Expand Down
4 changes: 2 additions & 2 deletions receiver/scraperhelper/scrapercontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestScrapeController(t *testing.T) {
if !test.nilNextConsumer {
nextConsumer = sink
}
defaultCfg := DefaultScraperControllerSettings("receiver")
defaultCfg := NewDefaultScraperControllerSettings("receiver")
cfg := &defaultCfg
if test.scraperControllerSettings != nil {
cfg = test.scraperControllerSettings
Expand Down Expand Up @@ -332,7 +332,7 @@ func TestSingleScrapePerTick(t *testing.T) {
scrapeMetricsCh := make(chan int, 10)
tsm := &testScrapeMetrics{ch: scrapeMetricsCh}

defaultCfg := DefaultScraperControllerSettings("")
defaultCfg := NewDefaultScraperControllerSettings("")
cfg := &defaultCfg

tickerCh := make(chan time.Time)
Expand Down
Loading

0 comments on commit 9b07d5f

Please sign in to comment.