O11Y-5250: Add retry #194
O11Y-5250: Add retry #194
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
changliu-wk
requested review from
blakeroberts-wk and
evanweible-wf
as code owners
| } catch (e) { | ||
| _log.warning('Failed to export ${spans.length} spans. $e'); | ||
| } | ||
| await Future.delayed(Duration(seconds: retries)); |
There was a problem hiding this comment.
We should use exponential backoff. We should probably add jitter too, though there's implicit disagreement in the spec:
- https://opentelemetry.io/docs/specs/otel/protocol/exporter/#retry
- https://opentelemetry.io/docs/specs/otlp/#otlphttp-throttling
Both say exponential backoff, only one says jitter
There was a problem hiding this comment.
Added jitter in.
| } | ||
|
|
||
| Duration calculateJitteredDelay(int retries, Duration baseDelay) { | ||
| final random = Random(); |
There was a problem hiding this comment.
Can we make this a class member variable?
| final jitter = random.nextDouble() * baseDelay.inMilliseconds; | ||
| return Duration( | ||
| milliseconds: baseDelay.inMilliseconds + jitter.toInt() * retries); |
There was a problem hiding this comment.
We should use exponential delay, something like this:
| final jitter = random.nextDouble() * baseDelay.inMilliseconds; | |
| return Duration( | |
| milliseconds: baseDelay.inMilliseconds + jitter.toInt() * retries); | |
| final delay = baseDelay.inMilliseconds * pow(2, retries) | |
| final jitter = random.nextDouble() * delay; | |
| return Duration(milliseconds: (delay + jitter).toInt()); |
There was a problem hiding this comment.
This assumes the first retry will have "retries" set to zero, not one
| return; | ||
| } | ||
| // Exponential backoff with jitter | ||
| final delay = calculateJitteredDelay(retries, Duration(seconds: 1)); |
There was a problem hiding this comment.
I think it's reasonable to start with something smaller like 100 milliseconds
|
@Workiva/release-management-p |
|
QA +1 |
rmconsole-wf
added
RM Ready
Merge Requirements Met
and removed
Merge Requirements Unmet
labels
Which problem is this PR solving?
When call client.post collector_exporter didn't check if the response is succeeded or not, this PR would like to add retry effort when the response is retryable.
Short description of the change
How Has This Been Tested?
Please describe the tests that you ran to verify your change. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: