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

Uniform defaults for exporterhelper #4455

Open
1 of 3 tasks
mx-psi opened this issue Nov 18, 2021 · 1 comment
Open
1 of 3 tasks

Uniform defaults for exporterhelper #4455

mx-psi opened this issue Nov 18, 2021 · 1 comment
Labels
release:required-for-ga Must be resolved before GA release

Comments

@mx-psi
Copy link
Member

mx-psi commented Nov 18, 2021

The exporterhelper package provides certain reusable helpers for common needs like queueing, retrying with backoff or timeouts. As of e23c9d0 the timeout setting is enabled by default but the rest are not:

opts := &baseSettings{
TimeoutSettings: DefaultTimeoutSettings(),
// TODO: Enable queuing by default (call DefaultQueueSettings)
QueueSettings: QueueSettings{Enabled: false},
// TODO: Enable retry by default (call DefaultRetrySettings)
RetrySettings: RetrySettings{Enabled: false},
}

This issue is to address the TODOs in the code. The current status is a bit confusing, since the documentation does not state which settings are enabled by default and which are not.

The individual items are to

  • decide if timeout capabilities should be enabled by default and document if so,
  • decide if queue capabilities should be enabled by default and document if so and
  • decide if retry capabilities should be enabled by default and document if so.

I think timeout and queueing are safe to enable by default, while retries are a bit more doubtful: some of the exporters in contrib do several network requests per ConsumeTraces/Metrics/Logs call, so in situations of partial success (e.g. 1st request passes and the 2nd fails), it may not be safe or desirable to re-do previous requests. It may be fine to enable retries with a caveat on partial success situations.

@github-actions github-actions bot added the Stale label Nov 19, 2023
@atoulme atoulme added release:required-for-ga Must be resolved before GA release and removed Stale labels Dec 19, 2023
@dmitryax
Copy link
Member

This should wait for #6767 and #8122 to be resolved first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga Must be resolved before GA release
Projects
Status: Todo
Development

No branches or pull requests

4 participants