fix(proxy): wire LLM latency metrics into the streaming response path (#63)
CI / CI (push) Successful in 34m13s
CI / CI (push) Successful in 34m13s
`NewLLMMetricsCapture`/`ResponseWriterWrapper` had **zero call sites**, so `llm_ttft_seconds`, `llm_itl_seconds` and `llm_tokens_total` were exported as HELP/TYPE stubs with no samples (verified against the live `/metrics`). These are the only per-request LLM latency series covering the Ollama models (`ornith:35b`, `qwen2.5:3b-instruct`), which expose no `/metrics` of their own — so those models had zero latency visibility. ## Change Wire the wrapper around the response writer in `ServeHTTP` for model-routed requests, and delete the unused `io.WriteCloser` duplicate that made the gap hard to spot (two near-identical implementations, neither constructed). ## Correctness details - **Only SSE responses are measured.** A non-streaming completion is written in one `Write`, which would otherwise be recorded as a TTFT sample equal to the whole request latency, with no ITL samples — plausible-looking but meaningless numbers. - **`requestStart` is captured at the top of `ServeHTTP`** so TTFT measures from request acceptance, not from when the upstream response is copied. - **Token count skips the `data: [DONE]` terminator.** - **`Unwrap()`** keeps `http.ResponseController` (Flush/Hijack/deadlines) working through the wrapper, so SSE unbuffering (#33) is unaffected. ## Tests | Test | Guards | |---|---| | `TestResponseWriterWrapperRecordsSSEMetrics` | TTFT=1, ITL>0, tokens=2 (terminator excluded) | | `TestResponseWriterWrapperIgnoresNonStreaming` | JSON response records nothing | | `TestResponseWriterWrapperPreservesStreaming` | bytes + Flush pass through | | `TestHandlerRecordsLLMMetricsForStreaming` | end-to-end through `ServeHTTP` | The last one fails if the wrapper is ever wired nowhere again — verified by disabling the wiring locally and watching it fail. ## Verification needed post-merge Confirm samples appear: ``` curl -s https://api.riotpiao.com/metrics | grep -E "llm_(ttft|itl)_seconds_count|llm_tokens_total" ```
This commit was merged in pull request #63.
This commit is contained in:
+12
-1
@@ -314,6 +314,10 @@ func writeProblemDetail(w http.ResponseWriter, status int, problemType, title, d
|
||||
|
||||
// ServeHTTP implements http.Handler.
|
||||
func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
// Captured before any work so TTFT measures from the moment the gateway
|
||||
// accepted the request, not from when the upstream response is copied.
|
||||
requestStart := time.Now()
|
||||
|
||||
if h.handleAuthEndpoint(w, r) {
|
||||
return
|
||||
}
|
||||
@@ -358,7 +362,14 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
// For streaming responses (SSE, chunked), disable buffering. Issue #33.
|
||||
w.Header().Set("X-Accel-Buffering", "no")
|
||||
|
||||
proxy.ServeHTTP(w, r)
|
||||
// Capture per-request LLM latency metrics (TTFT/ITL/tokens) while the
|
||||
// response is copied from the upstream. Inert for non-SSE responses.
|
||||
var respWriter http.ResponseWriter = w
|
||||
if route.ModelName != "" {
|
||||
respWriter = NewResponseWriterWrapper(w, route.ModelName, observability.Global(), requestStart)
|
||||
}
|
||||
|
||||
proxy.ServeHTTP(respWriter, r)
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user