fix(sdk): catch and log unhandled exceptions in MultiSpanProcessor

[somuai]

Description

Fixes #5624

Per the OpenTelemetry specification:

• OnEnd(Span): "This method MUST be called synchronously within the Span.End() API, therefore it should not block or throw an exception."
• OnStart(Span, Context): "This method MUST be called synchronously within the Tracer.StartSpan API, therefore it should not block or throw an exception."
• Error handling: "OpenTelemetry implementations MUST NOT throw unhandled exceptions at runtime. API methods that accept external callbacks MUST handle all errors."

Previously, SynchronousMultiSpanProcessor and ConcurrentMultiSpanProcessor did not catch exceptions raised by underlying SpanProcessor instances during on_start, _on_ending, on_end, shutdown, and force_flush.

When a span processor raised in on_end during context manager exit (Span.__exit__), the exception escaped and replaced or suppressed the application's actual exception. Furthermore, a failure in an earlier processor in the list prevented subsequent processors from being called.

This change wraps calls to underlying processors in try...except Exception: blocks and logs failures via logger.exception(...), ensuring that:

1. Application exceptions are never replaced or suppressed by processor failures.
2. All registered span processors are invoked even if one throws.
3. Thread pool executor submission failures (such as during interpreter shutdown) are handled gracefully.

Type of change

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

How Has This Been Tested?

• Added test_on_end_exception_does_not_raise to MultiSpanProcessorTestBase (runs against both Synchronous and Concurrent processors).
• Added test_on_start_exception_does_not_raise to MultiSpanProcessorTestBase.
• Added test_on_ending_exception_does_not_raise to MultiSpanProcessorTestBase.
• Added test_shutdown_exception_does_not_raise to MultiSpanProcessorTestBase.
• Added test_on_end_exception_does_not_replace_application_exception verifying that application exceptions inside with tracer.start_as_current_span(...) are properly propagated and never replaced.
• Verified all 35 tests in opentelemetry-sdk/tests/trace/test_span_processor.py pass.
• Verified ruff check passes with 0 errors.

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
• Documentation has been updated

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on the author · refreshed 2026-10-20 04:41 UTC

Respond to 3 review items (e.g. link a commit, explain why not, ask a follow-up):

• Inline threads: 1, 2
• Top-level threads: 3

Status above doesn't look right?

• Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
• Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
• Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

[chrikrah]

I read this against the issue and ran the repro on main first, so the notes below are from the code
rather than from the description. The shape looks right to me: guarding every callback rather than
only on_end covers the case the issue's own suggested diff misses, where a ConcurrentMultiSpanProcessor
is installed at the top level and the exception escapes from future.result().

Two things I could not find a test for, both behaviour this pull request changes:

1. force_flush. A processor that raises now sets all_flushed = False and the method returns False
   instead of propagating. The other four callbacks each got a test; this one did not, and it is the
   only one of the five with a return value a caller acts on.
2. The submit side of _submit_and_await. If self._executor.submit raises, the processor is dropped
   from futures and the call continues. That is reachable at interpreter exit, where
   concurrent.futures.thread shuts its executor down in its own atexit hook before
   TracerProvider.shutdown runs, and submit then raises RuntimeError: cannot schedule new futures after shutdown.

One question rather than a request. force_flush reports failure through its return value, but
shutdown() returns nothing, so a processor that fails to shut down is now invisible to the caller.
Given the issue is about failures that happen quietly, is that asymmetry deliberate?

One thing worth stating because a reviewer might otherwise ask: the new tests sit on
MultiSpanProcessorTestBase, so both TestSynchronousMultiSpanProcessor and
TestConcurrentMultiSpanProcessor inherit them. That matters here, because the two classes fail
differently before the fix. The synchronous loop stops at the first raiser, so later processors never
run, while the concurrent one submits every future before awaiting any and only loses the results
after the first failure.

[somuai]

Thank you @chrikrah for the detailed and thorough review!

