From db94050dd7502b0590468c4ad62527c02395b727 Mon Sep 17 00:00:00 2001 From: oss-agent-shin Date: Wed, 27 May 2026 12:27:42 -0700 Subject: [PATCH] fix(otel): normalize unhashable scope in _emit_once (LIT-3299) (#29016) * fix(otel): normalize unhashable scope in _emit_once (LIT-3299) Add _make_hashable helper that recursively converts list/set/dict scope elements to hashable forms before they become part of the dedupe_key. Fixes HTTP 500 (TypeError: unhashable type: 'list') when a guardrail is configured with a list-valued mode and OTEL is on. * test(otel): regression tests for LIT-3299 _emit_once normalization 11 new tests covering: list-valued guardrail_mode (the original crash), list/tuple dedupe equivalence, nested list, dict scope with mixed-type keys, set scope, scalar pass-through, and that distinct list modes do not collapse to the same dedupe slot. * fix(otel): normalize tuple-with-unhashable in _emit_once (LIT-3299) Address Greptile P2: a tuple containing a list would still raise. Add a tuple branch to _make_hashable that recursively normalizes tuple elements. * test(otel): move pytest import to top + add tuple-with-unhashable test Address Greptile P2 style nit (mid-file import) and add coverage for the tuple-containing-unhashable case. --- litellm/integrations/opentelemetry.py | 54 +++++- .../integrations/test_opentelemetry.py | 181 ++++++++++++++++++ 2 files changed, 234 insertions(+), 1 deletion(-) diff --git a/litellm/integrations/opentelemetry.py b/litellm/integrations/opentelemetry.py index 81fdc5a1e21..c6c33b85b84 100644 --- a/litellm/integrations/opentelemetry.py +++ b/litellm/integrations/opentelemetry.py @@ -916,6 +916,54 @@ class OpenTelemetry(OTELGenAISemconvMixin, CustomLogger): # End of Team/Key Based Logging Control Flow ######################################################### + @staticmethod + def _make_hashable(value): + """Recursively convert a scope element into a hashable form. + + Used by :meth:`_emit_once` because callers pass scope elements that + come from user-supplied config (e.g. ``guardrail_mode`` which users + can configure as a list like ``["pre_call", "post_call"]``). Without + this normalization the resulting ``dedupe_key`` tuple contains an + unhashable element and ``spans_logged.get(...)`` raises ``TypeError: + unhashable type: 'list'`` (returning HTTP 500). + + Conversions: + - ``list`` -> ``tuple`` (preserves order, recursive) + - ``tuple`` -> ``tuple`` (recursive, in case any + element is itself + unhashable e.g. a list) + - ``set`` / ``frozenset`` -> ``frozenset`` (order-independent) + - ``dict`` -> ``tuple`` of ``(key, value)`` pairs + sorted by ``repr(key)`` so equivalent + dicts hash the same and dicts with + mixed-type / non-comparable keys still + normalize without crashing. + - everything else -> returned unchanged (already-hashable + scalars: ``None``, ``str``, ``int``, + ``float``, ``bool``, ...). + + The conversion is structural and equality-preserving for the common + case (``list`` and its equivalent ``tuple`` collapse to the same + dedupe slot), so existing dedupe semantics are unchanged. + """ + if isinstance(value, list): + return tuple(OpenTelemetry._make_hashable(v) for v in value) + if isinstance(value, tuple): + # Tuples are already hashable iff every element is hashable; a + # tuple containing e.g. a list would still fail. Normalize each + # element so a tuple-of-lists is also safe. + return tuple(OpenTelemetry._make_hashable(v) for v in value) + if isinstance(value, (set, frozenset)): + return frozenset(OpenTelemetry._make_hashable(v) for v in value) + if isinstance(value, dict): + return tuple( + sorted( + ((k, OpenTelemetry._make_hashable(v)) for k, v in value.items()), + key=lambda kv: repr(kv[0]), + ) + ) + return value + def _emit_once(self, kwargs: dict, *scope: object) -> bool: """Return True the first time this handler is asked to emit a span for the given (handler, scope) on this kwargs; False on repeats. @@ -958,7 +1006,11 @@ class OpenTelemetry(OTELGenAISemconvMixin, CustomLogger): spans_logged = {} _otel_internal["spans_logged"] = spans_logged - dedupe_key = (self.__class__.__name__, id(self), *scope) + dedupe_key = ( + self.__class__.__name__, + id(self), + *(self._make_hashable(s) for s in scope), + ) if spans_logged.get(dedupe_key) is True: return False diff --git a/tests/test_litellm/integrations/test_opentelemetry.py b/tests/test_litellm/integrations/test_opentelemetry.py index b65e629c890..81f57d06f84 100644 --- a/tests/test_litellm/integrations/test_opentelemetry.py +++ b/tests/test_litellm/integrations/test_opentelemetry.py @@ -1,3 +1,4 @@ +import pytest import asyncio import json import os @@ -5142,3 +5143,183 @@ class TestOpenTelemetryPreprocessingDuration(unittest.TestCase): span, exp = self._span() otel.set_preprocessing_duration_attribute(span, None) assert "litellm.preprocessing.duration_ms" not in self._attr(span, exp) + + +# --------------------------------------------------------------------------- +# LIT-3299: _emit_once must accept list/dict/set scope elements without crashing +# --------------------------------------------------------------------------- + + +@pytest.fixture +def lit_3299_otel_handler(): + """Build a real OpenTelemetry handler with span emission disabled. + + Constructed via the real OpenTelemetry class so the test exercises the + actual ``_emit_once`` / ``_make_hashable`` path, not a stub. + """ + from unittest.mock import patch + from litellm.integrations.opentelemetry import OpenTelemetry, OpenTelemetryConfig + + with patch.object(OpenTelemetry, "_init_otel_logger_on_litellm_proxy"): + return OpenTelemetry(config=OpenTelemetryConfig(exporter="console")) + + +def test_lit_3299_emit_once_handles_list_guardrail_mode(lit_3299_otel_handler): + """LIT-3299: a list-valued guardrail_mode in scope must not crash. + + Before the fix this raised ``TypeError: unhashable type: 'list'`` at + ``spans_logged.get(dedupe_key)``, returning HTTP 500 from the proxy. + """ + kwargs: dict = {} + assert ( + lit_3299_otel_handler._emit_once( + kwargs, + "guardrail", + "my-guardrail", + 1.0, + ["pre_call", "post_call"], + ) + is True + ) + assert ( + lit_3299_otel_handler._emit_once( + kwargs, + "guardrail", + "my-guardrail", + 1.0, + ["pre_call", "post_call"], + ) + is False + ) + + +def test_lit_3299_list_and_tuple_collapse_to_same_slot(lit_3299_otel_handler): + """list and its equivalent tuple must dedupe to the same key. + + Without this guarantee a single guardrail invocation whose ``mode`` + varies in container type between calls would emit two spans rather + than one. + """ + kwargs: dict = {} + assert ( + lit_3299_otel_handler._emit_once( + kwargs, "guardrail", "g1", 1.0, ["pre_call", "post_call"] + ) + is True + ) + assert ( + lit_3299_otel_handler._emit_once( + kwargs, "guardrail", "g1", 1.0, ("pre_call", "post_call") + ) + is False + ) + + +def test_lit_3299_handles_nested_list(lit_3299_otel_handler): + """Nested list of lists must also normalize (defensive).""" + kwargs: dict = {} + assert lit_3299_otel_handler._emit_once(kwargs, [["a", "b"], "c"]) is True + assert lit_3299_otel_handler._emit_once(kwargs, [["a", "b"], "c"]) is False + + +def test_lit_3299_handles_dict_scope(lit_3299_otel_handler): + """dict-valued scope element (e.g. arbitrary metadata) must not crash.""" + kwargs: dict = {} + assert lit_3299_otel_handler._emit_once(kwargs, {"k": "v", "m": ["x", "y"]}) is True + # Equivalent dict (different insertion order) collapses to same slot. + assert ( + lit_3299_otel_handler._emit_once(kwargs, {"m": ["x", "y"], "k": "v"}) is False + ) + + +def test_lit_3299_handles_set_scope(lit_3299_otel_handler): + """set-valued scope element must not crash and is order-independent.""" + kwargs: dict = {} + assert lit_3299_otel_handler._emit_once(kwargs, {"pre_call", "post_call"}) is True + assert lit_3299_otel_handler._emit_once(kwargs, {"post_call", "pre_call"}) is False + + +def test_lit_3299_make_hashable_returns_scalars_unchanged(): + """Already-hashable scalars pass through untouched.""" + from litellm.integrations.opentelemetry import OpenTelemetry as O + + for v in (None, "x", 1, 1.5, True, ("a", "b"), b"bytes"): + assert O._make_hashable(v) == v + + +def test_lit_3299_make_hashable_list_to_tuple(): + """list -> tuple with element order preserved and recursive.""" + from litellm.integrations.opentelemetry import OpenTelemetry as O + + assert O._make_hashable(["a", "b"]) == ("a", "b") + assert O._make_hashable([["a", "b"], "c"]) == (("a", "b"), "c") + + +def test_lit_3299_make_hashable_dict_to_sorted_pairs(): + """dict -> tuple of (k, normalized v) pairs sorted by repr(k).""" + from litellm.integrations.opentelemetry import OpenTelemetry as O + + assert O._make_hashable({"b": 2, "a": 1}) == (("a", 1), ("b", 2)) + assert O._make_hashable({"a": ["x", "y"]}) == (("a", ("x", "y")),) + + +def test_lit_3299_make_hashable_dict_with_mixed_type_keys(): + """Defensive: dict with non-comparable mixed-type keys still normalizes. + + A plain ``sorted(dict.items())`` would raise on Python 3 because str/int + aren't orderable. We sort by ``repr(key)`` instead so this path is + always safe. + """ + from litellm.integrations.opentelemetry import OpenTelemetry as O + + out = O._make_hashable({1: "a", "2": "b"}) + assert isinstance(out, tuple) + {out: True} # hashable -> usable as dict key + + +def test_lit_3299_make_hashable_set_to_frozenset(): + """set -> frozenset; order-independent and hashable.""" + from litellm.integrations.opentelemetry import OpenTelemetry as O + + assert O._make_hashable({"a", "b"}) == frozenset({"a", "b"}) + assert O._make_hashable(frozenset({"a", "b"})) == frozenset({"a", "b"}) + + +def test_lit_3299_distinguishes_different_list_modes(lit_3299_otel_handler): + """Sanity: two different list-valued modes must NOT collapse to one slot.""" + kwargs: dict = {} + assert ( + lit_3299_otel_handler._emit_once(kwargs, "guardrail", "g", ["pre_call"]) is True + ) + assert ( + lit_3299_otel_handler._emit_once( + kwargs, "guardrail", "g", ["pre_call", "post_call"] + ) + is True + ) + + +def test_lit_3299_make_hashable_tuple_with_unhashable_element(): + """Greptile P2 follow-up: a tuple whose elements are unhashable must + also normalize, not just top-level lists/dicts/sets.""" + from litellm.integrations.opentelemetry import OpenTelemetry as O + + # tuple-of-list -> tuple-of-tuple + assert O._make_hashable((["a", "b"], "c")) == (("a", "b"), "c") + # tuple-of-dict -> tuple-of-sorted-pairs + assert O._make_hashable(({"k": "v"},)) == ((("k", "v"),),) + # the result must itself be hashable + {O._make_hashable((["a"], {"b": "c"})): True} + + +def test_lit_3299_emit_once_handles_tuple_with_inner_list(lit_3299_otel_handler): + """A tuple-of-list passed as a scope element must not crash _emit_once.""" + kwargs: dict = {} + assert ( + lit_3299_otel_handler._emit_once(kwargs, "guardrail", "g", (["a", "b"], "c")) + is True + ) + assert ( + lit_3299_otel_handler._emit_once(kwargs, "guardrail", "g", (["a", "b"], "c")) + is False + )