fix(metrics): record streamed calls by their final outcome - #772
Conversation
Signed-off-by: nachiketb <nachiketb@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review. WalkthroughChangesThe client now tracks routed request metrics through streamed response completion or drop. It records stream failures explicitly, updates terminal metric APIs, removes duplicate server error counting, and extends failed-stream metric tests. Stream-aware metrics
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to Streaming request metrics retain successful completion handling and record recognized stream failures without duplicate server error increments. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
A rabbit watched the stream run bright Comment |
What
Defer routed-call metrics until a streaming answer finishes or is dropped. Stream and decode errors count as failures, and an abandoned open stream cannot count as a success. Record each outcome once and remove the server's duplicate Prometheus error increment.
Why
Fixes SWITCH-1441. An HTTP 200 stream was counted as successful before a later in-band error arrived, contradicting
/v1/stats.Notes for reviewers
85 added production lines. Extends one existing HTTP regression with three assertions; no new test functions. Stream events and buffered responses remain unchanged. Metrics include the time spent consuming the stream.
The local HTTP regression failed on unchanged main because a failed stream incremented
switchyard_requests_total. It now verifies no success or successful-call latency is recorded, total errors increase, and the per-model error is counted exactly once. The reported bug no longer reproduces through the server.Validation: targeted server
streamtests (14 passed), buffered stats control (1 passed), client observabilitystreamtests (3 passed, including cancellation), and classifier accounting control (1 passed). Formatting, workspace clippy, and diff checks passed. Only local mock providers; no full suite or live provider calls.Landing note: this independent main-based PR overlaps #773 in
run.rs. The combined resolution calls the stream-outcome observer at each completion candidate and counts algorithm-supplied answers once. That resolution passed both regressions plus classifier-accounting and streaming-success controls in a temporary worktree.Summary by CodeRabbit