fix(sdk): improve force flush logic#5179
fix(sdk): improve force flush logic#5179grvmishra788 wants to merge 6 commits intoopen-telemetry:mainfrom
Conversation
|
Hi @alexmojaki! Could you please review this one? TIA. |
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Looks good, thanks @grvmishra788
|
@DylanRussell can you take a look? |
There was a problem hiding this comment.
Pull request overview
Fixes an SDK bug where multi-processor force_flush() could incorrectly stop iterating when a processor returned None (or other falsy values), causing later processors to be skipped. This aligns behavior with the documented “only fail on explicit timeout/False” semantics and ensures all registered processors are invoked (unless the overall timeout is exceeded).
Changes:
- Make
SpanProcessor.force_flush()returnTrueby default (instead of implicitlyNone). - Update synchronous and concurrent multi span/log processor
force_flush()implementations to treat only an explicitFalseas failure and to continue invoking all processors. - Add unit tests covering
NoneandFalsereturn values for both synchronous and concurrent multi-processor variants; add a changelog entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| opentelemetry-sdk/src/opentelemetry/sdk/trace/init.py | Fixes multi span processor force_flush() logic to distinguish None vs explicit False, and makes base SpanProcessor.force_flush() return True. |
| opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/init.py | Fixes multi log record processor force_flush() logic to distinguish None vs explicit False. |
| opentelemetry-sdk/tests/trace/test_span_processor.py | Adds coverage ensuring None does not short-circuit and that all span processors are still invoked. |
| opentelemetry-sdk/tests/logs/test_multi_log_processor.py | Adds coverage for None/False return handling in synchronous and concurrent multi log record processors. |
| CHANGELOG.md | Documents the bug fix in the Unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Fixes #4631
Before:
SynchronousMultiSpanProcessor.force_flushandSynchronousMultiLogRecordProcessor.force_flushusedif not sp.force_flush(...): return False, which stopped iterating through processors as soon as any one returned a falsy value. This meant that a processor returningNonewould cause all subsequent processors to be silently skipped. The concurrent variants had the same truthiness check on results, incorrectly treatingNoneas failure.After: All multi-processor
force_flushimplementations now:is Falseinstead ofnot resultto distinguish between an explicitFalse(timeout exceeded) andNone(not implemented / nothing to flush).SpanProcessor.force_flushreturnsTrueby default instead of implicitly returningNone.Type of change
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: