feat: add forceflush_timeout configurability#3223
feat: add forceflush_timeout configurability#3223kovaszab wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3223 +/- ##
=======================================
- Coverage 80.8% 80.6% -0.2%
=======================================
Files 128 128
Lines 23288 23204 -84
=======================================
- Hits 18821 18725 -96
- Misses 4467 4479 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| /// Corresponding environment variable: `OTEL_BLRP_FORCE_FLUSH_TIMEOUT`. | ||
| /// | ||
| /// Note: Programmatically setting this will override any value set via the environment variable. | ||
| pub fn with_forceflush_timeout(mut self, forceflush_timeout: Duration) -> Self { |
There was a problem hiding this comment.
for shutdown, we allow users to pass timeout in each shutdown call itself. Could we stick with the same for flush also?
There was a problem hiding this comment.
Its best to be consistent with shutdown, if at all we are adding timeout to flush.
i.e Add this all the way from provider itself.
There was a problem hiding this comment.
You're right, I'll refactor the code to mirror the propagation of the provider's timeout like we do for shutdown_with_timeout.
cijothomas
left a comment
There was a problem hiding this comment.
See https://github.com/open-telemetry/opentelemetry-rust/pull/3223/files#r2495071383
Want to see if there is a reason for this be done differently than Shutdown timeouts.
Changes
Adds configurability of force flushing timeout for batch processors (BatchSpanProcessor, BatchLogProcessor).
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes