Skip to content

Commit

Permalink
OTLP exporter: Let final retry error include last retryable error mes…
Browse files Browse the repository at this point in the history
…sage (#3514)

* Let retry return the first retryable error

* examples/otel-collector: use default collector port

* use correct port

* revert example change

* tidy

* changelog

* lint

* Address comments in PR.

* Updated Changelog.

* Fixes for PR.

* merge changelog

* remove one error

Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
Co-authored-by: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
4 people authored Jan 4, 2023
1 parent 4607516 commit efd8a7d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `traceIDRatioSampler` (given by `TraceIDRatioBased(float64)`) now uses the rightmost bits for sampling decisions,
fixing random sampling when using ID generators like `xray.IDGenerator`
and increasing parity with other language implementations. (#3557)
- The OTLP exporter for traces and metrics will print the final retryable error message when attempts to retry time out. (#3514)
- The instrument kind names in `go.opentelemetry.io/otel/sdk/metric` are updated to match the API. (#3562)
- `InstrumentKindSyncCounter` is renamed to `InstrumentKindCounter`
- `InstrumentKindSyncUpDownCounter` is renamed to `InstrumentKindUpDownCounter`
Expand Down
7 changes: 5 additions & 2 deletions exporters/otlp/internal/retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func (c Config) RequestFunc(evaluate EvaluateFunc) RequestFunc {
delay = throttle
}

if err := waitFunc(ctx, delay); err != nil {
return err
if ctxErr := waitFunc(ctx, delay); ctxErr != nil {
return fmt.Errorf("%w: %s", ctxErr, err)
}
}
}
Expand All @@ -129,6 +129,9 @@ func (c Config) RequestFunc(evaluate EvaluateFunc) RequestFunc {
// Allow override for testing.
var waitFunc = wait

// wait takes the caller's context, and the amount of time to wait. It will
// return nil if the timer fires before or at the same time as the context's
// deadline. This indicates that the call can be retried.
func wait(ctx context.Context, delay time.Duration) error {
timer := time.NewTimer(delay)
defer timer.Stop()
Expand Down
49 changes: 38 additions & 11 deletions exporters/otlp/internal/retry/retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,16 @@ func TestWait(t *testing.T) {
expected error
}{
{
ctx: context.Background(),
delay: time.Duration(0),
expected: nil,
ctx: context.Background(),
delay: time.Duration(0),
},
{
ctx: context.Background(),
delay: time.Duration(1),
expected: nil,
ctx: context.Background(),
delay: time.Duration(1),
},
{
ctx: context.Background(),
delay: time.Duration(-1),
expected: nil,
ctx: context.Background(),
delay: time.Duration(-1),
},
{
ctx: func() context.Context {
Expand All @@ -59,7 +56,12 @@ func TestWait(t *testing.T) {
}

for _, test := range tests {
assert.Equal(t, test.expected, wait(test.ctx, test.delay))
err := wait(test.ctx, test.delay)
if test.expected == nil {
assert.NoError(t, err)
} else {
assert.ErrorIs(t, err, test.expected)
}
}
}

Expand Down Expand Up @@ -133,7 +135,7 @@ func TestBackoffRetry(t *testing.T) {
origWait := waitFunc
var done bool
waitFunc = func(_ context.Context, d time.Duration) error {
delta := math.Ceil(float64(delay)*backoff.DefaultRandomizationFactor) - float64(delay)
delta := math.Ceil(float64(delay) * backoff.DefaultRandomizationFactor)
assert.InDelta(t, delay, d, delta, "retry not backoffed")
// Try twice to ensure call is attempted again after delay.
if done {
Expand All @@ -150,6 +152,31 @@ func TestBackoffRetry(t *testing.T) {
}), assert.AnError)
}

func TestBackoffRetryCanceledContext(t *testing.T) {
ev := func(error) (bool, time.Duration) { return true, 0 }

delay := time.Millisecond
reqFunc := Config{
Enabled: true,
InitialInterval: delay,
MaxInterval: delay,
// Never stop retrying.
MaxElapsedTime: 10 * time.Millisecond,
}.RequestFunc(ev)

ctx, cancel := context.WithCancel(context.Background())
count := 0
cancel()
err := reqFunc(ctx, func(context.Context) error {
count++
return assert.AnError
})

assert.ErrorIs(t, err, context.Canceled)
assert.Contains(t, err.Error(), assert.AnError.Error())
assert.Equal(t, 1, count)
}

func TestThrottledRetryGreaterThanMaxElapsedTime(t *testing.T) {
// Ensure the throttle delay is used by making longer than backoff delay.
tDelay, bDelay := time.Hour, time.Nanosecond
Expand Down

0 comments on commit efd8a7d

Please sign in to comment.