Skip to content

Conversation

@kxbnb
Copy link

@kxbnb kxbnb commented Jan 20, 2026

Summary
Adds error handling to the vertexai instrumentation so exceptions are properly recorded on OTel spans.

Changes

  • Import ERROR_TYPE from semconv error attributes
  • Add try/except to _awrap (async) wrapper
  • Add try/except to _wrap (sync) wrapper
  • On exception: set ERROR_TYPE attribute, record_exception, set status to ERROR, end span

Validation

  • Lint passes
  • All 13 tests pass

Testing

npx nx run opentelemetry-instrumentation-vertexai:lint
uv run pytest tests/ -v

Part of #412


Important

Add error handling to Vertex AI instrumentation by logging exceptions in _awrap and _wrap functions.

  • Error Handling:
    • Add try/except in _awrap and _wrap to log exceptions.
    • On exception: set ERROR_TYPE, record exception, set status to ERROR, end span.
  • Imports:
    • Import ERROR_TYPE from semconv.error_attributes.
  • Validation:
    • Lint and all 13 tests pass.

This description was created by Ellipsis for 9272570. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error tracking in OpenTelemetry VertexAI instrumentation: traces now record exception class names and messages as error attributes when failures occur.
    • Consistent error capturing across synchronous and asynchronous, streaming and non‑streaming flows, ensuring spans are marked and recorded on failures for better observability.

✏️ Tip: You can customize this high-level summary in your review settings.

Adds error handling to the sync and async wrappers in vertexai
instrumentation to properly log errors via OTel spans when
API calls fail.

Changes:
- Set ERROR_TYPE attribute with exception class name
- Record exception on span
- Set span status to ERROR with message

Part of traceloop#412

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

Adds centralized error handling to Vertex AI instrumentation: exceptions caught in sync/async streaming and wrapper paths now set an ERROR_TYPE span attribute, record the exception, set span status to ERROR, end the span, and re-raise; successful paths unchanged. (≤50 words)

Changes

Cohort / File(s) Summary
Vertex AI instrumentation — error handling
packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py
Added try/except around streaming response builders (_build_from_streaming_response, _abuild_from_streaming_response) and wrapper invocations (_wrap, _awrap). On exception: set span attribute ERROR_TYPE, call span.record_exception, set span status to ERROR with the exception message, end the span, then re-raise. Imported ERROR_TYPE.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop and peek where errors creep,

I mark their kinds and never sleep,
Spans now know each fault and name,
I log, I end, then pass the flame,
A rabbit's hop keeps telemetry tame. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main change: adding error logging and exception handling to the Vertex AI instrumented functions, which directly aligns with the changeset's focus on error attribute tracking and try/except blocks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 9272570 in 1 minute and 50 seconds. Click for details.
  • Reviewed 44 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py:249
  • Draft comment:
    Good use of try/except in _awrap. Capturing exceptions with ERROR_TYPE, recording the error, updating status, ending the span, then re-raising preserves expected behavior. Consider if exposing str(e) in span status is acceptable.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment says "Consider if exposing str(e) in span status is acceptable." This is asking the PR author to think about whether this is okay, rather than stating that there is definitely a problem. It's a speculative comment that doesn't provide clear evidence of an issue. The comment doesn't say "This will expose sensitive information" or "Remove str(e) from the span status" - it just asks the author to consider it. This violates the rule about not asking the PR author to confirm their intention or double-check things. The comment is not actionable because it doesn't suggest a specific code change. However, this could be a legitimate security concern about potentially exposing sensitive information in exception messages. If exception messages could contain PII or sensitive data, this would be a real issue. The comment might be trying to raise awareness of a security best practice. While security concerns are valid, the comment doesn't provide any evidence that this is actually a problem in this specific context. It's purely speculative ("Consider if...") rather than stating a definite issue. Without concrete evidence that Vertex AI exceptions contain sensitive data, or a clear recommendation to change the code, this is just asking the author to think about it, which violates the rules. This comment should be deleted because it's speculative and asks the author to consider something without providing clear evidence of a problem or a specific code change. It violates the rule against asking the PR author to confirm, verify, or ensure things.
2. packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py:310
  • Draft comment:
    The try/except in _wrap similarly ensures errors are captured on synchronous calls. Recording ERROR_TYPE, exception details, and setting ERROR status before ending the span follows best practices. Verify that exposing str(e) is acceptable for your use case.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify if exposing str(e) is acceptable, which falls under asking the author to confirm their intention or ensure behavior is intended. This violates the rules.
3. packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py:32
  • Draft comment:
    Typographical suggestion: The content on line 32 appears to consist solely of a ')'. If this is unintended, please remove or correct it.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_2A2AFRrdybQ3m0BF

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py`:
- Around line 249-256: The current try/except only wraps the initial await
wrapped(*args, **kwargs) so streaming iteration errors bypass span handling;
update the streaming handlers (_build_from_streaming_response and
_abuild_from_streaming_response) to wrap their for and async for loops in
try/except blocks that mirror the outer handler: on exception set
span.set_attribute(ERROR_TYPE, e.__class__.__name__), span.record_exception(e),
span.set_status(Status(StatusCode.ERROR, str(e))), call span.end(), then
re-raise the exception so iteration-level errors are captured and the span is
properly closed.

Addresses CodeRabbit review feedback - the streaming iteration loops
also need try/except to capture errors that occur during streaming.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py`:
- Line 193: The async streaming call incorrectly passes the generator object
`response` to `handle_streaming_response`; change the call to pass the
accumulated string `complete_response` instead so the span receives the final
text. Update the invocation of handle_streaming_response(span, event_logger,
llm_model, response, token_usage) to use handle_streaming_response(span,
event_logger, llm_model, complete_response, token_usage) in the async flow to
match the sync behavior.
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py (1)

149-172: Error handling implementation looks correct.

The try/except properly wraps the iteration loop, capturing streaming exceptions with appropriate span attributes, exception recording, status setting, and span termination before re-raising.

Minor observation: handle_streaming_response (line 167) already calls span.set_status(Status(StatusCode.OK)) when the span is recording (see line 146), making line 171 redundant. This is harmless but could be cleaned up for clarity.

Fixes bug in async streaming handler where `response` (generator) was
passed instead of `complete_response` (accumulated string).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant