Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exporterhelper: address timeout misalignment w/ context deadline #11198

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions exporter/exporterhelper/retry_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ func (rs *retrySender) send(ctx context.Context, req Request) error {
backoffDelay = max(backoffDelay, throttleErr.delay)
}

if deadline, has := ctx.Deadline(); has && time.Until(deadline) < backoffDelay {
// The delay is longer than the deadline. There is no point in
// waiting for cancelation.
return fmt.Errorf("request will be cancelled before next retry: %w", err)
}

backoffDelayStr := backoffDelay.String()
span.AddEvent(
"Exporting failed. Will retry the request after interval.",
Expand Down
40 changes: 37 additions & 3 deletions exporter/exporterhelper/timeout_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte
import (
"context"
"errors"
"fmt"
"time"
)

Expand All @@ -17,6 +18,15 @@ type TimeoutConfig struct {
// Timeout is the timeout for every attempt to send data to the backend.
// A zero timeout means no timeout.
Timeout time.Duration `mapstructure:"timeout"`

// OnShortTimeout is the exporter's disposition toward short timeouts,
// any time the incoming context's deadline is shorter than the
// configured Timeout. The choices are:
//
// - sustain: use the shorter-than-configured timeout
// - abort: abort the request for having insufficient deadline on arrival
// - ignore: ignore the previous deadline and proceed with the configured timeout
OnShortTimeout string `mapstructure:"on_short_timeout"`
}

func (ts *TimeoutConfig) Validate() error {
Expand All @@ -35,7 +45,8 @@ func NewDefaultTimeoutSettings() TimeoutSettings {
// NewDefaultTimeoutConfig returns the default config for TimeoutConfig.
func NewDefaultTimeoutConfig() TimeoutConfig {
return TimeoutConfig{
Timeout: 5 * time.Second,
Timeout: 5 * time.Second,
OnShortTimeout: "sustain",
}
}

Expand All @@ -50,8 +61,31 @@ func (ts *timeoutSender) send(ctx context.Context, req Request) error {
if ts.cfg.Timeout == 0 {
return req.Export(ctx)
}
// Intentionally don't overwrite the context inside the request, because in case of retries deadline will not be
// updated because this deadline most likely is before the next one.
// If there is deadline that will expire before the configured timeout,
// take one of several optional behaviors.
if deadline, has := ctx.Deadline(); has {
available := time.Until(deadline)
if available < ts.cfg.Timeout {
switch ts.cfg.OnShortTimeout {
case "ignore":
// The following call erases the
// deadline, a new one will be
// inserted below.
ctx = context.WithoutCancel(ctx)
case "abort":
// We should return a gRPC-or-HTTP
// response status saying the request
// was aborted.
return fmt.Errorf("Aborted: context deadline %v shorter than configured timeout %v", available, ts.cfg.Timeout)
case "sustain":
// Allow the shorter-than-configured
// timeout, means do nothing.
default:
// Default is sustain, the legacy behavior.
}
}
}

tCtx, cancelFunc := context.WithTimeout(ctx, ts.cfg.Timeout)
defer cancelFunc()
return req.Export(tCtx)
Expand Down