fix(otel): hashable scope for _emit_once when guardrail_mode is list - #31262
Conversation
Greptile SummaryFixes
Confidence Score: 5/5Safe to merge. The change is confined to the v1 OTel logging callback path; the dedupe fix is additive, all hashable-scope call sites are unaffected, and the new helper has a depth cap with a safe fallback. The change is narrow: one new module-level helper, one four-line substitution in No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds _freeze_for_dedupe helper (depth-capped recursive normalizer) and applies it inside _emit_once when building the dedupe tuple key; fixes the TypeError: unhashable type: 'list' crash when guardrail_mode is a list. |
| tests/test_litellm/integrations/test_opentelemetry.py | Adds four new unit tests to TestOpenTelemetrySpanDedupe: list/dict/set scope parts, distinct list keys, cyclic input depth cap, and end-to-end _create_guardrail_span with list guardrail_mode. All tests are properly mocked with no real network calls. |
| tests/code_coverage_tests/recursive_detector.py | Adds _freeze_for_dedupe to IGNORE_FUNCTIONS with an accurate justification comment — consistent with all other depth-guarded recursive helpers in this list. |
Reviews (6): Last reviewed commit: "fix: avoid explicit casting" | Re-trigger Greptile
Greptile SummaryFixes a
Confidence Score: 5/5Safe to merge — the change is contained to a single helper function and one call site, with no modifications to existing tests or observable behavior for the already-working string-mode path. The fix is minimal and surgical: No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds _freeze_for_dedupe module-level helper and applies it inside _emit_once to make the dedupe key hashable when guardrail_mode is a list; docstring updated to match |
| tests/test_litellm/integrations/test_opentelemetry.py | Adds four regression tests to TestOpenTelemetrySpanDedupe: list scope crash, distinct list scope uniqueness, dict/set scope parts, and end-to-end _create_guardrail_span with list mode — all using in-memory OTEL components, no network calls |
Reviews (2): Last reviewed commit: "fix(otel): hashable scope for _emit_once..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| _FREEZE_MAX_DEPTH = 16 | ||
|
|
||
|
|
||
| def _freeze_for_dedupe(value: object, _depth: int = 0) -> object: |
There was a problem hiding this comment.
q: why can't this return a union of all the different possible return types?
There was a problem hiding this comment.
Tried it — the passthrough return value would need a cast(Hashable, value) to satisfy basedpyright, which CLAUDE.md says don't. So object → object it is.
There was a problem hiding this comment.
passthrough return value would need a cast(Hashable, value) to satisfy basedpyright
Why? Is value not hashable?
There was a problem hiding this comment.
Hashable would need a cast to compile, which the repo rules forbid.
There was a problem hiding this comment.
if that's not the case, I can use Union
There was a problem hiding this comment.
Better to have type info. If it's not possible to have type info without casting, I would rather cast
There was a problem hiding this comment.
Wait actually can you swap the hash() try/except + cast for if isinstance(value, (str, int, float, bytes)) or value is None: return value else repr(value). Type narrow instead of cast
|
@greptileai re-review please |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1483a1b. Configure here.
`_emit_once` keys `spans_logged` by `(class, id, *scope)`. When a guardrail entry's `guardrail_mode` arrives as a `List[GuardrailEventHooks]` (the shape Presidio expands to with `output_parse_pii: true`, and the shape `event_hook` carries for any `mode: [...]` in config), the tuple contains a list and `spans_logged.get(dedupe_key)` raises `TypeError: unhashable type: 'list'`. On the post-call path this fires inside the logging callback and is swallowed; the request returns 200 but the OTEL `guardrail` span is silently dropped. On the blocking path the same error surfaces as HTTP 500. Adds `_freeze_for_dedupe`, a small recursive normalizer that turns lists and tuples into tuples, sets into frozensets, dicts into frozensets of `(key, value)` pairs, and falls back to `repr` for arbitrary unhashables. Applied inside `_emit_once` before the dict lookup, so all three callsites are protected without touching the guardrail-specific callsite. Helper assumes acyclic input; `guardrail_mode` values are built fresh from config (str enums, lists of str enums, TypedDict of str/list-of-str), so no cycle can arise in practice. Regression tests in `TestOpenTelemetrySpanDedupe` cover the list crash, distinct-list-scope collision, dict and set scope parts, and an end-to-end `_create_guardrail_span` exercise that confirms exactly one `guardrail` span is emitted across repeated lifecycle entrypoints. Each new test fails on a reverted helper (4/4 mutation kill)
…sive detector CI's recursive_detector blocks new recursive functions in litellm/ unless they are in the allowlist with a documented bound. Cap the helper at 16 levels and return repr(value) past the cap; this is well past the realistic depth of guardrail_mode (1-3 levels) and means a future caller passing a cyclic container can no longer push the proxy logging path into a RecursionError. Add a regression test that exercises the cycle path.
… union Per review feedback from @mateo-berri: replace the loose `-> object` annotation with a recursive `HashableScope` union (str | int | float | bool | bytes | None | Tuple[HashableScope, ...] | FrozenSet[HashableScope]) so the helper's contract is visible at the signature. Replace the `try/except hash(value); return value` passthrough with an explicit isinstance check over the hashable-scalar types so the type checker can narrow without requiring `cast(Hashable, value)` on the return. Symmetric: dict keys also flow through the freezer (a TypedDict key is already a string in practice, so behaviorally identical). All 16 regression tests still pass; mutation kill behavior preserved
1483a1b to
12c8eda
Compare
|
@greptileai please re-review on the latest HEAD (rebased onto staging, includes @mateo-berri's avoid-cast commit) |
a545c49
into
litellm_internal_staging
Relevant issues
Fixes #28486
Linear ticket
Resolves LIT-3428
Resolves LIT-3764
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Link: https://github.com/BerriAI/litellm/actions?query=branch%3Alitellm_lit_3428_otel_emit_once_hashable
Link: https://github.com/BerriAI/litellm/actions?query=branch%3Alitellm_lit_3428_otel_emit_once_hashable
Links: TBD
Screenshots / Proof of Fix
Live proxy on this PR's worktree, real Anthropic API, OTEL callback on, custom guardrail with
mode: ["pre_call", "post_call"](the shape Presidio expands to whenoutput_parse_pii: true). Same curl on the broken commit and the fixed commit; the only difference is whether_freeze_for_dedupeis applied inside_emit_once.Repro config (custom guardrail records to
standard_logging_objectso_create_guardrail_spanactually runs):Launch:
Curl (identical on both runs):
Before (origin/litellm_internal_staging, no fix)
The user sees a 200 because the crash happens inside the logging callback and is swallowed; the request itself succeeds. The damage is silent observability loss. Proxy log:
Traceback in the log (each occurrence — sync + async success callbacks both fire):
On the blocking path (e.g. callbacks that mark guardrail-failure synchronously), the same error surfaces as HTTP 500; see GH #28486.
After (this PR)
One of the emitted guardrail spans (truncated to the relevant attributes):
{ "name": "guardrail", "attributes": { "openinference.span.kind": "GUARDRAIL", "guardrail_name": "lit-3428-list-mode", "guardrail_mode": "['pre_call', 'post_call']", "guardrail_response": "{\"checked\": true, \"stage\": \"pre_call\"}", "guardrail_status": "success" } }Regression check that the existing
mode: pre_call(string) path is unaffected: the same curl withmode: pre_callreturns 200, emits the guardrail span, no crash.Type
🐛 Bug Fix
Changes
_emit_oncekeysspans_loggedby(class, id, *scope)._create_guardrail_spanpassesguardrail_information["guardrail_mode"]into that scope; its type isOptional[Union[GuardrailEventHooks, List[GuardrailEventHooks], GuardrailMode]](litellm/types/utils.py), so a YAMLmode: [pre_call, post_call]makes it a real list at runtime. Lists are not hashable, sospans_logged.get(dedupe_key)raisesTypeError: unhashable type: 'list'.The fix is a small recursive normalizer applied inside
_emit_oncebefore the dict lookup:It is applied at the helper, not at the guardrail callsite, so all three
_emit_oncecallsites ("success","failure", and the guardrail one) are protected without per-site work. The helper assumes acyclic input;guardrail_modevalues are built fresh from config (str enums, lists of str enums, TypedDict of str/list-of-str leaves), so a self-referential value cannot arise in practice.The dedupe contract is preserved: distinct list scopes produce distinct keys (a fixed-string mutant fails the new test), and the existing string-scope behavior is byte-for-byte the same (the helper passes already-hashable scalars straight through).
Regression coverage in
TestOpenTelemetrySpanDedupe(tests/test_litellm/integrations/test_opentelemetry.py):test_emit_once_accepts_list_valued_scope_part— the exact crash casetest_emit_once_distinct_list_scopes_dont_collide— distinctness is preservedtest_emit_once_accepts_dict_and_set_scope_parts— covers theGuardrailModeTypedDict shape and a future set-shaped scopetest_create_guardrail_span_does_not_raise_on_list_mode— end-to-end through_create_guardrail_spanconfirming exactly one guardrail span emits across repeated lifecycle entrypointsEach new test fails when the
_freeze_for_dedupeapplication is reverted; mutation kill rate 4/4Scope
This is a surgical patch for v1 (
litellm/integrations/opentelemetry.py), not an architectural change. The reason v1 needs a dedupe at all is that_create_guardrail_spanis registered at three lifecycle hooks (async_post_call_success_hook,_handle_success,_handle_failure) and re-reads the guardrail entry list fromstandard_logging_payloadeach time;_emit_onceis the workaround that collapses the three-way fan-out into one span. That fan-out is the structural reason a stray list-valuedguardrail_modecould ever reach a dict key in the first place.v2 (
litellm/integrations/otel/) does not have this shape: it emits each guardrail span directly from the guardrail-recording code at the moment a guardrail finishes (emit_guardrail_spaninlitellm/integrations/otel/logger.py), so the emitter never sees the same entry twice and never needs to hashguardrail_mode. v2 also normalizesguardrail_modeto a display string at the front door via_guardrail_mode_strinlitellm/integrations/otel/model/payloads.py, so the wider type can never crash a downstream callsite.The right long-term move is to retire v1's three-hook registration in favor of v2's direct-emit pattern; that is out of scope here and tracked separately. This PR is the minimum patch that unblocks v1 customers hitting the crash today
Note
Low Risk
Narrow change to v1 OTEL span dedupe on the logging callback path; behavior for hashable scopes is unchanged aside from fixing the list/dict/set crash.
Overview
Fixes
TypeError: unhashable type: 'list'when OpenTelemetry dedupes spans andguardrail_modeis a list (e.g. Presidio withoutput_parse_pii: trueor YAMLmode: [pre_call, post_call]).Adds
_freeze_for_dedupeto turn list/tuple, set, and dict scope parts into hashable keys (depth cap 16,reprfallback), and applies it inside_emit_onceso all dedupe sites are covered without changing string-scope behavior. Guardrail span emission and dedupe are restored; regression tests cover list/dict/set scopes, distinct list keys, cyclic input, and end-to-end_create_guardrail_span.Reviewed by Cursor Bugbot for commit 1483a1b. Bugbot is set up for automated code reviews on this repo. Configure here.