feat(telemetry): one agent_turn span per speech handle - #7143
Conversation
bc899e3 to
e3a6d53
Compare
1cd55fc to
cb30881
Compare
cb30881 to
8711b59
Compare
8711b59 to
fa6a7d9
Compare
fa6a7d9 to
8548b9c
Compare
8548b9c to
a8dd605
Compare
4d52f09 to
8f6a814
Compare
34962fe to
93d145c
Compare
93d145c to
18cc1b4
Compare
18cc1b4 to
c17f8b5
Compare
c17f8b5 to
5fe9604
Compare
98737dc to
1971bb7
Compare
1971bb7 to
1fc0e02
Compare
1fc0e02 to
301ed4e
Compare
301ed4e to
198ad41
Compare
198ad41 to
028b552
Compare
028b552 to
836bec6
Compare
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if isinstance(error, Exception): | ||
| trace_utils.record_exception(span, error) |
There was a problem hiding this comment.
🟡 Pipeline failures leave turns successful
When pipeline inference stores a failure in SpeechHandle._error, _mark_done() receives no error. The overall turn then closes successfully despite the failed reply.
| if isinstance(error, Exception): | |
| trace_utils.record_exception(span, error) | |
| effective_error = error or self._error | |
| if isinstance(effective_error, Exception): | |
| trace_utils.record_exception(span, effective_error) |
Was this helpful? React with 👍 or 👎 to provide feedback.
836bec6 to
c88ad15
Compare
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 2 new potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
There was a problem hiding this comment.
🟥 Turn failures can expose customer content
Failed reply tasks call _mark_done without their exception. The original traceback enters logs instead of the span's redaction-aware exception path.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (carry := discarded._take_agent_turn()) is not None: | ||
| successor._continue_agent_turn(carry, discarded=discarded) |
There was a problem hiding this comment.
🟡 Rapid preemptive retries duplicate turns
When replacement precedes the canceled reply task's first run, _continue_discarded_turn transfers nothing. Both tasks later create separate spans for one turn.
Prompt for agents
Handle preemptive replacements whose discarded SpeechHandle has not opened its agent_turn span yet. In livekit-agents/livekit/agents/voice/agent_activity.py, _continue_discarded_turn currently transfers only an existing span. A newly created task may not have run before another synchronous preemptive callback replaces it, so _take_agent_turn returns None. The canceled task can later enter _agent_turn and create one span while the successor creates another. Preserve the logical turn association across this pre-start state, or prevent the canceled handle from opening an independent span. Add coverage where replacement occurs before the discarded task first enters _agent_turn.
Was this helpful? React with 👍 or 👎 to provide feedback.
a5d279a to
b8b852f
Compare
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| span, self._agent_turn_span = self._agent_turn_span, None | ||
| if span is None or not span.is_recording(): | ||
| return | ||
| from ..telemetry import otel_metrics, utils as trace_utils | ||
|
|
||
| if isinstance(error, Exception): | ||
| trace_utils.record_exception(span, error) | ||
| if self._agent_turn_started_at is not None and self._agent_turn_agent_name is not None: | ||
| otel_metrics.record_invoke_agent_duration( | ||
| time.perf_counter() - self._agent_turn_started_at, | ||
| agent_name=self._agent_turn_agent_name, | ||
| ) | ||
| span.end() |
There was a problem hiding this comment.
🟡 Trace sampling disables duration metrics
When tracing drops an agent_turn, _end_agent_turn returns before recording its invoke-agent duration metric. Trace sampling therefore removes latency observations.
| span, self._agent_turn_span = self._agent_turn_span, None | |
| if span is None or not span.is_recording(): | |
| return | |
| from ..telemetry import otel_metrics, utils as trace_utils | |
| if isinstance(error, Exception): | |
| trace_utils.record_exception(span, error) | |
| if self._agent_turn_started_at is not None and self._agent_turn_agent_name is not None: | |
| otel_metrics.record_invoke_agent_duration( | |
| time.perf_counter() - self._agent_turn_started_at, | |
| agent_name=self._agent_turn_agent_name, | |
| ) | |
| span.end() | |
| span, self._agent_turn_span = self._agent_turn_span, None | |
| if span is None: | |
| return | |
| from ..telemetry import otel_metrics, utils as trace_utils | |
| if self._agent_turn_started_at is not None and self._agent_turn_agent_name is not None: | |
| otel_metrics.record_invoke_agent_duration( | |
| time.perf_counter() - self._agent_turn_started_at, | |
| agent_name=self._agent_turn_agent_name, | |
| ) | |
| if not span.is_recording(): | |
| return | |
| if isinstance(error, Exception): | |
| trace_utils.record_exception(span, error) | |
| span.end() |
Was this helpful? React with 👍 or 👎 to provide feedback.
652c975 to
b8b852f
Compare
b8b852f to
6bc2a3c
Compare
6bc2a3c to
6050ce8
Compare
A reply that calls a tool runs two generations (LLM steps) in two tasks; they were two agent_turn spans linked only by lk.parent_generation_id, so one response read as two turns. The speech handle now owns a single agent_turn for its whole life: the first reply task opens it, a follow-up generation continues it, and it ends with the speech in SpeechHandle._mark_done (recording the speech's error, if any, and the invoke_agent duration for the whole turn). Each generation is a 'generation' event on the span carrying its id and parent id; lk.generation_id names the latest one and lk.generation_count how many there were. Tool, inference and playout spans of every step nest under the one turn. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the successor A preemptive attempt invalidated at commit, or superseded by a newer attempt, was cancelled and its successor opened a second agent_turn for the same user turn. The discarded speech now detaches its open span and the successor continues it, with a preemptive_generation_discarded event; an attempt cancelled with no successor still ends as its own turn. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chenghao-mou
left a comment
There was a problem hiding this comment.
lgtm. One of the Devin comment seems valid when the task is only cancelled later:
`SpeechHandle._cancel()` does not immediately cancel the task. It:
1. Sets `_interrupt_fut`.
2. Leaves the task runnable.
3. Calls `task.cancel()` only after the five-second interruption timeout.
What
A reply that calls a tool runs two generations (LLM steps) in two tasks. They were two
agent_turnspans linked only bylk.parent_generation_id, so one response rendered as two turns. A speech handle is now exactly oneagent_turn.How
say) opensagent_turnunderagent_session; the follow-up generation after a tool call runs in a new task but continues the open span instead of opening a second one. It ends with the speech inSpeechHandle._mark_done, whichever step that was on, recording the speech's error (redaction-aware) and thegen_ai.invoke_agent.durationmetric for the whole turn.generationevent on the span with itslk.generation_idandlk.parent_generation_id. On the span itselflk.generation_idnames the latest generation and the newlk.generation_counthow many there were (more than one means tools ran before the final reply).lk.speech_idis set at creation.llm_node,function_tool,tts_node,realtime_inferenceandagent_speakingnest under the one turn.on_user_turn_completedchanged the transcript) hands its openagent_turnto that successor: the wasted generation stays visible under the one turn with apreemptive_generation_discardedevent, andlk.speech_idfollows the speech that answered. An attempt cancelled with no successor yet (the user resumed) still ends as its own turn. The queue-wait and interruption helpers tolerate a span already ended with the speech.Tests
tests/test_agent_turn_span.py: a tool-calling reply through the fake session yields oneagent_turnwith twogenerationevents, bothllm_nodes, the tool and the speech inside it, and the turn outlasting the last child; a plain reply is one generation. Existing coverage (test_coverage_spans,test_eou_wait_span,test_agent_session) passes unchanged.Stacked on #7137.
🤖 Generated with Claude Code