[router] Stream outcome observability for 2xx SSE streams by sherlockwu · Pull Request #38737 · sgl-project/sglang

sherlockwu · 2026-09-09T21:00:44Z

Summary

Add sgl_router_stream_outcome_total{worker_url, model_id, outcome} classifying every committed 2xx SSE stream as ok, stream_error_event, upstream_error, or client_disconnect. Non-2xx responses remain covered by responses_total; scope is documented in monitoring/README.md.
Detect in-band engine error events line-anchored per the SSE spec: a data: line whose payload's first JSON key is error. This tolerates framing variants (data: with no space, whitespace after {) and cannot match lookalike text inside event payloads. The scanner is a single pass per chunk with a bounded line buffer.
Keep the metric consistent with the circuit breaker: a transport failure outranks an in-band error event when labeling, and only transport failures affect the breaker. Routing behavior is unchanged.

Testing

cargo fmt --check and cargo clippy --all-targets -- -D warnings
cargo test --lib (557 passed) and cargo test --test proxy (88 passed), including new tests for SSE framing variants, scanner buffer bounds, and error-event-then-transport-failure classification.

CI States

Latest PR Test (Base): ✅ Run #34547251154
Latest PR Test (Extra): ❌ Run #34547250800
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.

…, access log Three observation-only additions; no routing or breaker behavior changes. sgl_router_stream_outcome_total{worker_url,model_id,outcome}: an engine commits 200 OK the moment it starts writing the SSE stream, so a failure after that point (queue-full shed, watchdog abort) is reported in-band as a data: {"error"...} event followed by a clean close — indistinguishable from success to every headers-time metric. The SSE pump's completion hook now carries a StreamEnd verdict (transport_ok / saw_inband_error / client_disconnect; in-band detection via an allocation-light line scanner matching sglang's create_streaming_error_response shape, parity-safe against models that merely talk about errors), and the chat handler classifies each 2xx stream as ok | inband_error | upstream_error | client_disconnect. The circuit breaker still judges transport_ok only. sgl_router_itl_seconds{model_id}: inter-token latency, from a new SSE-pump inter-chunk hook (gap between successive non-empty upstream chunk arrivals; 2xx-gated like TTFT). Bucket edges are copied verbatim from the engine's sglang:inter_token_latency_seconds grid so histogram_quantile comparisons between router and engine interpolate on the same buckets. Access log: exactly one http_request line per request (method, path, status, duration_ms) at the existing edge middleware, covering early-exit responses no handler sees; /healthz, /readyz and /metrics log at DEBUG so probe traffic doesn't bury API traffic. Ported from combine/router-admission-http2-loadaware, re-derived against main's current SSE pump and middleware rather than copied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

…er-consistent labels - Detect SSE error events per line (data: + first JSON key "error") instead of one exact byte string, so framing variants match and payload lookalikes cannot; scan is one pass with a bounded line probe. - Classify transport failure before saw_error_event so the metric label always agrees with the circuit breaker. - Drop the edge access log: it duplicated the chat_completions INFO log and recorded header-time status/duration for streams. - Document that stream_outcome_total covers committed 2xx streams only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The router already noticed client disconnects — #38737 even reports one as `StreamEnd::client_disconnect` — but never told the engine, so the engine kept generating on a dead connection and burned GPU until it hit its own token limit. Wire up an abort. Mint a `router-<x-request-id>-<uuid>` rid per plain-mode chat request and inject it into the forwarded body so the engine adopts it, then POST `/abort_request {rid, abort_all:false}` when the request is torn down early: - Streaming: the SSE pump's completion report decides. `engine_may_still_be_generating` spares only a stream drained to its clean end with the client still attached — the one shape that proves the engine stopped on its own. A `client_disconnect` means the reader went away mid-generation; a transport failure (which folds in a pump panic) means the router lost the connection, which is no evidence about the engine at all. `saw_error_event` is not consulted: an engine that reports its own failure as an SSE `data: {"error"…}` event still closes cleanly, and that clean close is what says it is done. - Non-streaming: an `AbortOnDrop` guard, disarmed once a complete response is in hand. A handler-future drop (client disconnect) or a stale-request-janitor timeout leaves it armed. The streaming arm takes the same guard for the pre-headers window, where no pump — and so no completion report — exists yet. - `abort_would_be_pointless` stands the guard down for a pre-dispatch failure (`BreakerOpen`, `WorkerMisconfigured`): the request never went out, so there is no rid to abort, and POSTing anyway would add load to the one worker the router just decided to stop using. A transport error or a timeout keeps it armed — either can leave the engine still generating. - The abort replays the request's own `Authorization`. SGLang marks `/abort_request` `ADMIN_OPTIONAL`, so an engine started with `--api-key` 401s an unauthenticated abort; without this the feature silently does nothing on exactly the deployments that secure their engines. A refused abort warns rather than logging at debug. - `send_abort` stays best-effort otherwise: 5s timeout, failures logged not propagated, and never circuit-breaker gated — an abort is a courtesy to the engine, not a verdict on the worker. Two cases opt out and keep today's behavior exactly: - PD-disaggregated mode. Prefill is deliberately detached so it outlives the client for KV-transfer correctness; aborting only the decode half mid-transfer is a riskier change, out of scope here. - A request that arrived with its own `rid`. The scheduler aborts every in-flight request whose rid *starts with* the one it is handed (`req.rid.startswith(recv_req.rid)`), so honouring a caller-chosen abort key would let `{"rid": "router-"}` cancel a worker's entire router-minted population on disconnect. Overwriting the caller's `rid` is not an option either — it is the handle they asked the engine to file the request under. That same prefix rule is why a minted rid ends in a fresh UUID rather than being the correlation id alone: two callers can pick colliding `x-request-id` values, but neither can predict the other's UUID, so one cannot steer an abort onto the other's request. The `x-request-id` half is what lets an operator take a rid out of an engine log line and find the caller's own request; it is dropped when absent, empty, over 64 chars, or outside an id alphabet. Note the minted rid is client-visible: SGLang reports a request's rid as `meta_info["id"]`, which is what the OpenAI response `id` carries, so a plain-mode response's `id` becomes this value instead of the engine-minted `uuid4().hex`. `RequestProbe` probes `rid` as `Option<IgnoredAny>` — presence only. SGLang accepts `rid` as a string *or* a list of strings, so typing it as `Option<String>` would turn a body the engine accepts today into a router-side 400. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ae21XCf9NZQ2tcZ68HHqnr

