AlinsRan opened a new pull request, #13711: URL: https://github.com/apache/apisix/pull/13711
### Description Fixes #13698 `$apisix_upstream_response_time` and `$llm_time_to_first_token` changed unit depending on the upstream status code. The AI latency vars are only assigned on the success path (`ai-providers/base.lua`), in milliseconds. The 429/5xx early exit in `ai-proxy/base.lua` returns without setting them, so the log-phase fallback in `init.lua` fills `$apisix_upstream_response_time` from nginx `$upstream_response_time` — which is in **seconds**. For the same 0.2s upstream latency: ``` status=200 apisix_upstream_response_time=201 llm_time_to_first_token=201 status=500 apisix_upstream_response_time=0.200 llm_time_to_first_token=0 ``` A 1000x difference in the same log field, which breaks latency aggregation and alert rules. This assigns both vars on the early-exit paths using the same clock source the success path uses (`ctx.llm_request_start_time`), so an error response is comparable with a served one: | Path | `apisix_upstream_response_time` | `llm_time_to_first_token` | |---|---|---| | 429 / 5xx from upstream | ms | ms (time until the upstream answered) | | upstream responded with no body | ms | ms | | transport error (connect/timeout) | ms | stays `0` — no first token ever arrived | | request never sent (400) | unset | unset | | internal error in `pcall(do_request)` | unset | unset | `init.lua`'s fallback is deliberately left alone: it also serves non-AI routes, where mirroring nginx `$upstream_response_time` in seconds is the existing documented behaviour. After this change the fallback is simply unreachable for AI routes that reached the upstream. Because `llm_time_to_first_token` now carries a real value on errors, the prometheus `llm_latency` observation needed a `status < 400` guard — that histogram has no status label, and letting fast 429 rejections in would skew the percentiles used for capacity planning. One small side effect of the guard: a request where the upstream returned 200 but the body could not be parsed (plugin ends up returning 500) is now excluded from `llm_latency`; that sample was anomalous to begin with. **Behaviour change to note in the changelog**: on 429/5xx, `$apisix_upstream_response_time` goes from a seconds decimal to a milliseconds integer, and `$llm_time_to_first_token` from `0` to a real value. That is the unification the issue asks for, but anyone who wrote a dashboard specifically around the old error-path unit will need to adjust. ### Tests New `t/plugin/ai-proxy-latency-vars.t` covers 200 / 500 / 429 / transport error, plus a case where a 200 and a 500 happen in one nginx lifecycle. Verified it is discriminating: on master 4 of its subtests fail (the 500, 429, mixed-lifecycle and transport-error cases) and pass with this change, while the 200 case passes either way, pinning the success path against regression. Existing `t/plugin/ai-proxy*.t`, `t/plugin/ai-proxy-multi*.t` and `t/plugin/prometheus-ai-proxy.t` were run against this branch with no new failures. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
