fix: record errors on spans for anthropic, groq, and mistralai#3970
fix: record errors on spans for anthropic, groq, and mistralai#3970Dheeraj-Bhaskaruni wants to merge 1 commit intotraceloop:mainfrom
Conversation
…mentations When API calls to foundation model providers raise exceptions, the spans were not being marked as failed. This adds span.record_exception() and span.set_status(ERROR) to the error paths in both sync and async wrappers for these three packages. - anthropic: added record_exception, set_status(ERROR), and span.end() - groq: added record_exception, set_status(ERROR), and span.end() - mistralai: wrapped API calls in try/except with full error recording Fixes traceloop#412
📝 WalkthroughWalkthroughThe changes add exception handling to synchronous and asynchronous wrappers in three OpenTelemetry instrumentation packages (anthropic, groq, mistralai). When exceptions occur during API calls, spans now explicitly record the exception, set status to ERROR with the exception message, and end the span before re-raising. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.py (1)
567-581:⚠️ Potential issue | 🔴 CriticalAnthropicStream and context manager wrappers do not properly handle span closure on streaming errors.
The exception handler in
AnthropicStream.__next__()(lines 226-230) increments the exception counter but does not callrecord_exception(),set_status(StatusCode.ERROR), orspan.end(). When iteration fails mid-stream, the span remains incomplete and unmarked as errored.Additionally,
WrappedMessageStreamManager.__exit__()andWrappedAsyncMessageStreamManager.__aexit__()delegate entirely to the underlying stream manager without checking or handling the span state. If an exception propagates during thewith/async withblock, the span is never marked as errored or properly closed.While
AnthropicAsyncStream.__anext__()(lines 374-384) correctly handles streaming exceptions by setting error status and ending the span, the sync path and both context manager wrappers leave spans in an incomplete state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.py` around lines 567 - 581, The sync streaming path and context-manager wrappers must mirror the async error-handling: in AnthropicStream.__next__ ensure that when an exception occurs you call span.record_exception(e), span.set_status(Status(StatusCode.ERROR, str(e))) and span.end() (and record duration_histogram/exception_counter like the async path) before re-raising; similarly, in WrappedMessageStreamManager.__exit__ and WrappedAsyncMessageStreamManager.__aexit__ detect if an exception is propagating (non-None exc_type/exception) and, if a span exists on the wrapper, call span.record_exception(exc), span.set_status(Status(StatusCode.ERROR, str(exc))) and span.end() (and update duration/exception metrics) so spans are always marked errored and closed on stream errors.packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py (1)
256-269:⚠️ Potential issue | 🟠 MajorStream exception handling bypasses record_exception() and span.end() for mid-stream failures.
The try/except blocks at lines 269–276 and 355–361 only catch exceptions during generator creation, not during chunk iteration. Exceptions raised while consuming
_create_stream_processor()or_create_async_stream_processor()will not triggerrecord_exception(),set_status(StatusCode.ERROR), orspan.end()since those operations are only called after normal loop completion. Thefor/async forloops inside these processors must wrap iteration in try/except, plus add a regression test for exceptions raised after the first chunk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py` around lines 256 - 269, The stream processors (_create_stream_processor and _create_async_stream_processor) only handle exceptions during generator creation but not during iteration, so wrap the actual for/async for iteration inside a try/except that mirrors the existing exception handler: on any exception compute end_time, build attributes via error_metrics_attributes(e), record duration to duration_histogram if present, call span.record_exception(e), span.set_status(Status(StatusCode.ERROR, str(e))), span.end(), then re-raise; ensure the normal span.end() remains on normal completion. Also add a regression test that raises an exception after yielding the first chunk to verify record_exception, set_status and span.end are invoked for mid-stream failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.py`:
- Around line 417-423: The streaming accumulator generators
_accumulate_streaming_response and _aaccumulate_streaming_response currently
iterate without protection, so exceptions raised mid-stream leave spans open;
update both functions to wrap their iteration loop in a try/except/finally: in
except call span.record_exception(e) and
span.set_status(Status(StatusCode.ERROR, str(e))) then re-raise, and in finally
ensure span.end() and any _handle_response() cleanup always run; also add a unit
test that simulates an exception thrown mid-stream (e.g., a generator that
raises partway through) to assert record_exception, set_status and span.end are
invoked to prevent regression.
---
Outside diff comments:
In
`@packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.py`:
- Around line 567-581: The sync streaming path and context-manager wrappers must
mirror the async error-handling: in AnthropicStream.__next__ ensure that when an
exception occurs you call span.record_exception(e),
span.set_status(Status(StatusCode.ERROR, str(e))) and span.end() (and record
duration_histogram/exception_counter like the async path) before re-raising;
similarly, in WrappedMessageStreamManager.__exit__ and
WrappedAsyncMessageStreamManager.__aexit__ detect if an exception is propagating
(non-None exc_type/exception) and, if a span exists on the wrapper, call
span.record_exception(exc), span.set_status(Status(StatusCode.ERROR, str(exc)))
and span.end() (and update duration/exception metrics) so spans are always
marked errored and closed on stream errors.
In
`@packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py`:
- Around line 256-269: The stream processors (_create_stream_processor and
_create_async_stream_processor) only handle exceptions during generator creation
but not during iteration, so wrap the actual for/async for iteration inside a
try/except that mirrors the existing exception handler: on any exception compute
end_time, build attributes via error_metrics_attributes(e), record duration to
duration_histogram if present, call span.record_exception(e),
span.set_status(Status(StatusCode.ERROR, str(e))), span.end(), then re-raise;
ensure the normal span.end() remains on normal completion. Also add a regression
test that raises an exception after yielding the first chunk to verify
record_exception, set_status and span.end are invoked for mid-stream failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7db9efe9-3b41-47dd-95e1-3740189167fc
📒 Files selected for processing (3)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.pypackages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.pypackages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.py
| try: | ||
| response = wrapped(*args, **kwargs) | ||
| except Exception as e: | ||
| span.record_exception(e) | ||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||
| span.end() | ||
| raise |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && wc -l opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 120
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && sed -n '400,480p' opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 2298
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && grep -n "_accumulate_streaming_response\|_aaccumulate_streaming_response" opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 310
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && sed -n '200,280p' opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 3142
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && sed -n '410,445p' opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 948
🏁 Script executed:
cd packages/opentelemetry-instrumentation-mistralai && sed -n '460,495p' opentelemetry/instrumentation/mistralai/__init__.pyRepository: traceloop/openllmetry
Length of output: 1066
Stream consumption failures bypass span cleanup.
The new exception handlers (lines 417–423 and 469–478) only catch errors while obtaining the response object via wrapped(). When streaming is enabled, control is returned to the caller via generator functions (_accumulate_streaming_response and _aaccumulate_streaming_response) that iterate the stream without any try/except/finally wrapper. If an exception occurs during iteration—such as a network error, malformed chunk, or timeout—the exception propagates to the caller while the generator's remaining code (including _handle_response() and span.end()) is never executed. This leaves the span open and unrecorded.
Both accumulator functions must wrap their iteration loops in try/except/finally to ensure record_exception(), set_status(), and span.end() are always called. Add a test that raises an exception mid-stream to prevent regression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.py`
around lines 417 - 423, The streaming accumulator generators
_accumulate_streaming_response and _aaccumulate_streaming_response currently
iterate without protection, so exceptions raised mid-stream leave spans open;
update both functions to wrap their iteration loop in a try/except/finally: in
except call span.record_exception(e) and
span.set_status(Status(StatusCode.ERROR, str(e))) then re-raise, and in finally
ensure span.end() and any _handle_response() cleanup always run; also add a unit
test that simulates an exception thrown mid-stream (e.g., a generator that
raises partway through) to assert record_exception, set_status and span.end are
invoked to prevent regression.
2 participants
Summary
Fixes #412
When API calls to foundation model providers raise exceptions, the OpenTelemetry spans were not being properly marked as failed. This PR adds error recording to three instrumentation packages:
span.record_exception(),span.set_status(ERROR), orspan.end()try/exceptat all aroundwrapped()calls, so exceptions propagated with spans never ended or markedChanges
All three packages now follow the same pattern used by
opentelemetry-instrumentation-openai:Files changed
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.pypackages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.pypackages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.pyTest plan
ruff checkpasses on all changed filesSummary by CodeRabbit