github-actions Bot added the documentation Improvements or additions to documentation label Sep 9, 2026

sherlockwu added router run-ci CI: run the baseline test suite on this PR labels Sep 9, 2026

sherlockwu force-pushed the port/router-stream-observability branch 2 times, most recently from bb62fcf to 2641ce7 Compare September 9, 2026 23:19

sherlockwu changed the title ~~[router] Streaming observability: stream-outcome truth, ITL histogram, access log~~ [router] Streaming observability: stream-outcome count, ITL histogram, access log Sep 10, 2026

sherlockwu changed the title ~~[router] Streaming observability: stream-outcome count, ITL histogram, access log~~ [router] Streaming observability: stream-outcome count, access log Sep 10, 2026

sherlockwu changed the title ~~[router] Streaming observability: stream-outcome count, access log~~ [router] Stream error observability and access logging Sep 10, 2026

sherlockwu changed the title ~~[router] Stream error observability and access logging~~ [router] Stream outcome observability and access logging Sep 10, 2026

sherlockwu changed the title ~~[router] Stream outcome observability and access logging~~ [router] Stream outcome observability for 2xx SSE streams Sep 10, 2026

sherlockwu and others added 5 commits September 10, 2026 16:35

refactor router stream observability

c0afe88

Co-authored-by: Cursor <cursoragent@cursor.com>

refine router stream outcome observability

67b516d

Co-authored-by: Cursor <cursoragent@cursor.com>

simplify router stream outcome tracking

9041522

Co-authored-by: Cursor <cursoragent@cursor.com>

sherlockwu force-pushed the port/router-stream-observability branch from f8787bf to 7a44da3 Compare September 10, 2026 16:36

Kangyan-Zhou reviewed Sep 10, 2026

View reviewed changes

Comment thread experimental/sgl-router/src/server/routes/chat.rs Outdated

sherlockwu changed the title ~~[router] Stream outcome observability for 2xx SSE streams~~ [Router] Stream outcome observability for 2xx SSE streams Sep 10, 2026

[router] Pin stream outcome precedence with regression tests

a6482f1

sherlockwu requested a review from Kangyan-Zhou September 11, 2026 01:54

Kangyan-Zhou approved these changes Sep 11, 2026

View reviewed changes

sherlockwu changed the title ~~[Router] Stream outcome observability for 2xx SSE streams~~ [sgl-router] Stream outcome observability for 2xx SSE streams Sep 14, 2026

sherlockwu merged commit f539c1f into main Sep 14, 2026
95 of 99 checks passed

sherlockwu deleted the port/router-stream-observability branch September 14, 2026 02:46

sherlockwu changed the title ~~[sgl-router] Stream outcome observability for 2xx SSE streams~~ [router] Stream outcome observability for 2xx SSE streams Sep 14, 2026

This was referenced Sep 14, 2026

[Router] Attribute every request's outcome and error status from the response #39460

Closed

[Router] Abort the engine when a client disconnects mid-request #39461

Merged

Navigation Menu

Repository navigation

[router] Stream outcome observability for 2xx SSE streams - #38737

[router] Stream outcome observability for 2xx SSE streams#38737
sherlockwu merged 6 commits into
mainfrom
port/router-stream-observability

sherlockwu commented Sep 9, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sherlockwu commented Sep 9, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

CI States

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sherlockwu commented Sep 9, 2026 •

edited by github-actions Bot

Loading