feat: implement retries #103
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frigus02
reviewed
Jul 25, 2025
Closed
frigus02
reviewed
Jul 26, 2025
alexbrt
force-pushed
the
main
branch
2 times, most recently
from
995ab29 to
6d833f0
Compare
|
Sorry for the slow replies. I was pretty swamped the last days. I'll try to get to this later today or latest on Wednesday. |
frigus02
reviewed
Aug 6, 2025
The Mutex is already acting as a box.
|
Added tests. I think this is good to merge now. I'll try to release a new version over the weekend |
frigus02
reviewed
Aug 9, 2025
frigus02
added a commit
that referenced
this pull request
Aug 9, 2025
Azure Application Insights frequently times out, returns HTTP 503
errors, or only accepts partial payloads. This adds transparent retries.
Users of this crate won't have to configure anything.
Using `backon` crate, because it seems small and easy to use.
Design decisions:
- Don't expose implementation. Add default retries that should work for
everyone:
- min_delay = 500ms
- max_delay = 5s
- with jitter (to avoid thundering herds when multiple exports run in parallel)
We wanted no max_times or total delay, because the BatchSpanProcessor
already provides a `max_export_timeout` option. This automatically
ensures that the exporter will stop. However, it's only available in the
experimental async version of the BatchSpanProcessor. And it's missing
in the SimpleSpanProcessor and metric and logs processors. Therefore
this sets a total delay of 35s.
- Add a `with_retry_notify` option. Can be used to log retries and
thereby debug exporter flakyness.
- Use `futures-timer` crate to provide sleep functionality during
retries. Ideally we could use
opentelemetry_sdk::runtime::Runtime::delay for that. But that's behind
an experimental feature and we the SDK doesn't provide the exporters
with a runtime. We'd have to add a configuration to the exporter where
users would need to specify their choosen runtime, which then has to
fit to the rest of their setup, otherwise they see runtime errors.
E.g. tokio-sleep fails if not executed in the context of a Tokio
reactor with:
there is no reactor running, must be called from the context of a Tokio 1.x runtime
This error would also only happen if a retry has to happen, which
means when a call to AppInsights fails. This would make it very hard
to debug.
We're using futures-timer instead, which works with all runtimes,
including tokio and futures-executor. The OpenTelemetry SDK uses the
latter for most default processors right now.
---------
Co-authored-by: Jan Kuehle <[email protected]>
|
Published with version 0.42.0. I hope I didn't remove anything you needed in the commits today. If I did, please let me know. And either way, thanks a lot for getting this started! This has been on my todo list for far too long. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Azure Application Insights frequently times out, returns HTTP 503 errors, or only accepts partial payloads. This PR adds support for configuring a
backon-based backoff strategy to automatically retry telemetry uploads in such cases.