Skip to content

Conversation

@kovaszab
Copy link

@kovaszab kovaszab commented Nov 5, 2025

Changes

Adds configurability of force flushing timeout for batch processors (BatchSpanProcessor, BatchLogProcessor).

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@kovaszab kovaszab requested a review from a team as a code owner November 5, 2025 13:59
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

❌ Patch coverage is 49.01961% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.6%. Comparing base (cff5728) to head (2d11e07).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
opentelemetry-sdk/src/logs/batch_log_processor.rs 33.3% 4 Missing ⚠️
opentelemetry-sdk/src/logs/logger_provider.rs 55.5% 4 Missing ⚠️
opentelemetry-sdk/src/trace/export.rs 0.0% 4 Missing ⚠️
opentelemetry-sdk/src/trace/span_processor.rs 60.0% 4 Missing ⚠️
opentelemetry-sdk/src/logs/log_processor.rs 60.0% 2 Missing ⚠️
...y-sdk/src/logs/log_processor_with_async_runtime.rs 33.3% 2 Missing ⚠️
opentelemetry-sdk/src/logs/mod.rs 0.0% 2 Missing ⚠️
opentelemetry-proto/src/transform/logs.rs 0.0% 1 Missing ⚠️
...telemetry-sdk/src/logs/concurrent_log_processor.rs 0.0% 1 Missing ⚠️
opentelemetry-sdk/src/trace/mod.rs 0.0% 1 Missing ⚠️
... and 1 more
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

/// 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for shutdown, we allow users to pass timeout in each shutdown call itself. Could we stick with the same for flush also?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'll refactor the code to mirror the propagation of the provider's timeout like we do for shutdown_with_timeout.

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants