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] Do not re-enqueue failed requests #9090

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Dec 12, 2023

The current re-enqueuing behavior is not obvious and cannot be configured. It takes place only for persistent queue and only if retry_on_failure::enabled=true even if retry_on_failure is a setting for a different backoff retry strategy.

This change removes the re-enqueuing behavior in favor of the retry_on_failure option. Consider increasing retry_on_failure::max_elapsed_time to reduce chances of data loss.

Resolves #8382

@dmitryax dmitryax requested review from a team and bogdandrutu December 12, 2023 19:11
@dmitryax dmitryax force-pushed the remove-reqeueing branch 5 times, most recently from dcf9229 to ed43c53 Compare December 12, 2023 19:22
@dmitryax
Copy link
Member Author

@swiatekm-sumo, PTAL

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (9d2e43d) 91.52% compared to head (46a11f8) 91.49%.

Files Patch % Lines
exporter/exporterhelper/internal/err.go 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9090      +/-   ##
==========================================
- Coverage   91.52%   91.49%   -0.03%     
==========================================
  Files         319      320       +1     
  Lines       17218    17194      -24     
==========================================
- Hits        15758    15731      -27     
- Misses       1163     1166       +3     
  Partials      297      297              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The current re-enqueuing behavior is not obvious and cannot be configured. It takes place only for persistent queue and only if `retry_on_failure::enabled=true` even if `retry_on_failure` is a setting for a different backoff retry strategy. This change removes the re-enqueuing behavior. Consider increasing `retry_on_failure::max_elapsed_time` to reduce chances of data loss.
@bogdandrutu bogdandrutu merged commit ef4a5be into open-telemetry:main Dec 15, 2023
31 of 32 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 15, 2023
@dmitryax dmitryax deleted the remove-reqeueing branch December 15, 2023 05:32
sokoide pushed a commit to sokoide/opentelemetry-collector that referenced this pull request Dec 18, 2023
The current re-enqueuing behavior is not obvious and cannot be
configured. It takes place only for persistent queue and only if
`retry_on_failure::enabled=true` even if `retry_on_failure` is a setting
for a different backoff retry strategy.

This change removes the re-enqueuing behavior in favor of the
`retry_on_failure` option. Consider increasing
`retry_on_failure::max_elapsed_time` to reduce chances of data loss.

Resolves
open-telemetry#8382
carsonip added a commit to carsonip/opentelemetry-collector that referenced this pull request Mar 7, 2024
Requeuing was removed in open-telemetry#9090. Update docs accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reenqueue may not be needed
3 participants