fix(llm): attribute fallback inference telemetry to providers - #2454
fix(llm): attribute fallback inference telemetry to providers#2454chasef07 wants to merge 1 commit into
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if (this.adapterSpanName !== undefined) { | ||
| this.#llmRequestSpan?.end(); | ||
| return; |
There was a problem hiding this comment.
🟡 Closed fallback streams retain orchestration spans
close() during a pending provider request leaves monitorMetrics blocked until the adapter queue closes. The abort signal closes only the output queue, while run and its child stream continue. The orchestration span outlives the canceled request, potentially through every provider timeout.
Prompt for agents
Fallback adapter cancellation does not propagate from LLMStream.close() to the active child stream in agents/src/llm/fallback_adapter.ts. monitorMetrics in agents/src/llm/llm.ts waits for the adapter queue to close before ending llm_fallback_adapter, so closing the consumer while a provider request is pending leaves the span and provider work active until completion or timeout. Update the adapter cancellation flow so close wakes the metrics monitor, terminates the orchestration span, and closes or aborts the current child stream without starting further fallback or recovery work.
Was this helpful? React with 👍 or 👎 to provide feedback.
Description
An LLM
FallbackAdaptercurrently reports a secondllm_requestandllm_metricsrecord alongside the real provider request. For example, one primary request using 10 input and 2 output tokens produces another identical usage record for modelFallbackAdapter, providerunknown. The enclosingllm_nodealso copies the wrapper model name into its inference attributes.Keep fallback orchestration visible while attributing inference and usage to the actual providers. Agent-start metadata describes the configured primary; individual request spans retain the actual primary or backup model used.
Changes Made
LLMStreamadapter-span hook and usellm_fallback_adapterfor the fallback stream. Forward its chunks without emitting another inference metrics record or duplicating GenAI attributes.llm_nodedoes not claim the configured wrapper as a model. Resolve nested adapters to their configured primary for agent-start metadata.Pre-Review Checklist
Testing
FallbackAdapterusage records and request spans before the fix. Voice-generation and agent-start tests separately reproduced the remaining wrapper attribution before its fix.pnpm test agents --silent: 2,499 passed, 5 skipped.pnpm --filter @livekit/agents buildpnpm --filter @livekit/agents typecheckpnpm --filter @livekit/agents api:checkpnpm -w lint:fixandpnpm format:check(existing lint warnings only).Additional Notes
FallbackAdapter.modelandlabel()retain their wrapper identity. Retry order and provider error behavior are unchanged. Consumers that counted wrapper-generated metrics will see those duplicate records disappear. Live Cloud analytics UI behavior has not been independently verified; this PR verifies the emitted telemetry and usage collector inputs.Note to reviewers: Please ensure the pre-review checklist is completed before starting your review.