fix(otel): normalise list guardrail_mode to tuple in _emit_once to prevent TypeError - #28556
Conversation
|
|
Greptile SummaryFixes a
Confidence Score: 4/5Safe to merge — the change is a one-line normalisation in a well-contained helper with a direct regression test covering the failure path. The fix is correct and minimal. The only notable gap is that the new regression test doesn't assert mock_span.end.assert_called_once(), which the other guardrail tests do include, leaving an unchecked span-lifecycle branch. No files require special attention beyond the minor missing assertion in the new test.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Normalises list-valued scope parts to tuples in _emit_once so the deduplication key is always hashable; fix is minimal and correctly targeted at the root cause. |
| tests/test_litellm/integrations/test_opentelemetry.py | Adds a focused regression test for the list-mode crash; uses mocks only (no network calls), but omits mock_span.end.assert_called_once() that other guardrail tests include. |
Reviews (1): Last reviewed commit: "test(otel): add regression for list guar..." | Re-trigger Greptile
| # Must not raise TypeError: unhashable type: 'list' | ||
| otel._create_guardrail_span(kwargs=kwargs, context=None) | ||
| otel.tracer.start_span.assert_called_once() |
There was a problem hiding this comment.
The new test verifies that
start_span is called but doesn't assert that the span was properly ended. The sibling test test_create_guardrail_span_with_valid_info includes mock_span.end.assert_called_once(). Without that assertion here, a future regression where the span is created but never closed would go undetected by this test.
| # Must not raise TypeError: unhashable type: 'list' | |
| otel._create_guardrail_span(kwargs=kwargs, context=None) | |
| otel.tracer.start_span.assert_called_once() | |
| # Must not raise TypeError: unhashable type: 'list' | |
| otel._create_guardrail_span(kwargs=kwargs, context=None) | |
| otel.tracer.start_span.assert_called_once() | |
| mock_span.end.assert_called_once() |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 3/5 ❌ Why blocked:
Details: Score docked for: 1 PR-related CI failure (lint). Fix the issues above and push an update — the bot will re-review automatically.
|
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Problem
When a guardrail is configured with a list-valued
mode(e.g.mode: ["pre_call", "post_call"]) and the OpenTelemetry integration is active, every request that triggers the guardrail returns HTTP 500:Bisected to #27757 (commit
1c4e4d4).Closes #28486.
Root cause
_emit_onceconstructs a deduplification key:scopeis unpacked via*, so if one element is alist(theguardrail_mode), the resulting tuple contains alist. Tuples that contain unhashable elements (likelist) are themselves unhashable — sospans_logged.get(dedupe_key)raisesTypeError.Fix
Normalise each
scopepart before building the key: convertlistvalues totuple(which is hashable and preserves identity for dedupe purposes):All non-list scope parts pass through unchanged (strings, ints, floats,
None, etc. are all hashable).Changes
litellm/integrations/opentelemetry.py: normalisescopeparts in_emit_oncetests/test_litellm/integrations/test_opentelemetry.py: addtest_guardrail_mode_as_list_does_not_crash