Repository navigation
fix(telemetry): record a LiteLlm streamed answer once, not chunks plus aggregate - #7694
ishan-1010 wants to merge 1 commit into
Conversation
…s aggregate LiteLlm streams chunks without a finish reason and then yields their aggregate with one. record_llm_response kept recording until that finish reason, so gen_ai.output.messages held every chunk and then the whole answer again (a 5-chunk answer was 6 messages, a streamed tool call 4 half-built calls plus the real one). When a complete response with a finish reason follows only partial chunks without one, drop the chunks' output messages and keep the aggregate. Streams whose last chunk carries the finish reason (Gemini) and streams that fail before their aggregate keep their chunks. Fixes google#7630
|
Thanks, Ishan—I’ve attached two supplemental native tests against
The full I’m not claiming a full telemetry-suite pass: the broader run hit a missing SOCKS dependency in the functional tests. The patch only changes tests and was AI-assisted. |
|
Hi Ishan — I’ve now ported the two supplemental tests from the attached patch onto your PR head ( |
|
@surajksharma07 added the v1/stable path to the description as out of scope. it logs 6 @GBX-Max1220 you can open a PR against |
|
Hi @ishan-1010, thank you for the contribution, This looks a lot like a cross-functional issue, having to do with LLM model implementations rather than just telemetry. Telemetry is only a symptom. From what I gathered, Gemini when streaming emits last meaningful chunk with the finish_reason set. I'm wondering if this can be fixed on the Model-level, so that both are compatible, instead of patching this in Telemetry @GWeale could you please look into this🙏 |
|
Hi @RKest, thanks for looking into this. You're right, telemetry is just where it shows up. Gemini puts the finish reason on its last chunk, so telemetry stops there. LiteLLM sends it in a separate empty chunk, so LiteLlm only has it on the aggregate at the end, and both get recorded. I can move the fix into LiteLlm, for example by holding each chunk back by one so the last one carries the finish reason like Gemini. One thing to check first: telemetry would then record one message per chunk, like it does for Gemini, so around 136 for the Groq example instead of 1. Is that what you'd want, or should a LiteLlm stream end up as one message? |
Well, I wanted to follow the example of https://github.com/open-telemetry/opentelemetry-python-genai which used to emit one message per chunk. In-hindsight this seems like a better approach I'll work on a comprehensive solution to this, because it looks very severe, as to my knowledge all models except Gemini are impacted. |
|
Sounds good, thanks. Should I close this once yours is up, or keep it as the LiteLlm part? |
Could you please verify this PR here fixes your issue: I'll try to have it merged later today or tomorrow. Monday at the latest |
… not its chunks The inference span recorded every streamed chunk as its own output message, ending at the first response with a finish reason. Model adapters put that finish reason in different places: Gemini on an empty partial chunk before its aggregate, so the span recorded the chunks; LiteLlm on the aggregate, so the span recorded the chunks and then the whole answer again. Both yield the whole answer as a `partial=False` aggregate after the `partial=True` chunks. Telemetry now skips the `partial=True` chunks and records that aggregate. A `BaseLlm` that leaves `partial` unset ran no aggregator, so its chunks are still recorded up to the finish reason, but now as one output message whose parts are the chunks' parts, as opentelemetry-instrumentation-google-genai reports a stream. Text parts are not joined: thought and answer text are both `text` parts in the output messages. Addresses the LiteLlm duplicate reported in #7694. Co-authored-by: Baixin Guo <141702260+GBX-Max1220@users.noreply.github.com> Co-authored-by: Max Ind <maxind@google.com> PiperOrigin-RevId: 1009687738
|
Tried it locally with LiteLLM, every case records one message now. Looks good, I'll close this one once yours is merged. my run |
… not its chunks The inference span recorded every streamed chunk as its own output message, ending at the first response with a finish reason. Model adapters put that finish reason in different places: Gemini on an empty partial chunk before its aggregate, so the span recorded the chunks; LiteLlm on the aggregate, so the span recorded the chunks and then the whole answer again. Both yield the whole answer as a `partial=False` aggregate after the `partial=True` chunks. Telemetry now skips the `partial=True` chunks and records that aggregate. A `BaseLlm` that leaves `partial` unset ran no aggregator, so its chunks are still recorded up to the finish reason, but now as one output message whose parts are the chunks' parts, as opentelemetry-instrumentation-google-genai reports a stream. Text parts are not joined: thought and answer text are both `text` parts in the output messages. Addresses the LiteLlm duplicate reported in #7694. Co-authored-by: Baixin Guo <141702260+GBX-Max1220@users.noreply.github.com> Co-authored-by: Max Ind <maxind@google.com> PiperOrigin-RevId: 1012105751
|
Hi @ishan-1010, #7815 is merged Thank you for your help with testing and the patience🙏 |
Link to Issue or Description of Change
Problem:
LiteLlm streams chunks without a finish reason and then yields their aggregate with one.
record_llm_responsekeeps recording until a finish reason arrives, sogen_ai.output.messagesheld every chunk and then the whole answer again.Solution:
As suggested in #7630: when a complete response with a finish reason follows only partial chunks without one, drop the chunks' output messages and keep the aggregate. Streams whose last chunk carries the finish reason (Gemini) and streams that fail before their aggregate keep their chunks.
Testing Plan
Unit Tests:
New tests in
tests/unittests/telemetry/test_instrumentation.py:test_aggregate_after_unfinished_chunks_is_recorded_once(fails on main: 3 messages)test_tool_call_aggregate_replaces_its_partial_call_chunks(fails on main: a call witharguments: nullplus the real one)test_stream_cut_off_before_its_aggregate_keeps_its_chunkstest_stream_whose_last_chunk_finishes_keeps_its_chunks(Gemini shape)Manual End-to-End (E2E) Tests:
gen_ai.output.messageson main vs this PR, LiteLlm withStreamingMode.SSE:openai/gpt-oss-20b, text answeropenai/gpt-oss-20b, tool calltool_callwitharguments: nulltool_call({"city": "Oslo"})Same counts in the completion log with
EVENT_ONLY.Checklist
Additional context
Out of scope: with the v1 schema (
ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=1) or stable semconv,trace_inference_resultemits onegen_ai.choicelog per response, and those are already emitted when the aggregate arrives. On the local SSE server that is 6 logs for a 5-chunk text answer and 5 for a streamed tool call, the same on main and on this branch. That needs a different fix.The silent cut-off from #7630 (a dropped stream is finalized as
stop) is tracked separately in #7693.