mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-07 07:58:09 +00:00
fix(otel v2): set body on GenAI operation exception event to keep OTLP batches exportable
`GenAIEventRecorder.record_operation_exception` built the `Event` without a `body`, so `Event.body` defaulted to `None`. The OTLP protobuf encoder has no representation for `None` — `_encode_value` raises `Invalid type <class 'NoneType'> of value None` while serialising the batch. `BatchLogRecordProcessor._export_batch` catches the exception, clears the records, and returns `idx` as if they were exported. The result: one unencodable record destroys the entire batch, with no crash, no retry, and no operator signal beyond a traceback. Because `record_operation_exception` is the only emit site on the v2 logs signal, every batch on that pipeline contains only unencodable records. The logs signal never delivers anything at all — operators see GenAI exception events simply never arrive at the backend, while spans and metrics from the same proxies flow normally. The reproducer in the issue document shows 0/30-day log delivery across three real production environments running v1.95.0 / v1.96.2. This is the same root cause as the cycle 11 / #36759 fix for the v1 metrics/events path (`gen_ai.system=None`) — an OTel attribute / event field carrying a `None` that the OTLP encoder rejects. The v1 fix did not touch the v2 path: the two are in separate code trees (`litellm/integrations/opentelemetry.py` vs `litellm/integrations/otel/plumbing/events.py`) and the v2 path was byte-identical at the time of the cycle 11 work. Upstream OTel added a `None` branch to `_encode_value` in `opentelemetry-exporter-otlp-proto-common` 1.43.0. LiteLLM hard-pins `opentelemetry-api/-sdk/-exporter-otlp==1.28.0`, so as shipped this event can never be exported without a code change. Fix: pass `body=message` to the `Event` constructor. The exception message is the natural body for a WARN record and adds nothing the attributes don't already carry. The fix is one line of code plus 8 lines of comment cross-referencing the issue. Pin the fix with a new test file `tests/test_litellm/integrations/otel/test_otel_v2_event_recorder.py`: - `test_record_operation_exception_sets_non_none_body` constructs the recorder with a mocked `EventLogger`, drives the sync `record_operation_exception` call, captures the `Event` that was passed to `emit`, and asserts the body is a non-`None` string equal to the exception message. Pins the name, the `exception.type` attribute, the `exception.message` attribute, and the new body. - `test_record_operation_exception_with_stacktrace_still_sets_body` exercises the same code path with a non-`None` stacktrace and asserts the body is still set (the stacktrace goes on `exception.stacktrace`, the body is still the message). Both pins are verified to fail without the fix (`assert None is not None` — the captured `Event.body` is `None`) and pass with it. The test mocks `event_logger.emit` as a plain `MagicMock` (not `AsyncMock`) because the recorder is sync and calls `emit` without `await` — matching the issue's standalone repro and the pre-existing v2 emit site. The test docstring documents this. Fixes #36863.
This commit is contained in:
parent
2bb297efa0
commit
a250a33431
2 changed files with 120 additions and 0 deletions
|
|
@ -36,6 +36,15 @@ class GenAIEventRecorder:
|
|||
self.event_logger.emit(
|
||||
Event(
|
||||
name=GenAIEvent.OPERATION_EXCEPTION,
|
||||
# ``Event.body`` defaults to ``None``. The OTLP protobuf
|
||||
# encoder has no representation for ``None`` (``_encode_value``
|
||||
# raises ``Invalid type <class 'NoneType'>``) — which makes
|
||||
# ``BatchLogRecordProcessor._export_batch`` discard the
|
||||
# entire batch on the first unencodable record, including
|
||||
# healthy records batched alongside. The exception message
|
||||
# is the natural body for a WARN record and adds nothing
|
||||
# the attributes don't already carry. See #36863.
|
||||
body=message,
|
||||
timestamp=timestamp_ns,
|
||||
trace_id=span_context.trace_id,
|
||||
span_id=span_context.span_id,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,111 @@
|
|||
"""Regression pin for #36863: OTel v2 GenAI exception events must not leave
|
||||
`Event.body` as ``None`` — the OTLP protobuf encoder rejects ``None`` and
|
||||
discards the entire batch.
|
||||
|
||||
The v2 events path is separate from the v1 metrics/events path fixed in
|
||||
cycle 11 (#36759) and lives in `litellm/integrations/otel/plumbing/events.py`
|
||||
under the v2 directory tree (`litellm/integrations/otel/`).
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
pytest.importorskip("opentelemetry")
|
||||
|
||||
from unittest.mock import MagicMock # noqa: E402
|
||||
|
||||
from litellm.integrations.otel.plumbing.events import GenAIEventRecorder # noqa: E402
|
||||
|
||||
|
||||
class TestGenAIEventRecorderBodyNotNone:
|
||||
"""
|
||||
Regression pin for #36863.
|
||||
|
||||
`GenAIEventRecorder.record_operation_exception` previously built the
|
||||
`Event` without a `body`, which defaults to `None`. The OTLP protobuf
|
||||
encoder has no representation for `None` and raises
|
||||
`Invalid type <class 'NoneType'> of value None` during serialization.
|
||||
`BatchLogRecordProcessor._export_batch` catches the exception and
|
||||
silently discards the entire batch — every event batched alongside
|
||||
the failing one is lost too. Because `record_operation_exception` is
|
||||
the *only* emit site on the v2 logs signal, every batch on that
|
||||
pipeline contains only unencodable records and the signal never
|
||||
delivers anything at all.
|
||||
|
||||
Pin: `record_operation_exception` must pass a non-`None` `body` to
|
||||
the `Event` constructor. The exception message is the natural body
|
||||
for a WARN record and adds nothing the attributes don't already
|
||||
carry.
|
||||
"""
|
||||
|
||||
def _make_recorder(self) -> tuple[GenAIEventRecorder, MagicMock]:
|
||||
"""Construct a recorder with a MagicMock event_logger so the
|
||||
`emit` call can be captured and asserted on without needing the
|
||||
real OTel SDK's logger pipeline."""
|
||||
event_logger = MagicMock()
|
||||
recorder = GenAIEventRecorder(event_logger=event_logger)
|
||||
return recorder, event_logger
|
||||
|
||||
def test_record_operation_exception_sets_non_none_body(self):
|
||||
"""
|
||||
The regression pin: the Event emitted to the event_logger must
|
||||
have a non-None body. Before the fix the body defaulted to None,
|
||||
which the OTLP encoder rejects.
|
||||
|
||||
Note: the recorder is sync and calls ``self.event_logger.emit(...)``
|
||||
without ``await`` (matching the issue's standalone repro and the
|
||||
pre-existing v2 emit site), so the test mocks ``emit`` as a plain
|
||||
``MagicMock`` rather than an ``AsyncMock``.
|
||||
"""
|
||||
recorder, event_logger = self._make_recorder()
|
||||
|
||||
recorder.record_operation_exception(
|
||||
span_context=MagicMock(trace_id=1, span_id=2, trace_flags=0),
|
||||
error_type="BadRequestError",
|
||||
message="Provider returned 400: invalid top_p",
|
||||
stack_trace=None,
|
||||
timestamp_ns=1234567890,
|
||||
)
|
||||
|
||||
# The event_logger must have been called exactly once.
|
||||
assert event_logger.emit.call_count == 1, (
|
||||
"record_operation_exception must call event_logger.emit "
|
||||
f"exactly once; got call_count={event_logger.emit.call_count}"
|
||||
)
|
||||
# Capture the Event that was passed in.
|
||||
event = event_logger.emit.call_args.args[0]
|
||||
# The fix: body is no longer None. The OTLP encoder rejects
|
||||
# None; the natural body for a WARN record is the exception
|
||||
# message.
|
||||
assert event.body is not None, (
|
||||
"Event.body must not be None — the OTLP encoder rejects None "
|
||||
"and BatchLogRecordProcessor discards the entire batch "
|
||||
"(see #36863). Got: %r" % (event.body,)
|
||||
)
|
||||
assert event.body == "Provider returned 400: invalid top_p"
|
||||
# And the rest of the Event shape stays intact.
|
||||
assert event.name == "gen_ai.client.operation.exception"
|
||||
# Exception attributes are still required by the semconv and
|
||||
# should not have moved to body.
|
||||
assert event.attributes["exception.type"] == "BadRequestError"
|
||||
assert event.attributes["exception.message"] == "Provider returned 400: invalid top_p"
|
||||
|
||||
def test_record_operation_exception_with_stacktrace_still_sets_body(self):
|
||||
"""
|
||||
When the payload carries a stacktrace, the Event must still
|
||||
have a non-None body. Same regression pin, with the
|
||||
``exception.stacktrace`` attribute present too.
|
||||
"""
|
||||
recorder, event_logger = self._make_recorder()
|
||||
|
||||
recorder.record_operation_exception(
|
||||
span_context=MagicMock(trace_id=1, span_id=2, trace_flags=0),
|
||||
error_type="RateLimitError",
|
||||
message="rate limited",
|
||||
stack_trace="Traceback (most recent call last):\n ...",
|
||||
timestamp_ns=1234567890,
|
||||
)
|
||||
|
||||
event = event_logger.emit.call_args.args[0]
|
||||
assert event.body is not None
|
||||
assert event.body == "rate limited"
|
||||
assert event.attributes["exception.stacktrace"].startswith("Traceback")
|
||||
Loading…
Add table
Reference in a new issue