Add beta post-call CRM/analytics telemetry collector (#6664) - #6666
Add beta post-call CRM/analytics telemetry collector (#6664)#6666samanyugoyal2010 wants to merge 3 commits into
Conversation
|
|
d002ecd to
5b7a51b
Compare
Adds livekit.agents.beta.gtm_telemetry: an opt-in collector that builds a deterministic PostCallReport (transcript, tool executions, metrics summary) from an AgentSession, a signed webhook dispatcher with bounded retries, and pure HubSpot/Salesforce payload adapters. Reuses existing session internals (session.history, session.usage, session._recorded_events) instead of live event listeners, so attach() timing doesn't affect correctness and duplicate/out-of-order event delivery is a non-issue. Transcript and per-turn latency are sourced from ChatMessage/ChatMessage.metrics rather than the deprecated metrics_collected event. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B33WAJ4Y7n7iKXHPoFYDWm
5b7a51b to
684b08a
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Two bugs from review (@devin-ai-integration): - attach() never cleared _cached_report, so reusing a collector across a second call returned the first call's report. - detach() called session.off("close", self._on_close), but attach() had registered the listener via session.once(), which wraps the callback in an internal closure that .off() can't match — the handler kept firing after detach, and finalize() then raised (call before attach) inside the now-orphaned handler, only surfacing as a caught-and-logged error. Switch to session.on(...) so .off() removes the exact registered callable, and clear the cached report whenever attach() binds a new session. Co-Authored-By: samanyugoyal2010 <samanyu.aiprojects@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B33WAJ4Y7n7iKXHPoFYDWm
| if attempt < max_attempts: | ||
| delay_idx = min(attempt - 1, len(self._config.retry_backoff) - 1) | ||
| await self._sleep(self._config.retry_backoff[delay_idx]) | ||
|
|
||
| assert last_error is not None # loop always sets it before falling through | ||
| raise last_error |
There was a problem hiding this comment.
🟡 Webhook delivery crashes with an unexpected error when the retry delay list is left empty
The retry delay is looked up by index without checking the list is non-empty (self._config.retry_backoff[delay_idx] at livekit-agents/livekit/agents/beta/gtm_telemetry/webhook.py:162) before sleeping between attempts, so a configuration with no delays makes delivery blow up with an unrelated error instead of the documented delivery failure.
Impact: A post-call report configured with an empty retry-delay list fails with a confusing crash that callers catching the delivery error will not handle.
Index arithmetic on an empty backoff tuple, plus the negative max_retries path
WebhookConfig.retry_backoff is a plain tuple[float, ...] with no validation in _validate (livekit-agents/livekit/agents/beta/gtm_telemetry/webhook.py:50-58). With retry_backoff=() and max_retries >= 1, the first failed attempt reaches delay_idx = min(attempt - 1, len(()) - 1) = -1, and ()[-1] raises IndexError out of send(), bypassing WebhookDeliveryError.
Relatedly, max_retries=-1 makes max_attempts = 0, the loop body never runs, and assert last_error is not None (livekit-agents/livekit/agents/beta/gtm_telemetry/webhook.py:164) fires an AssertionError (or falls through to raise None under -O).
Both cases are cheap to prevent by validating retry_backoff non-empty (when max_retries > 0) and max_retries >= 0 in the model validator.
Prompt for agents
WebhookConfig accepts retry_backoff=() and negative max_retries with no validation (livekit-agents/livekit/agents/beta/gtm_telemetry/webhook.py, _validate model validator). In PostCallWebhookDispatcher.send, the backoff index computation min(attempt - 1, len(retry_backoff) - 1) yields -1 for an empty tuple and indexing raises IndexError, escaping the documented WebhookDeliveryError contract; a negative max_retries makes the attempt loop body never execute and trips the `assert last_error is not None` fallthrough. Add validation in WebhookConfig._validate rejecting max_retries < 0 and an empty retry_backoff when retries are enabled (or make send fall back to a default delay when the tuple is empty).
Was this helpful? React with 👍 or 👎 to provide feedback.
attach() only cleared _cached_report when binding a genuinely different session object; a same-object restart (aclose() then start() again on the same AgentSession, which resets _started_at/_recorded_events) never re-invokes attach(), so finalize()'s cache short-circuit had no way to notice call 2 had begun and kept returning call 1's report. Fingerprint the cache with session._started_at captured when it was built, and only trust the cache if the session is unreachable (detach()'d or GC'd, matching the existing documented "cache survives detach" contract) or the session's current _started_at still matches. Deliberately not using id(session._recorded_events) as the fingerprint: finalize() only ever holds a copy of that list, so the original is freed the instant a restart reassigns it, and CPython's list free-list routinely hands the freed block straight back out to the next same-size (empty) list — an id() match would be a realistic false positive in exactly this scenario, not just a theoretical one. Co-Authored-By: samanyugoyal2010 <samanyu.aiprojects@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B33WAJ4Y7n7iKXHPoFYDWm
2 participants
Adds livekit.agents.beta.gtm_telemetry: an opt-in collector that builds a deterministic PostCallReport (transcript, tool executions, metrics summary) from an AgentSession, a signed webhook dispatcher with bounded retries, and pure HubSpot/Salesforce payload adapters.
Reuses existing session internals (session.history, session.usage, session._recorded_events) instead of live event listeners, so attach() timing doesn't affect correctness and duplicate/out-of-order event delivery is a non-issue. Transcript and per-turn latency are sourced from ChatMessage/ChatMessage.metrics rather than the deprecated metrics_collected event.