Skip to content

Commit

Permalink
[exporter/prometheusremotewrite] Fix exporter not respecting retrySet…
Browse files Browse the repository at this point in the history
…tings.enabled flag (open-telemetry#27592)

**Description:** Previously the remote write exporter would incorrectly
retry if `retrySettings.enabled` was set to false.

**Testing:** Unit tests
  • Loading branch information
bryan-aguilar authored and JaredTan95 committed Oct 18, 2023
1 parent dad51d5 commit e8113c2
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 12 deletions.
27 changes: 27 additions & 0 deletions .chloggen/prw_respectEnabledRetry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

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

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix remote write exporter not respecting retrySettings.enabled flag

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [27592]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
29 changes: 17 additions & 12 deletions exporter/prometheusremotewriteexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ func (prwe *prwExporter) export(ctx context.Context, requests []*prompb.WriteReq
}

func (prwe *prwExporter) execute(ctx context.Context, writeReq *prompb.WriteRequest) error {
// Retry function for backoff
retryFunc := func() error {
// executeFunc can be used for backoff and non backoff scenarios.
executeFunc := func() error {
// Uses proto.Marshal to convert the WriteRequest into bytes array
data, err := proto.Marshal(writeReq)
if err != nil {
Expand Down Expand Up @@ -264,16 +264,21 @@ func (prwe *prwExporter) execute(ctx context.Context, writeReq *prompb.WriteRequ
return backoff.Permanent(consumererror.NewPermanent(rerr))
}

// Use the BackOff instance to retry the func with exponential backoff.
err := backoff.Retry(retryFunc, &backoff.ExponentialBackOff{
InitialInterval: prwe.retrySettings.InitialInterval,
RandomizationFactor: prwe.retrySettings.RandomizationFactor,
Multiplier: prwe.retrySettings.Multiplier,
MaxInterval: prwe.retrySettings.MaxInterval,
MaxElapsedTime: prwe.retrySettings.MaxElapsedTime,
Stop: backoff.Stop,
Clock: backoff.SystemClock,
})
var err error
if prwe.retrySettings.Enabled {
// Use the BackOff instance to retry the func with exponential backoff.
err = backoff.Retry(executeFunc, &backoff.ExponentialBackOff{
InitialInterval: prwe.retrySettings.InitialInterval,
RandomizationFactor: prwe.retrySettings.RandomizationFactor,
Multiplier: prwe.retrySettings.Multiplier,
MaxInterval: prwe.retrySettings.MaxInterval,
MaxElapsedTime: prwe.retrySettings.MaxElapsedTime,
Stop: backoff.Stop,
Clock: backoff.SystemClock,
})
} else {
err = executeFunc()
}

if err != nil {
return consumererror.NewPermanent(err)
Expand Down
12 changes: 12 additions & 0 deletions exporter/prometheusremotewriteexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,12 @@ func runExportPipeline(ts *prompb.TimeSeries, endpoint *url.URL) error {
cfg := createDefaultConfig().(*Config)
cfg.HTTPClientSettings.Endpoint = endpoint.String()
cfg.RemoteWriteQueue.NumConsumers = 1
cfg.RetrySettings = exporterhelper.RetrySettings{
Enabled: true,
InitialInterval: 100 * time.Millisecond, // Shorter initial interval
MaxInterval: 1 * time.Second, // Shorter max interval
MaxElapsedTime: 2 * time.Second, // Shorter max elapsed time
}

buildInfo := component.BuildInfo{
Description: "OpenTelemetry Collector",
Expand Down Expand Up @@ -1011,6 +1017,9 @@ func TestRetryOn5xx(t *testing.T) {
exporter := &prwExporter{
endpointURL: endpointURL,
client: http.DefaultClient,
retrySettings: exporterhelper.RetrySettings{
Enabled: true,
},
}

ctx := context.Background()
Expand Down Expand Up @@ -1041,6 +1050,9 @@ func TestNoRetryOn4xx(t *testing.T) {
exporter := &prwExporter{
endpointURL: endpointURL,
client: http.DefaultClient,
retrySettings: exporterhelper.RetrySettings{
Enabled: true,
},
}

ctx := context.Background()
Expand Down

0 comments on commit e8113c2

Please sign in to comment.