Skip to content

fix arize observability bugs - #26526

Closed
mubashir1osmani wants to merge 8 commits into
BerriAI:litellm_internal_stagingfrom
mubashir1osmani:arixe
Closed

fix arize observability bugs#26526
mubashir1osmani wants to merge 8 commits into
BerriAI:litellm_internal_stagingfrom
mubashir1osmani:arixe

Conversation

@mubashir1osmani

Copy link
Copy Markdown
Contributor

Relevant issues

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

@mubashir1osmani
mubashir1osmani marked this pull request as ready for review April 25, 2026 21:51
@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.53202% with 347 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/integrations/arize/_utils.py 17.42% 237 Missing ⚠️
litellm/integrations/arize/arize_phoenix.py 7.56% 110 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes several Arize/Phoenix observability bugs: refactors image-trace payload handling with a size cap, adds a router-level parent span to group retries and fallbacks under one root in Phoenix, fixes guardrail span parenting, and adds fallback event spans.

  • P1 (_utils.py lines 779, 784): "application/json" or None is a constant expression that always evaluates to "application/json", silently tagging every output_text, result, and content tool/MCP response as JSON — Phoenix will attempt to parse plain-text tool output as JSON and fail to render it.
  • P2 (router.py): asyncio.CancelledError (a BaseException in Python 3.8+) bypasses except Exception, leaving final_exception = None and causing the parent span to report OK status for cancelled requests.

Confidence Score: 3/5

Not safe to merge as-is — the constant MIME-type expression in _extract_chain_output will silently corrupt Phoenix span output metadata for all tool/MCP calls.

One clear P1 logic bug ("application/json" or None is always truthy) directly affects span output rendering in Phoenix, plus a P2 asyncio.CancelledError capture gap in router.py. The P1 brings the ceiling to 4; the additional P2 and lack of any tests pull it to 3.

litellm/integrations/arize/_utils.py (lines 776–784) needs the most attention before merge.

Important Files Changed

Filename Overview
litellm/integrations/arize/_utils.py Large refactor adding image-trace, chain-output, fallback-span, and response-ID utilities. Contains a P1 "application/json" or None constant expression bug that always forces JSON MIME type on plain-text tool/MCP results.
litellm/integrations/arize/arize_phoenix.py Adds router-level parent span management, guardrail-context fix, fallback event spans, and a no-op async_post_call_success_hook; logic looks correct but depends on the _utils.py fixes.
litellm/router.py Adds _start_router_parent_spans / _end_router_parent_spans hooks around the outer fallback call; asyncio.CancelledError can escape final_exception capture, causing cancelled spans to be recorded as OK.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Router
    participant ArizePhoenix
    participant OTelExporter

    Client->>Router: async_function_with_fallbacks (fallback_depth=0)
    Router->>ArizePhoenix: start_router_parent_span(kwargs)
    ArizePhoenix->>OTelExporter: start span "litellm_proxy_request" (CHAIN)
    Router->>Router: async_function_with_retries (attempt 1)
    Router->>ArizePhoenix: _handle_success / _handle_failure
    ArizePhoenix->>OTelExporter: child span "litellm_request" (LLM)
    alt Fallback triggered
        Router->>Router: async_function_with_fallbacks_common_utils
        Router->>ArizePhoenix: log_success_fallback_event / log_failure_fallback_event
        ArizePhoenix->>OTelExporter: sibling span "fallback: modelA -> modelB"
        Router->>Router: async_function_with_retries (fallback_depth=1)
        Router->>ArizePhoenix: _handle_success
        ArizePhoenix->>OTelExporter: child span "litellm_request" (LLM, retry)
    end
    Router->>ArizePhoenix: end_router_parent_span(kwargs, exception)
    ArizePhoenix->>OTelExporter: end span "litellm_proxy_request"
Loading

Reviews (5): Last reviewed commit: "fix lint" | Re-trigger Greptile

Comment thread litellm/integrations/arize/_utils.py Outdated
Comment thread litellm/integrations/arize/arize_phoenix.py Outdated
@veria-ai

veria-ai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Arize/Phoenix observability instrumentation improvements

This PR refactors image output tracing (with base64 size limits), adds parent span management for router-level fallback chains, and unifies structured output extraction for the Arize/Phoenix integration. The changes are confined to tracing/observability code that writes span attributes via safe_set_attribute (which casts all values to OTEL primitives). No auth, input validation, or data-flow boundaries are affected. The span registry has a soft cap at 200 stale entries but active entries can grow — this is a reliability concern, not a security one.


Status: 0 open
Risk: 1/10

Comment on lines +891 to +894
if isinstance(response_obj, dict):
provider_id = response_obj.get("id")
if provider_id:
return str(provider_id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 _resolve_response_id misses non-dict response objects

isinstance(response_obj, dict) is False for LiteLLM's ModelResponse / EmbeddingResponse objects, which are Pydantic BaseModel subclasses — not plain dicts. Those objects do expose a custom .get() method (see litellm/types/utils.py), but they are not detected here. As a result, every SDK-path LLM call silently skips the provider-issued ID (e.g. chatcmpl-XXX) and instead records litellm_call_id as llm.response.id, making it impossible to correlate Phoenix spans with the upstream provider's traces.

# Before: only captures dict responses
if isinstance(response_obj, dict):
    provider_id = response_obj.get("id")

# Fix: accept any object that has .get()
if hasattr(response_obj, "get"):
    provider_id = response_obj.get("id")

Comment thread litellm/integrations/arize/_utils.py Outdated
@mubashir1osmani
mubashir1osmani marked this pull request as draft April 28, 2026 14:01
Comment thread litellm/integrations/arize/_utils.py Outdated
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