[feiiiiii5]

Problem

A chat completion with stream=True that fails after the first chunk is exported with status OK. In ChatStream.__next__ / __anext__ the cleanup path runs first — _ensure_cleanup() sets StatusCode.OK and calls end() — and the set_status(StatusCode.ERROR, ...) on the next line can never take effect, because an ended span no longer records. The caller still gets the exception out of the iterator, so this is silent: the trace reports a successful call, with no error.type, no exception event, and no failed span.

That is a dropped connection, a 5xx from a gateway, a rate limit or a timeout between chunks — i.e. the error rate and success rate this instrumentation exists to report are wrong in exactly the situation people look at them.

Fix

Cleanup is what ends the span, so let it decide the final status. _ensure_cleanup(error=None) keeps the previous OK behaviour for the __del__ / __exit__ callers, and when an error is passed it sets error.type, records the exception and sets ERROR — the same three calls the non-streaming paths in this file already make (chat_wrappers.py:117 sync, :218 async).

Test

cd packages/opentelemetry-instrumentation-openai
uv run pytest tests/traces/test_streaming_errors.py -q
# on be498301: 2 failed
#   AssertionError: a stream that raised was exported as StatusCode.OK: the failed call reads as a success
# here:         2 passed

uv run pytest tests/ -q      # 272 passed, 1 skipped (the skip is pre-existing)
uv run ruff check .          # All checks passed

The new file drives ChatStream directly, the way tests/traces/test_semconv_compliance.py::TestPartialStreamCleanupOutputMessages does: a stub stream yields one chunk and then raises, once through for and once through async for, and the test asserts the exported span is ERROR with error.type == "RuntimeError" and an exception event.

One note on the PR template: it asks for a screenshot from an observability platform when an existing instrumentation changes. I cannot attach one, so the evidence above is the exported span itself — the status code, error.type and the exception event, read back from the in-memory span exporter. That is the same data a trace view would show, and it is asserted rather than eyeballed. The other template boxes are checked.
Related, different path: #4394 ends the span for an early close() on the Completions stream, where the span was never finished at all. This is the Chat stream, where the span is finished with the wrong status.

Summary by CodeRabbit

• Bug Fixes
  • Fixed error reporting for synchronous and asynchronous OpenAI chat streams. When a stream fails during iteration, the resulting trace now records the error and its type, making failed requests easier to identify.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 53f018f2-61eb-4373-9353-2abdcde78300

📥 Commits

Reviewing files that changed from the base of the PR and between eddd811 and 277f91b.

📒 Files selected for processing: 1

• packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Synchronous and asynchronous chat stream iteration now pass failures to shared cleanup. Cleanup records the exception and sets the span status to ERROR. Regression tests verify exception propagation and span data.

Changes: Chat stream error handling

Layer / File(s)	Summary
Stream error cleanup and regression coverage packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py, packages/opentelemetry-instrumentation-openai/tests/traces/test_streaming_errors.py	Both stream iteration methods pass errors to _ensure_cleanup. Cleanup records the exception and sets the span status to ERROR. Tests cover synchronous and asynchronous streams that raise RuntimeError.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: minimaluminiumalism

Merge Risk: ⚪ Minimal · up to 277f9

The change correctly records failed streaming requests as ERROR spans, with no actionable merge-blocking risk identified.

Security architecture risk: 🔵 Low · up to eddd8

The fix records stream failures before closing their spans and preserves caller-visible exceptions. It does not expand client authority. Stream exception details now enter telemetry, but their sensitivity and downstream privacy handling are not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure change concerns exception details from instrumented synchronous and asynchronous chat streams entering their existing span telemetry path. No concrete sensitive payload, broader tenant exposure or privilege gain was established; downstream filtering remains unknown.

Trust Boundaries and Controls

• observed — Both production wrappers enforce instrumentation suppression before creating a span or ChatStream. Suppressed calls return the wrapped client result directly. The changed finalizer checks whether the span is recording before writing exception telemetry.

Resilience and Maintainability Implications

• observed — Context-exit handling, interruption outside Exception, and repeated normal-exhaustion behavior are unchanged. Their pre-existing lifecycle limitations are not introduced or expanded by the iterator-error handoff.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: failed OpenAI streams now produce spans with ERROR status.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

_{Comment @coderabbitai help to get the list of available commands.}

[feiiiiii5]

Hi — picking this back up. Head is eddd8116; the CLA is signed and the branch is clean, it has just been sitting behind main.

Recap: a chat completion with stream=True that fails after the first chunk is exported as status OK. _ensure_cleanup() sets StatusCode.OK and calls end() on the way out, so the set_status(StatusCode.ERROR, ...) on the next line can never take effect — an ended span no longer records. The caller still gets the exception out of the iterator, so this is silent: no error.type, no exception event, no failed span. A dropped connection, gateway 5xx, rate limit or timeout between chunks all report as a successful call.

The fix lets cleanup, which is what ends the span, decide the final status: _ensure_cleanup(error=None) keeps the previous OK behaviour for the __del__ / __exit__ callers, and when an error is passed it makes the same three calls the non-streaming paths in this file already make (chat_wrappers.py:117 sync, :218 async). tests/traces/test_streaming_errors.py goes 2 failed on be498301 → 2 passed.

It has drifted behind main. Want me to rebase it? I'd rather not push a force-update onto a PR you may already have looked at without asking — one word and I'll rebase and re-run the suite.