Skip to content

Commit

Permalink
Use default OTel Collector retry settings (vs zero values) (#741)
Browse files Browse the repository at this point in the history
**Description:** The default values for RetrySettings are problematic
because they have zeros. If a caller should enable the retry settings
w/o setting all fields, some of them are zero. We observed that a
Multiplier of zero is bad because it causes a tight loop to retry spans.
  • Loading branch information
jmacd authored Aug 2, 2024
1 parent e12d349 commit 6c1cf4b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## Unreleased

- Use correct default retry settings. [#741](https://github.com/lightstep/otel-launcher-go/pull/741)

## [1.30.0](https://github.com/lightstep/otel-launcher-go/releases/tag/v1.30.0) - 2024-07-17

- Update OpenTelemetry-Arrow components to latest, now in collector-contrib. [#736](https://github.com/lightstep/otel-launcher-go/pull/736)
Expand Down
14 changes: 7 additions & 7 deletions lightstep/sdk/metric/exporters/otlp/otelcol/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type client struct {
}

func NewDefaultConfig() Config {
return Config{
cfg := Config{
SelfMetrics: true,
SelfSpans: true,
Batcher: concurrentbatchprocessor.Config{
Expand All @@ -82,12 +82,8 @@ func NewDefaultConfig() Config {
TimeoutSettings: exporterhelper.TimeoutSettings{
Timeout: 15 * time.Second,
},
RetryConfig: configretry.BackOffConfig{
Enabled: false,
},
QueueSettings: exporterhelper.QueueSettings{
Enabled: false,
},
RetryConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),
ClientConfig: configgrpc.ClientConfig{
Headers: map[string]configopaque.String{},
Compression: configcompression.TypeZstd,
Expand All @@ -101,6 +97,10 @@ func NewDefaultConfig() Config {
},
},
}
// All RetryConfig and QueueSettings are taken from the defaults, but disabled.
cfg.Exporter.RetryConfig.Enabled = false
cfg.Exporter.QueueSettings.Enabled = false
return cfg
}

func NewConfig(opts ...Option) Config {
Expand Down
14 changes: 7 additions & 7 deletions lightstep/sdk/trace/exporters/otlp/otelcol/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (c *client) Shutdown(ctx context.Context) error {
}

func NewDefaultConfig() Config {
return Config{
cfg := Config{
SelfMetrics: true,
SelfSpans: true,
Batcher: concurrentbatchprocessor.Config{
Expand All @@ -143,12 +143,8 @@ func NewDefaultConfig() Config {
TimeoutSettings: exporterhelper.TimeoutSettings{
Timeout: 15 * time.Second,
},
RetryConfig: configretry.BackOffConfig{
Enabled: false,
},
QueueSettings: exporterhelper.QueueSettings{
Enabled: false,
},
RetryConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),
ClientConfig: configgrpc.ClientConfig{
Headers: map[string]configopaque.String{},
Compression: configcompression.TypeZstd,
Expand All @@ -162,6 +158,10 @@ func NewDefaultConfig() Config {
},
},
}
// All RetryConfig and QueueSettings are taken from the defaults, but disabled.
cfg.Exporter.RetryConfig.Enabled = false
cfg.Exporter.QueueSettings.Enabled = false
return cfg
}

func NewConfig(opts ...Option) Config {
Expand Down

0 comments on commit 6c1cf4b

Please sign in to comment.