Skip to content

fix(sdk): improve force flush logic#5179

Open
grvmishra788 wants to merge 6 commits intoopen-telemetry:mainfrom
grvmishra788:worktree-issue-4631
Open

fix(sdk): improve force flush logic#5179
grvmishra788 wants to merge 6 commits intoopen-telemetry:mainfrom
grvmishra788:worktree-issue-4631

Conversation

@grvmishra788
Copy link
Copy Markdown

@grvmishra788 grvmishra788 commented May 5, 2026

Description

Fixes #4631

Before: SynchronousMultiSpanProcessor.force_flush and SynchronousMultiLogRecordProcessor.force_flush used if 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 returning None would cause all subsequent processors to be silently skipped. The concurrent variants had the same truthiness check on results, incorrectly treating None as failure.

After: All multi-processor force_flush implementations now:

  • Continue calling every registered processor regardless of individual return values (timeout-based early exit is preserved).
  • Use is False instead of not result to distinguish between an explicit False (timeout exceeded) and None (not implemented / nothing to flush).
  • SpanProcessor.force_flush returns True by default instead of implicitly returning None.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit tests (SDK)
  • Typecheck
  • Lint
  • Pre-commit
  • Spellcheck

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added

@grvmishra788 grvmishra788 marked this pull request as ready for review May 5, 2026 19:57
@grvmishra788 grvmishra788 requested a review from a team as a code owner May 5, 2026 19:57
@grvmishra788
Copy link
Copy Markdown
Author

Hi @alexmojaki! Could you please review this one? TIA.

Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @grvmishra788

@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest May 7, 2026
Copy link
Copy Markdown
Contributor

@alexmojaki alexmojaki left a comment

Choose a reason for hiding this comment

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

Thanks!

@aabmass
Copy link
Copy Markdown
Member

aabmass commented May 8, 2026

@DylanRussell can you take a look?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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() return True by default (instead of implicitly None).
  • Update synchronous and concurrent multi span/log processor force_flush() implementations to treat only an explicit False as failure and to continue invoking all processors.
  • Add unit tests covering None and False return 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

SynchronousMultiSpanProcessor.force_flush stops when a processor doesn't return True

5 participants