I have addressed each of the points in commit 8c3371e:

1. force_flush exception coverage: Added test_force_flush_exception_does_not_raise_and_returns_false to MultiSpanProcessorTestBase. It validates that when an underlying processor's force_flush raises an exception, the multi-processor does not propagate the exception, logs it, continues invoking subsequent processors, and returns False. Because it sits on the base test class, both TestSynchronousMultiSpanProcessor and TestConcurrentMultiSpanProcessor inherit and execute it. Additionally, updated ConcurrentMultiSpanProcessor.force_flush to ensure all_flushed = False if self._executor.submit fails during submission.
2. Submit side of _submit_and_await: Added test_executor_submit_exception_does_not_raise to TestConcurrentMultiSpanProcessor. It explicitly shuts down the underlying executor to simulate interpreter exit / post-shutdown states and validates that on_start, _on_ending, on_end, shutdown, and force_flush catch the resulting RuntimeError("cannot schedule new futures after shutdown") gracefully without raising, with force_flush returning False.
3. force_flush vs shutdown return asymmetry: The asymmetry is intentional and stems from the OpenTelemetry Specification:
   • The OpenTelemetry Trace SDK specification defines SpanProcessor.shutdown() with a void / None return type. Altering it to return a boolean would break interface parity with SpanProcessor and third-party processor implementations.
   • Conversely, the specification explicitly specifies SpanProcessor.force_flush([timeoutMillis]) as returning a boolean indicating success.
   • Catching and logging the exception with logger.exception in shutdown() ensures the failure is captured in diagnostics without deviating from the SDK interface specification.

[chrikrah]

Both land. Checked out 8c3371e and reverted only sdk/trace/__init__.py to its parent, keeping the tests:

with the change      3 passed
production reverted  1 failed, 2 passed

The one that fails is test_executor_submit_exception_does_not_raise, on RuntimeError('cannot schedule new futures after shutdown'), so the new all_flushed = False is covered.

test_force_flush_exception_does_not_raise_and_returns_false passes either way, in both subclasses. That path already swallowed a raising force_flush, so it guards existing behaviour rather than covering this diff. Both are worth keeping. Only test_executor_submit_exception_does_not_raise covers the change.

[somuai]

Gentle check-in on this PR — reviewer @chrikrah verified that the tests in commit 8c3371e land cleanly and properly cover the executor shutdown exception path. All CI checks and EasyCLA are green with zero merge conflicts. Friendly follow-up for the maintainers whenever convenient to take a final look or queue for merge. Thank you!

[armanunix]

Some real-world impact data in support of this fix, from testing GenAI instrumentations against a span processor that raises:

from opentelemetry.sdk.trace import TracerProvider, SpanProcessor

class BuggyProcessor(SpanProcessor):
    def on_start(self, span, parent_context=None):
        raise RuntimeError("PROCESSOR_FAULT")   # or raise in on_end

provider = TracerProvider()
provider.add_span_processor(BuggyProcessor())
# then instrument the OpenAI SDK with this provider and make a chat completion call

Result for an ordinary client.chat.completions.create(...) call (sync, streamed, async), with openai 2.54.0 / 3.22.1 and opentelemetry-sdk 1.45.0:

Instrumentation	Processor raises in on_start	Processor raises in on_end
opentelemetry-instrumentation-genai-openai 1.2b0	app call raises RuntimeError (sync, stream, async)	app call raises (sync, stream, async)
opentelemetry-instrumentation-openai (OpenLLMetry) 0.62.4	app call raises (sync, stream, async)	app call raises (sync, async)
openinference-instrumentation-openai 0.1.61	unaffected (it wraps its own tracing calls)	unaffected

So with the current SDK, a bug in any single span processor turns every traced LLM call into a failure for instrumentations that rely on the SDK not throwing, as the spec allows them to. Catching processor exceptions in SynchronousMultiSpanProcessor, as this PR does, would make all three behave like the last row.

[somuai]

Thank you @armanunix for providing this concrete real-world impact data. This directly demonstrates why the SDK MultiSpanProcessor must catch and log unhandled exceptions during on_start and on_end, as permitted by the specification, rather than allowing processor faults to crash caller workflows in GenAI and HTTP instrumentations.

The implementation and unit tests in this PR are verified and ready for maintainer review.

[Copilot]

🟡 Changes recommended

Concurrent force-flush failures can remain unlogged when another processor times out.

1 open finding

• Process completed futures even when another processor times out · New

What changed in this PR

Improves MultiSpanProcessor resilience by containing and logging processor failures.

Changes:

• Guards processor callbacks and executor operations.
• Adds regression tests for exception handling.
• Adds a changelog entry.

File	Description
opentelemetry-sdk/​src/​opentelemetry/​sdk/​trace/​__init__.py	Handles processor and executor exceptions.
opentelemetry-sdk/​tests/​trace/​test_span_processor.py	Tests failure containment.
.changelog/​5626.fixed	Documents the fix.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[somuai]

Resolved in commit f186a47: removed early return on non-empty not_done_futures, marked all_flushed = False, and continue processing done_futures so completed futures are inspected and any exceptions are logged. Added regression test test_force_flush_timeout_still_inspects_completed_failures.

[herin049]

The exception potentially raised from the span processor won't be raised here, it'll be raised on the result below. Likewise the call to submit should never raise, unless the executor is shutdown which indicates a bug in the implementation.

[somuai]

Agreed @herin049. The span processor exception is raised on future.result() in the result evaluation loop below, not at submit(). The try/except around submit() was originally added to guard against executor shutdown at interpreter exit (RuntimeError: cannot schedule new futures after shutdown). Rebased cleanly on upstream main, updated force_flush to inspect completed futures on timeout, and all 48 unit tests pass cleanly.

[herin049]

Same issue here.

[chrikrah]

@somuai, the Copilot timeout fix holds at f186a47. On @herin049's submit point: the exit path is already tracked in #5568 and #4461, and its inline fallback was #5569, which closed stale on 2026-10-02. Commenting rather than approving, because the guard here decides what that fallback can still do.

$ python -m pytest tests/trace/test_span_processor.py -q -p no:cacheprovider    # opentelemetry-sdk at f186a47, Python 3.12.6, import path asserted
48 passed
# sdk/trace/__init__.py from a517a73, tests kept
1 failed, 47 passed    # test_force_flush_timeout_still_inspects_completed_failures
# sdk/trace/__init__.py from merge base 2fa016b
12 failed, 36 passed

$ python -I exit_probe.py    # ConcurrentMultiSpanProcessor wrapping one processor, TracerProvider(shutdown_on_exit=True), one span, then exit
# base 2fa016b
Exception ignored in atexit callback: <bound method TracerProvider.shutdown ...>
RuntimeError: cannot schedule new futures after shutdown
# head f186a47
on_end ran
Exception while submitting task to SpanProcessor executor.
RuntimeError: cannot schedule new futures after shutdown
# head, except branch calls func(sp)(*args, **kwargs) inside its own try/except that logs
on_end ran
inner shutdown ran

$ python raise_probe.py    # one processor whose on_end raises ValueError, executor shut down, one span ended
# head f186a47
caller survived
# head, bare inline call in the except branch (as in #5569)
ESCAPED to caller: ValueError processor bug
# head, inline call inside its own try/except
caller survived

non-blocking: once this merges, the except RuntimeError that #5569 relied on never fires, so a separate exit fix has to rework this guard anyway. A bare inline call would also bring #5624 back. Under its own guard it runs the inner shutdown() and the caller survives. The cost to you is one branch and one test more. I'd still fold it in here.

@herin049, would you take the guarded fallback in this PR, or keep it to logging and settle the direction on #5568, where Pr1meGG asked on 2026-10-08?