diff --git a/crates/aisix-proxy/AGENTS.md b/crates/aisix-proxy/AGENTS.md index 412081d2..83e8ba38 100644 --- a/crates/aisix-proxy/AGENTS.md +++ b/crates/aisix-proxy/AGENTS.md @@ -30,11 +30,12 @@ opaque binary passthrough (audio, images) corrupts it. ## Every terminal path emits the access log — including the ones that give up early -The access log and `record_request` are emitted **by the handler**, at the end of -dispatch, because that is the only place that knows the provider, model and token -counts. A path that returns before reaching that tail therefore logs nothing, and -nothing errors: the caller gets a correct status while the gateway keeps no record -of the request, which is indistinguishable from the request never arriving. +The access log and `request_metrics::record` are emitted **by the handler**, at +the end of dispatch, because that is the only place that knows the provider, model +and token counts. A path that returns before reaching that tail therefore logs +nothing, and nothing errors: the caller gets a correct status while the gateway +keeps no record of the request, which is indistinguishable from the request never +arriving. Two shapes give up early, and both must answer through `reject::reject_before_dispatch` (it renders the envelope *and* emits the @@ -51,6 +52,27 @@ A handler that instead wraps its whole dispatch and logs the wrapper's status (`/mcp`, `/a2a`, `/passthrough`, `/v1/videos`, `/v1/files`) is already covered — don't add a second emit to those, or the request logs twice. +Emit the request metrics through `request_metrics::record` and nothing else. It +writes the legacy `aisix_requests_total` **and** the detailed `aisix_proxy_*` / +`aisix_llm_*` families from one call, so calling `Metrics::record_request` +directly silently produces a request that exists in one family and not the +others — the bug AISIX-Cloud#1234 fixed across ten endpoints. + +## A new proxy route has to be declared in three places + +Adding a `.route(…)` in `build_router` is not enough, and nothing fails loudly +if you stop there: + +1. `normalize_endpoint_label` — an unlisted path collapses to `"other"`, so the + route is invisible per-endpoint in every request series (how `/v1/videos` + shipped). +2. `request_metrics::LLM_ENDPOINTS` — decides whether the route counts as model + inference. Unlisted means proxy-only, which is the safe default but a silent + one. +3. The `ROUTES` table in `request_metrics`' tests — the only thing that makes + (1) and (2) fail loudly. It is a hand-maintained list of every route; a route + missing from it is a route the tests cannot check. + ## A per-model gate must say whether it binds the requested entry or each target `resolve_attempt_models` expands a routing model into targets, so `model_entry` / diff --git a/crates/aisix-proxy/src/a2a.rs b/crates/aisix-proxy/src/a2a.rs index 37c5805e..25133464 100644 --- a/crates/aisix-proxy/src/a2a.rs +++ b/crates/aisix-proxy/src/a2a.rs @@ -22,7 +22,7 @@ use std::time::{Duration, Instant}; use aisix_a2a::{upstream_from_a2a_agent, A2aBridge, A2aError, HttpBridge}; -use aisix_obs::{AccessLog, RequestOutcome, UsageEvent}; +use aisix_obs::{AccessLog, UsageEvent}; use axum::body::to_bytes; use axum::extract::{Request, State}; use axum::http::{header, HeaderMap, StatusCode}; @@ -65,6 +65,9 @@ pub async fn a2a_endpoint( .unwrap_or_else(new_request_id); let api_key_id = auth.entry.id.clone(); let http_method = request.method().clone(); + // `dispatch` takes the key by value; the terminal emit below still needs + // the caller's team / user labels (the handle is an `Arc` clone). + let caller_auth = auth.clone(); let response = dispatch(auth, &agent, &state, request, &request_id).await; @@ -91,11 +94,16 @@ pub async fn a2a_endpoint( routing_fallback_count: None, } .emit(); - state.metrics.record_request( - "a2a", - A2A_MODEL_LABEL, + crate::request_metrics::record( + &state, + "/a2a", + crate::request_metrics::Caller::new(&caller_auth), + crate::request_metrics::Upstream { + provider: "a2a", + model: A2A_MODEL_LABEL, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); response diff --git a/crates/aisix-proxy/src/audio.rs b/crates/aisix-proxy/src/audio.rs index 2ba576ee..a5203d2c 100644 --- a/crates/aisix-proxy/src/audio.rs +++ b/crates/aisix-proxy/src/audio.rs @@ -19,7 +19,7 @@ use aisix_core::AppliedGuardrail; use aisix_gateway::{ChatMessage, ChatResponse, FinishReason, UsageStats}; -use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, RequestOutcome, UsageEvent}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, UsageEvent}; use axum::body::Bytes; use axum::extract::{Multipart, State}; use axum::http::{header, HeaderMap}; @@ -46,6 +46,9 @@ struct AudioDispatchSuccess { /// Resolved ProviderKey UUID — feeds the per-PK telemetry attribution /// tags on the emitted UsageEvent (AISIX-Cloud#867 parity). provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234 parity with chat / messages / responses). + upstream_model: String, /// `(prompt_tokens, completion_tokens)` from the upstream `usage` /// block when the model returns one (gpt-4o-transcribe). `None` for /// whisper-1 (no usage block) — those still emit a zero-token event @@ -139,11 +142,12 @@ pub async fn transcriptions( &request_id, None, ); - state.metrics.record_request( - &success.provider, - &success.model_name, + record_audio_metrics( + &state, + "/v1/audio/transcriptions", + &auth, + &success, status, - RequestOutcome::from_status(status), elapsed, ); emit_audio_usage( @@ -171,11 +175,14 @@ pub async fn transcriptions( &request_id, Some(&err), ); - state.metrics.record_request( - "unknown", - "unknown", + // The model is never extracted from the multipart form on this + // path, so every label but the caller's stays `unknown`. + crate::request_metrics::record( + &state, + "/v1/audio/transcriptions", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream::default(), status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs. The model @@ -256,11 +263,12 @@ pub async fn translations( &request_id, None, ); - state.metrics.record_request( - &success.provider, - &success.model_name, + record_audio_metrics( + &state, + "/v1/audio/translations", + &auth, + &success, status, - RequestOutcome::from_status(status), elapsed, ); emit_audio_usage( @@ -288,11 +296,13 @@ pub async fn translations( &request_id, Some(&err), ); - state.metrics.record_request( - "unknown", - "unknown", + // Same as transcriptions: nothing resolved off the multipart form. + crate::request_metrics::record( + &state, + "/v1/audio/translations", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream::default(), status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs (model not @@ -351,33 +361,32 @@ pub async fn speech( .to_string(); match speech_dispatch(&state, &auth, body, &request_id, &client).await { - Ok(( - resp, - provider, - model_id, - provider_key_id, - applied_guardrails, - redactions, - monitor_hits, - captured, - )) => { + Ok(success) => { let elapsed = started.elapsed(); + let status = success.response.status().as_u16(); emit_access_log( "POST", "/v1/audio/speech", &model_name, - &provider, + &success.provider, &api_key_id, - 200, + status, elapsed, &request_id, None, ); - state.metrics.record_request( - &provider, - &model_name, - 200, - RequestOutcome::Success, + crate::request_metrics::record( + &state, + "/v1/audio/speech", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, + status, elapsed, ); // Issue #406: /v1/audio/speech (TTS) returns binary audio @@ -388,12 +397,12 @@ pub async fn speech( emit_usage_event( &state, &request_id, - &model_id, + &success.model_id, &model_name, &api_key_id, - &provider_key_id, - &applied_guardrails, - 200, + &success.provider_key_id, + &success.applied_guardrails, + status, elapsed, 0, 0, @@ -401,12 +410,12 @@ pub async fn speech( // the audio it produced — no duration cost basis here. 0.0, &client, - redactions, - monitor_hits, + success.redactions, + success.monitor_hits, /* guardrail_blocked */ false, - captured.as_ref(), + success.captured_content.as_ref(), ); - resp + success.response } Err(err) => { let status = err.status().as_u16(); @@ -424,11 +433,15 @@ pub async fn speech( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/audio/speech", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs with a @@ -858,6 +871,7 @@ async fn multipart_dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: upstream_model.clone(), usage, duration_seconds, applied_guardrails, @@ -903,6 +917,7 @@ async fn multipart_dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model, usage, duration_seconds, applied_guardrails, @@ -950,25 +965,30 @@ fn speech_input_to_chat(model: &str, body: &Value) -> aisix_gateway::ChatFormat } #[allow(clippy::type_complexity)] +/// `/v1/audio/speech`'s dispatch result. TTS reports no usage block, so +/// this carries only what the terminal emit needs — a struct rather than +/// the tuple it used to be, matching `AudioDispatchSuccess` above. +struct SpeechDispatchSuccess { + response: Response, + provider: String, + model_id: String, + provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234). + upstream_model: String, + applied_guardrails: Vec, + redactions: crate::redact::RedactionCounts, + monitor_hits: Vec, + captured_content: Option, +} + async fn speech_dispatch( state: &ProxyState, auth: &AuthenticatedKey, mut body: Value, request_id: &str, client_ctx: &ClientContext, -) -> Result< - ( - Response, - String, - String, - String, - Vec, - crate::redact::RedactionCounts, - Vec, - Option, - ), - ProxyError, -> { +) -> Result { let model_name = body .get("model") .and_then(|v| v.as_str()) @@ -1063,7 +1083,7 @@ async fn speech_dispatch( // Rewrite model field. if let Some(m) = body.get_mut("model") { - *m = Value::String(upstream_model); + *m = Value::String(upstream_model.clone()); } // Apply the PK's `request.*` overrides (body + headers) like the OpenAI @@ -1195,16 +1215,17 @@ async fn speech_dispatch( let mut out = axum::response::Response::new(axum::body::Body::from(body_bytes)); copy_response_header(&upstream_headers, &mut out, header::CONTENT_TYPE); - Ok(( - out, - provider_label, - model_entry.id.to_string(), - pk_entry.id.to_string(), + Ok(SpeechDispatchSuccess { + response: out, + provider: provider_label, + model_id: model_entry.id.to_string(), + provider_key_id: pk_entry.id.to_string(), + upstream_model, applied_guardrails, redactions, monitor_hits, captured_content, - )) + }) } /// Pull `(prompt_tokens, completion_tokens)` from an audio response @@ -1305,6 +1326,33 @@ fn probe_audio_duration_seconds(audio: &[u8]) -> Option { (seconds > 0.0).then_some(seconds) } +/// Terminal request-metric emit for the two transcription-shaped routes, +/// which share `AudioDispatchSuccess` and would otherwise repeat the same +/// label set twice. +fn record_audio_metrics( + state: &ProxyState, + endpoint: &'static str, + auth: &AuthenticatedKey, + success: &AudioDispatchSuccess, + status: u16, + elapsed: Duration, +) { + crate::request_metrics::record( + state, + endpoint, + crate::request_metrics::Caller::new(auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &success.model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, + status, + elapsed, + ); +} + /// Emit a UsageEvent for a successful transcription/translation. Tokens /// come from the upstream `usage` block when present (gpt-4o-transcribe); /// zero otherwise (whisper-1) — the request is still visible/attributed. diff --git a/crates/aisix-proxy/src/chat.rs b/crates/aisix-proxy/src/chat.rs index 06e122eb..f5bc61a0 100644 --- a/crates/aisix-proxy/src/chat.rs +++ b/crates/aisix-proxy/src/chat.rs @@ -21,8 +21,8 @@ use aisix_core::AppliedGuardrail; use aisix_gateway::{BridgeError, ChatFormat}; use aisix_guardrails::GuardrailVerdict; use aisix_obs::{ - content_capture_cap, AccessLog, CapturedContent, LatencyLabels, LlmUsage, Metrics, - RequestLabels, RequestOutcome, UsageEvent, UsageLabels, + content_capture_cap, AccessLog, CapturedContent, LatencyLabels, LlmUsage, Metrics, UsageEvent, + UsageLabels, }; use axum::extract::State; use axum::http::HeaderValue; @@ -150,13 +150,10 @@ pub async fn chat_completions( }; let client_type = state.client_classifier.classify(&client.user_agent); record_success( - &state.metrics, + &state, + &auth, &success.provider, &model_name, - &api_key_id, - auth.key().team_id.as_deref(), - auth.key().user_id.as_deref(), - auth.key().user_name.as_deref(), &provider_key_name, client_type, req.is_streaming(), @@ -350,7 +347,6 @@ pub async fn chat_completions( // volume, not label cardinality). let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - record_error(&state.metrics, metric_model, status, elapsed); // Access log: surface the upstream-billed counts when the // error fired AFTER the upstream call (output-content-filter // block). Pre-upstream errors (input filter, budget, @@ -379,26 +375,19 @@ pub async fn chat_completions( // Provider / upstream_model / provider_key are unknown on the // failure path; identity + status + outcome + stream + is_fallback // are what the success-rate query needs. - let fail_labels = RequestLabels { - endpoint: "/v1/chat/completions", - inbound_protocol: "openai", - provider: "unknown", - model: metric_model, - upstream_model: "unknown", - provider_key_id: "unknown", - provider_key_name: "unknown", - api_key_id: &api_key_id, - team_id: auth.key().team_id.as_deref().unwrap_or("unknown"), - user_id: auth.key().user_id.as_deref().unwrap_or("unknown"), - user_name: auth.key().user_name.as_deref().unwrap_or("unknown"), - stream: req.is_streaming(), - is_fallback: routing.fallback_count() > 0, + crate::request_metrics::record( + &state, + "/v1/chat/completions", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + stream: req.is_streaming(), + is_fallback: routing.fallback_count() > 0, + ..Default::default() + }, status, - outcome: RequestOutcome::from_status(status), - }; - state - .metrics - .record_proxy_and_llm_request(fail_labels, elapsed); + elapsed, + ); state.metrics.record_request_e2e_latency( LatencyLabels { endpoint: "/v1/chat/completions", @@ -3699,14 +3688,11 @@ fn finish_reason_label(reason: &aisix_gateway::FinishReason) -> String { #[allow(clippy::too_many_arguments)] fn record_success( - metrics: &Metrics, + state: &ProxyState, + auth: &AuthenticatedKey, provider: &str, model: &str, - api_key_id: &str, - team_id: Option<&str>, - user_id: Option<&str>, // #890 req-3 readable name + req-4 client type + req-1/req-2 dimensions. - user_name: Option<&str>, provider_key_name: &str, client_type: &str, stream: bool, @@ -3715,26 +3701,23 @@ fn record_success( s: &Success, elapsed: Duration, ) { - let outcome = RequestOutcome::from_status(status); - metrics.record_request(provider, model, status, outcome, elapsed); - let request_labels = RequestLabels { - endpoint: "/v1/chat/completions", - inbound_protocol: "openai", - provider, - model, - upstream_model: &s.upstream_model, - provider_key_id: &s.provider_key_id, - provider_key_name, - api_key_id, - team_id: team_id.unwrap_or("unknown"), - user_id: user_id.unwrap_or("unknown"), - user_name: user_name.unwrap_or("unknown"), - stream, - is_fallback, + let metrics = &state.metrics; + let caller = crate::request_metrics::Caller::new(auth); + crate::request_metrics::record( + state, + "/v1/chat/completions", + caller, + crate::request_metrics::Upstream { + provider, + model, + upstream_model: &s.upstream_model, + provider_key_id: &s.provider_key_id, + stream, + is_fallback, + }, status, - outcome, - }; - metrics.record_proxy_and_llm_request(request_labels, elapsed); + elapsed, + ); // SLO e2e histogram (AISIX-Cloud#1011): non-streaming only here — // `elapsed` for a stream is time-to-response-start; the stream's // on_complete records the full duration instead. @@ -3762,10 +3745,10 @@ fn record_success( upstream_model: &s.upstream_model, provider_key_id: &s.provider_key_id, provider_key_name, - api_key_id, - team_id: team_id.unwrap_or("unknown"), - user_id: user_id.unwrap_or("unknown"), - user_name: user_name.unwrap_or("unknown"), + api_key_id: caller.api_key_id, + team_id: caller.team_id, + user_id: caller.user_id, + user_name: caller.user_name, }, LlmUsage { input_tokens: s.prompt_tokens.unwrap_or(0).min(u64::from(u32::MAX)) as u32, @@ -4156,15 +4139,6 @@ pub(crate) fn emit_mid_stream_failed_attempt( ); } -fn record_error(metrics: &Metrics, model: &str, status: u16, elapsed: Duration) { - let outcome = RequestOutcome::from_status(status); - // Provider is unknown for pre-dispatch errors (auth, 404, etc.). - metrics.record_request("unknown", model, status, outcome, elapsed); - // Rate-limit rejections are counted at the quota gate itself - // (`quota::reject`), which covers every endpoint and knows the - // offending layer — counting here again would double-book chat. -} - #[allow(clippy::too_many_arguments)] fn emit_access_log( method: &str, diff --git a/crates/aisix-proxy/src/completions.rs b/crates/aisix-proxy/src/completions.rs index 2b788835..f76bbca3 100644 --- a/crates/aisix-proxy/src/completions.rs +++ b/crates/aisix-proxy/src/completions.rs @@ -15,7 +15,7 @@ //! 8. Providers that don't support completions return 501. use aisix_gateway::{BridgeError, ChatMessage, ChatResponse, FinishReason, UsageStats}; -use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, RequestOutcome, UsageEvent}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, UsageEvent}; use axum::extract::State; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; @@ -44,6 +44,9 @@ struct CompletionDispatchSuccess { /// Resolved ProviderKey UUID — feeds per-PK telemetry attribution /// (AISIX-Cloud#867 parity). provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234 parity with chat / messages / responses). + upstream_model: String, /// Upstream-reported token counts. `None` on the 501 /// NotImplemented path (provider doesn't support completions) /// or on a 200 with no `usage` block (rare edge). Handler @@ -139,11 +142,18 @@ pub async fn completions( &request_id, None, ); - state.metrics.record_request( - &success.provider, - &model_name, + crate::request_metrics::record( + &state, + "/v1/completions", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Issue #403: emit UsageEvent so cp-api's budget ledger @@ -186,11 +196,15 @@ pub async fn completions( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/completions", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs with a @@ -482,6 +496,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), usage, redactions, monitor_hits, @@ -517,6 +532,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), usage, redactions, monitor_hits, @@ -533,6 +549,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), // No upstream call → no usage to attribute. Handler // gates emission on `usage.is_some()` so 501 stays // out of /logs noise (same convention as #402). diff --git a/crates/aisix-proxy/src/count_tokens.rs b/crates/aisix-proxy/src/count_tokens.rs index bbc5db3e..65cdcc1e 100644 --- a/crates/aisix-proxy/src/count_tokens.rs +++ b/crates/aisix-proxy/src/count_tokens.rs @@ -39,7 +39,7 @@ //! user-facing passthrough and hit the identical "route missing from //! the list" bug. -use aisix_obs::{AccessLog, RequestOutcome}; +use aisix_obs::AccessLog; use axum::extract::rejection::JsonRejection; use axum::extract::State; use axum::http::{HeaderName, HeaderValue}; @@ -97,26 +97,33 @@ pub async fn count_tokens( .to_string(); match dispatch(&state, &auth, &body, &request_id, &client).await { - Ok((resp, provider)) => { + Ok(success) => { let elapsed = started.elapsed(); - let status = resp.status().as_u16(); + let status = success.response.status().as_u16(); emit_access_log( &model_name, - &provider, + &success.provider, &api_key_id, status, elapsed, &request_id, None, ); - state.metrics.record_request( - &provider, - &model_name, + crate::request_metrics::record( + &state, + "/v1/messages/count_tokens", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); - resp + success.response } Err(err) => { let status = err.status().as_u16(); @@ -132,11 +139,15 @@ pub async fn count_tokens( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/messages/count_tokens", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Anthropic-shape envelope (#336) — count_tokens callers are @@ -146,13 +157,24 @@ pub async fn count_tokens( } } +/// What the winning attempt resolved. `/v1/messages/count_tokens` emits no +/// UsageEvent, so the only consumer is the request-metric label set — which +/// still has to match what chat / messages / responses report +/// (AISIX-Cloud#1234). +struct CountTokensSuccess { + response: Response, + provider: String, + upstream_model: String, + provider_key_id: String, +} + async fn dispatch( state: &ProxyState, auth: &AuthenticatedKey, body: &Value, request_id: &str, client: &ClientContext, -) -> Result<(Response, String), ProxyError> { +) -> Result { let snapshot = state.snapshot.load(); let model_name = body @@ -292,7 +314,7 @@ async fn dispatch( ) .await { - Ok(resp) => return Ok((resp, "anthropic".to_string())), + Ok(success) => return Ok(success), Err(e) => { let retryable = matches!( &e, @@ -344,7 +366,7 @@ async fn count_tokens_to_target( timeouts: crate::routing::TimeoutBudget, request_id: &str, client: &ClientContext, -) -> Result { +) -> Result { let mut body = body.clone(); let pk_entry = crate::dispatch::resolve_provider_key(snapshot, model)?; let api_key = crate::dispatch::require_api_key(&pk_entry.value, model)?; @@ -495,7 +517,13 @@ async fn count_tokens_to_target( .insert(HeaderName::from_static("x-aisix-request-id"), hv); } - Ok(resp) + Ok(CountTokensSuccess { + response: resp, + // The loop above only ever dispatches Anthropic targets. + provider: "anthropic".to_string(), + upstream_model, + provider_key_id: pk_entry.id.to_string(), + }) } fn emit_access_log( diff --git a/crates/aisix-proxy/src/embeddings.rs b/crates/aisix-proxy/src/embeddings.rs index 612a6f69..ff8b5e02 100644 --- a/crates/aisix-proxy/src/embeddings.rs +++ b/crates/aisix-proxy/src/embeddings.rs @@ -16,7 +16,7 @@ use aisix_core::AppliedGuardrail; use aisix_gateway::{BridgeError, ChatFormat, ChatMessage, EmbeddingRequest}; -use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, RequestOutcome, UsageEvent}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, UsageEvent}; use axum::extract::State; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; @@ -119,7 +119,12 @@ pub async fn embeddings( match dispatch(&state, &auth, body, &request_id, &client).await { Ok(success) => { let elapsed = started.elapsed(); - let status = 200u16; + // The actual response status, not a hardcoded 200: the 501 + // NotImplemented branch also returns `Ok(success)`, and calling + // it a 200 both mislabels the access log and books it as + // `outcome="success"` on the request metrics. Same fix #426 made + // for completions / responses / rerank. + let status = success.response.status().as_u16(); emit_access_log( &model_name, &success.provider, @@ -129,11 +134,18 @@ pub async fn embeddings( &request_id, None, ); - state.metrics.record_request( - &success.provider, - &model_name, + crate::request_metrics::record( + &state, + "/v1/embeddings", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, status, - RequestOutcome::Success, elapsed, ); // Issue #226: emit UsageEvent so cp-api's budget ledger @@ -185,11 +197,15 @@ pub async fn embeddings( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/embeddings", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs with a @@ -223,6 +239,9 @@ struct EmbedDispatchSuccess { /// Resolved ProviderKey UUID — feeds the per-PK telemetry attribution /// tags on the emitted UsageEvent (AISIX-Cloud#867 parity). provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234 parity with chat / messages / responses). + upstream_model: String, /// The `{kind, hook}` set of guardrails that governed this request (#379 /// parity) — surfaced on the emitted UsageEvent. applied_guardrails: Vec, @@ -462,6 +481,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), applied_guardrails: applied_guardrails.clone(), redactions: redactions.clone(), monitor_hits: monitor_hits.clone(), @@ -485,6 +505,7 @@ async fn dispatch( provider: provider.to_ascii_lowercase(), model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), applied_guardrails: applied_guardrails.clone(), redactions: redactions.clone(), monitor_hits: monitor_hits.clone(), diff --git a/crates/aisix-proxy/src/images.rs b/crates/aisix-proxy/src/images.rs index 5ec13b6b..c9d4f768 100644 --- a/crates/aisix-proxy/src/images.rs +++ b/crates/aisix-proxy/src/images.rs @@ -12,7 +12,7 @@ use aisix_core::AppliedGuardrail; use aisix_gateway::BridgeError; -use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, RequestOutcome, UsageEvent}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, UsageEvent}; use axum::extract::State; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; @@ -37,6 +37,9 @@ struct ImageDispatchSuccess { /// Resolved ProviderKey UUID — feeds per-PK telemetry attribution /// (AISIX-Cloud#867 parity). provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234 parity with chat / messages / responses). + upstream_model: String, /// The `{kind, hook}` set of guardrails that governed this request (#379 /// parity) — surfaced on the emitted UsageEvent. applied_guardrails: Vec, @@ -98,20 +101,33 @@ pub async fn image_generations( match dispatch(&state, &auth, body, &request_id, &client).await { Ok(success) => { let elapsed = started.elapsed(); + // The actual response status, not a hardcoded 200: the 501 + // NotImplemented branch also returns `Ok(success)`, and calling + // it a 200 both mislabels the access log and books it as + // `outcome="success"` on the request metrics. Same fix #426 made + // for completions / responses / rerank. + let status = success.response.status().as_u16(); emit_access_log( &model_name, &success.provider, &api_key_id, - 200, + status, elapsed, &request_id, None, ); - state.metrics.record_request( - &success.provider, - &model_name, - 200, - RequestOutcome::Success, + crate::request_metrics::record( + &state, + "/v1/images/generations", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, + status, elapsed, ); // Issue #407: emit UsageEvent so cp-api's budget ledger + @@ -159,11 +175,15 @@ pub async fn image_generations( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/images/generations", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs with a @@ -370,6 +390,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), applied_guardrails: applied_guardrails.clone(), usage, upstream_called: true, @@ -387,6 +408,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), applied_guardrails: applied_guardrails.clone(), usage: None, // No upstream call happened → handler skips emit. diff --git a/crates/aisix-proxy/src/jobs.rs b/crates/aisix-proxy/src/jobs.rs index 1777f1ff..21c7fc2d 100644 --- a/crates/aisix-proxy/src/jobs.rs +++ b/crates/aisix-proxy/src/jobs.rs @@ -53,7 +53,7 @@ use std::time::{Duration, Instant}; use aisix_core::models::model::Adapter; use aisix_core::resource::ResourceEntry; use aisix_core::{Model, ProviderKey}; -use aisix_obs::{AccessLog, RequestOutcome, UsageEvent}; +use aisix_obs::{AccessLog, UsageEvent}; use axum::body::Body; use axum::extract::{Multipart, Query, State}; use axum::http::{header, HeaderMap, HeaderValue, Method, StatusCode}; @@ -658,6 +658,8 @@ fn finish( monitor_hits: Vec, ) -> Response { let elapsed = started.elapsed(); + // `path` carries the real job/file id — bounded route template only. + let endpoint = crate::normalize_endpoint_label(&path); match result { Ok((mut resp, target)) => { let status = resp.status().as_u16(); @@ -671,11 +673,16 @@ fn finish( &request_id, None, ); - state.metrics.record_request( - target.provider_label(), - target.display_name(), + crate::request_metrics::record( + state, + endpoint, + crate::request_metrics::Caller::new(auth), + crate::request_metrics::Upstream { + provider: target.provider_label(), + model: target.display_name(), + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); emit_job_usage_event( @@ -706,11 +713,16 @@ fn finish( &request_id, Some(&err), ); - state.metrics.record_request( - "", - label, + crate::request_metrics::record( + state, + endpoint, + crate::request_metrics::Caller::new(auth), + crate::request_metrics::Upstream { + provider: "", + model: label, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); crate::usage_attr::emit_error_usage_event( diff --git a/crates/aisix-proxy/src/lib.rs b/crates/aisix-proxy/src/lib.rs index 1789d912..3218c4d6 100644 --- a/crates/aisix-proxy/src/lib.rs +++ b/crates/aisix-proxy/src/lib.rs @@ -57,6 +57,7 @@ mod redact; mod reject; mod render; mod request_id; +mod request_metrics; mod rerank; mod responses; mod responses_bridge; @@ -294,11 +295,16 @@ fn normalize_endpoint_label(path: &str) -> &'static str { "/v1/audio/transcriptions" => "/v1/audio/transcriptions", "/v1/audio/translations" => "/v1/audio/translations", "/v1/audio/speech" => "/v1/audio/speech", + "/v1/videos" => "/v1/videos", "/mcp" | "/mcp/" => "/mcp", "/v1/realtime" => "/v1/realtime", "/v1/files" => "/v1/files", "/v1/batches" => "/v1/batches", "/v1/fine_tuning/jobs" => "/v1/fine_tuning/jobs", + // `/v1/videos/:id` and `/v1/videos/:id/content` collapse together: + // the id is the only thing that varies and neither is worth its own + // series. + _ if path.starts_with("/v1/videos/") => "/v1/videos/:id", _ if path.starts_with("/v1/files/") => "/v1/files/:id", _ if path.starts_with("/v1/batches/") => "/v1/batches/:id", _ if path.starts_with("/v1/fine_tuning/jobs/") => "/v1/fine_tuning/jobs/:id", @@ -309,6 +315,9 @@ fn normalize_endpoint_label(path: &str) -> &'static str { } } +/// Protocol family a route speaks, keyed off the normalized endpoint label. +/// Shared by the in-flight gauge and the detailed request families +/// (`request_metrics`) so the two can't disagree. fn inbound_protocol_for_endpoint(endpoint: &str) -> &'static str { if endpoint == "/v1/messages" || endpoint == "/v1/messages/count_tokens" { "anthropic" diff --git a/crates/aisix-proxy/src/mcp.rs b/crates/aisix-proxy/src/mcp.rs index 2d0e8a30..c7f6488f 100644 --- a/crates/aisix-proxy/src/mcp.rs +++ b/crates/aisix-proxy/src/mcp.rs @@ -17,7 +17,7 @@ use std::time::{Duration, Instant}; -use aisix_obs::{AccessLog, RequestOutcome, UsageEvent}; +use aisix_obs::{AccessLog, UsageEvent}; use axum::body::{to_bytes, Body}; use axum::extract::{Request, State}; use axum::http::StatusCode; @@ -103,20 +103,24 @@ async fn serve( .unwrap_or_else(new_request_id); let api_key_id = auth.entry.id.clone(); let method = request.method().clone(); + // `dispatch` takes the key by value; the terminal emit below still needs + // the caller's team / user labels (the handle is an `Arc` clone). + let caller_auth = auth.clone(); let response = dispatch(auth, scope.as_deref(), &state, request, &request_id).await; let elapsed = started.elapsed(); let status = response.status().as_u16(); + // Bounded route template, mirroring `/a2a` (the per-request server is + // on the usage event, not the access log). + let endpoint = if scope.is_some() { + "/mcp/{server}" + } else { + "/mcp" + }; AccessLog { method: method.as_str(), - // Bounded route template, mirroring `/a2a` (the per-request server is - // on the usage event, not the access log). - path: if scope.is_some() { - "/mcp/{server}" - } else { - "/mcp" - }, + path: endpoint, status, latency: elapsed, provider: Some("mcp"), @@ -136,11 +140,16 @@ async fn serve( routing_fallback_count: None, } .emit(); - state.metrics.record_request( - "mcp", - MCP_MODEL_LABEL, + crate::request_metrics::record( + &state, + endpoint, + crate::request_metrics::Caller::new(&caller_auth), + crate::request_metrics::Upstream { + provider: "mcp", + model: MCP_MODEL_LABEL, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); response diff --git a/crates/aisix-proxy/src/messages.rs b/crates/aisix-proxy/src/messages.rs index fa08321b..d2bc7610 100644 --- a/crates/aisix-proxy/src/messages.rs +++ b/crates/aisix-proxy/src/messages.rs @@ -37,8 +37,8 @@ use aisix_core::AppliedGuardrail; use aisix_obs::{ - content_capture_cap, AccessLog, CapturedContent, LatencyLabels, LlmUsage, RequestLabels, - RequestOutcome, UsageEvent, UsageLabels, + content_capture_cap, AccessLog, CapturedContent, LatencyLabels, LlmUsage, UsageEvent, + UsageLabels, }; use axum::extract::State; use axum::http::{HeaderName, HeaderValue}; @@ -179,37 +179,21 @@ pub async fn messages( &routing, None, ); - state.metrics.record_request( - &provider_label, - &model_name, + crate::request_metrics::record( + &state, + "/v1/messages", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &provider_label, + model: &model_name, + upstream_model: &upstream_model, + provider_key_id: &provider_key_id, + stream: stream_requested, + is_fallback: routing.fallback_count() > 0, + }, status, - RequestOutcome::from_status(status), elapsed, ); - let outcome = RequestOutcome::from_status(status); - // #890 req-3: readable provider-key name resolved from the snapshot. - let provider_key_name = { - let snap = state.snapshot.load(); - crate::usage_attr::provider_key_metric_name(&snap, &provider_key_id) - }; - let labels = RequestLabels { - endpoint: "/v1/messages", - inbound_protocol: "anthropic", - provider: &provider_label, - model: &model_name, - upstream_model: &upstream_model, - provider_key_id: &provider_key_id, - provider_key_name: &provider_key_name, - api_key_id: &api_key_id, - team_id: auth.key().team_id.as_deref().unwrap_or("unknown"), - user_id: auth.key().user_id.as_deref().unwrap_or("unknown"), - user_name: auth.key().user_name.as_deref().unwrap_or("unknown"), - stream: stream_requested, - is_fallback: routing.fallback_count() > 0, - status, - outcome, - }; - state.metrics.record_proxy_and_llm_request(labels, elapsed); // SLO e2e histogram (AISIX-Cloud#1011): non-streaming only — // a stream records its full duration at completion instead. if !stream_requested { @@ -308,36 +292,22 @@ pub async fn messages( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, - status, - RequestOutcome::from_status(status), - elapsed, - ); // #890 req-2: count the FAILED request on the rich request metrics // so a success rate is computable (denominator incl. failures). // Provider/upstream/provider_key are unknown on the failure path. - let fail_labels = RequestLabels { - endpoint: "/v1/messages", - inbound_protocol: "anthropic", - provider: "unknown", - model: metric_model, - upstream_model: "unknown", - provider_key_id: "unknown", - provider_key_name: "unknown", - api_key_id: &api_key_id, - team_id: auth.key().team_id.as_deref().unwrap_or("unknown"), - user_id: auth.key().user_id.as_deref().unwrap_or("unknown"), - user_name: auth.key().user_name.as_deref().unwrap_or("unknown"), - stream: stream_requested, - is_fallback: routing.fallback_count() > 0, + crate::request_metrics::record( + &state, + "/v1/messages", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + stream: stream_requested, + is_fallback: routing.fallback_count() > 0, + ..Default::default() + }, status, - outcome: RequestOutcome::from_status(status), - }; - state - .metrics - .record_proxy_and_llm_request(fail_labels, elapsed); + elapsed, + ); state.metrics.record_request_e2e_latency( LatencyLabels { endpoint: "/v1/messages", diff --git a/crates/aisix-proxy/src/passthrough.rs b/crates/aisix-proxy/src/passthrough.rs index 1d2ed83e..bc209a57 100644 --- a/crates/aisix-proxy/src/passthrough.rs +++ b/crates/aisix-proxy/src/passthrough.rs @@ -22,7 +22,7 @@ //! Standard proxy authentication applies (`Authorization: Bearer ` or //! `x-api-key`). No model-level authorisation is enforced beyond that. -use aisix_obs::{AccessLog, RequestOutcome}; +use aisix_obs::AccessLog; use axum::body::Body; use axum::extract::{Request, State}; use axum::http::{header, HeaderMap, HeaderValue, Method}; @@ -40,6 +40,31 @@ use crate::state::ProxyState; /// a label (unbounded Prometheus cardinality — #451). const PASSTHROUGH_MODEL_LABEL: &str = "passthrough"; +/// `provider` label for a passthrough request naming a provider nothing is +/// configured for. +const UNRESOLVED_PROVIDER_LABEL: &str = "unresolved"; + +/// Bound the `provider` metric label to the configured provider set. +/// `:provider` is a caller-supplied path segment, and the error path used +/// it verbatim — so `/passthrough//x` minted one series per random +/// value. Same guard as `usage_attr::metric_model_label`, on the provider +/// axis (the success path never needed it: `provider_label` there is the +/// resolved model's own provider). +fn provider_metric_label<'a>(snap: &aisix_core::AisixSnapshot, provider: &'a str) -> &'a str { + let configured = snap.models.entries().iter().any(|e| { + e.value + .provider + .as_deref() + .map(|p| p.eq_ignore_ascii_case(provider)) + .unwrap_or(false) + }); + if configured { + provider + } else { + UNRESOLVED_PROVIDER_LABEL + } +} + /// Headers that the passthrough endpoint ALWAYS strips before /// forwarding to upstream, regardless of customer configuration. /// @@ -167,15 +192,21 @@ pub async fn passthrough( &request_id, None, ); - state.metrics.record_request( - &provider_label, - // The raw `rest` wildcard is caller-controlled; using it as - // the `model` label would create unbounded metric - // cardinality. Passthrough has no resolved model, so record - // a fixed sentinel (#451). - PASSTHROUGH_MODEL_LABEL, + crate::request_metrics::record( + &state, + "/passthrough/:provider/*rest", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &provider_label, + // The raw `rest` wildcard is caller-controlled; using it + // as the `model` label would create unbounded metric + // cardinality. Passthrough has no resolved model, so + // record a fixed sentinel (#451). + model: PASSTHROUGH_MODEL_LABEL, + provider_key_id: &provider_key_id, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // #699: record the passthrough call in the UsageEvent stream — @@ -207,11 +238,17 @@ pub async fn passthrough( &request_id, Some(&err), ); - state.metrics.record_request( - &provider, - PASSTHROUGH_MODEL_LABEL, + let snap = state.snapshot.load(); + crate::request_metrics::record( + &state, + "/passthrough/:provider/*rest", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: provider_metric_label(&snap, &provider), + model: PASSTHROUGH_MODEL_LABEL, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // #699 / #655 parity: surface the failed request in Logs with a diff --git a/crates/aisix-proxy/src/realtime.rs b/crates/aisix-proxy/src/realtime.rs index 9538e45d..607a4c6c 100644 --- a/crates/aisix-proxy/src/realtime.rs +++ b/crates/aisix-proxy/src/realtime.rs @@ -43,7 +43,7 @@ use std::collections::HashMap; use std::time::{Duration, Instant}; use aisix_core::models::model::Adapter; -use aisix_obs::{AccessLog, RequestOutcome, UsageEvent}; +use aisix_obs::{AccessLog, UsageEvent}; use axum::extract::ws::{CloseFrame, Message as AxMessage, WebSocket, WebSocketUpgrade}; use axum::extract::{Query, State}; use axum::http::{HeaderMap, Method}; @@ -147,12 +147,17 @@ pub(crate) async fn realtime( // Count the refusal like every other pre-dispatch rejection // (unresolved labels, same as `reject_before_dispatch`) — logs // and the request-rate metrics must not disagree about whether - // these requests exist. - state.metrics.record_request( - "unknown", - crate::usage_attr::UNRESOLVED_MODEL_LABEL, + // these requests exist. Authentication may not have run, so the + // caller is attributed only when a key was resolved. + crate::request_metrics::record( + &state, + "/v1/realtime", + crate::request_metrics::Caller::unattributed(None), + crate::request_metrics::Upstream { + model: crate::usage_attr::UNRESOLVED_MODEL_LABEL, + ..Default::default() + }, status, - RequestOutcome::from_status(status), started.elapsed(), ); crate::usage_attr::emit_error_usage_event( @@ -675,11 +680,16 @@ async fn run_session( Some((&provider_label, &requested_model)), session_error.as_ref(), ); - state.metrics.record_request( - &provider_label, - &model_entry.value.display_name, + crate::request_metrics::record( + &state, + "/v1/realtime", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &provider_label, + model: &model_entry.value.display_name, + ..Default::default() + }, close_status, - RequestOutcome::from_status(close_status), elapsed, ); diff --git a/crates/aisix-proxy/src/reject.rs b/crates/aisix-proxy/src/reject.rs index b4029d18..18519a78 100644 --- a/crates/aisix-proxy/src/reject.rs +++ b/crates/aisix-proxy/src/reject.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use std::time::Instant; use aisix_core::{ApiKey, ResourceEntry}; -use aisix_obs::{AccessLog, RequestOutcome}; +use aisix_obs::AccessLog; use axum::extract::FromRequestParts; use axum::http::request::Parts; use axum::response::{IntoResponse, Response}; @@ -85,11 +85,22 @@ pub(crate) fn reject_before_dispatch( error: Some(&error), } .emit(); - state.metrics.record_request( - UNRESOLVED_PROVIDER_LABEL, - UNRESOLVED_MODEL_LABEL, + // `path` must be normalized, not passed through: `AisixPath` below hands + // this the RAW `parts.uri.path()` so the access log can name the + // malformed segment, and that string is caller-controlled (#451). + // `request_metrics` keys the LLM-vs-proxy split off the result, so a 413 + // on /v1/chat/completions lands in the same families as the + // model-not-found the handler itself records. + crate::request_metrics::record( + state, + crate::normalize_endpoint_label(path), + crate::request_metrics::Caller::unattributed(api_key_id), + crate::request_metrics::Upstream { + provider: UNRESOLVED_PROVIDER_LABEL, + model: UNRESOLVED_MODEL_LABEL, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); match envelope { diff --git a/crates/aisix-proxy/src/request_metrics.rs b/crates/aisix-proxy/src/request_metrics.rs new file mode 100644 index 00000000..da7eaf5a --- /dev/null +++ b/crates/aisix-proxy/src/request_metrics.rs @@ -0,0 +1,304 @@ +//! The one chokepoint for the per-request outcome metrics every handler +//! emits at the end of dispatch. +//! +//! Three families ride on a single call: +//! +//! - `aisix_requests_total` / `aisix_request_duration_seconds` — the legacy +//! compatibility series, four labels, every endpoint. +//! - `aisix_proxy_requests_total` / `aisix_proxy_failed_requests_total` / +//! `aisix_proxy_request_duration_seconds` — the detailed series over ALL +//! proxied traffic. +//! - `aisix_llm_requests_total` / `aisix_llm_request_duration_seconds` — the +//! subset of the above that is a model-inference call, per +//! [`LLM_ENDPOINTS`]. +//! +//! Splitting the two tiers is the point: an MCP tool call, a batch-file +//! upload and a 413 are all proxy requests, but counting them as LLM +//! requests would corrupt every per-request token/cost average and the LLM +//! success rate. What is NOT a judgement call is that both tiers must cover +//! every endpoint — before AISIX-Cloud#1234 only chat + messages emitted the +//! detailed families at all, so ten endpoints were absent from the +//! success-rate and request-count queries built on them while still showing +//! up in the legacy series. +//! +//! Handlers call [`record`] instead of touching `Metrics` directly, and the +//! tier is decided from the endpoint rather than by the caller, so a new +//! endpoint cannot land with a half-wired label set — the same anti-drift +//! move `usage_attr` makes for the UsageEvent side. + +use std::time::Duration; + +use aisix_obs::{RequestLabels, RequestOutcome}; + +use crate::auth::AuthenticatedKey; +use crate::state::ProxyState; +use crate::usage_attr::provider_key_metric_name; + +/// Label value every `RequestLabels` field falls back to when the path +/// never resolved it. Matches `RequestLabels::default()`. +const UNKNOWN: &str = "unknown"; + +/// Caller identity for the detailed label set. +#[derive(Clone, Copy)] +pub(crate) struct Caller<'a> { + pub api_key_id: &'a str, + pub team_id: &'a str, + pub user_id: &'a str, + pub user_name: &'a str, +} + +impl<'a> Caller<'a> { + pub(crate) fn new(auth: &'a AuthenticatedKey) -> Self { + let key = auth.key(); + Self { + api_key_id: &auth.entry.id, + team_id: key.team_id.as_deref().unwrap_or(UNKNOWN), + user_id: key.user_id.as_deref().unwrap_or(UNKNOWN), + user_name: key.user_name.as_deref().unwrap_or(UNKNOWN), + } + } + + /// A path that gave up before it could attribute the request to a team + /// or user — the pre-dispatch rejections. `api_key_id` is `Some` once + /// the auth extractor has run and `None` for the middleware + /// short-circuits that precede it (see `reject`). + pub(crate) fn unattributed(api_key_id: Option<&'a str>) -> Self { + Self { + api_key_id: api_key_id.unwrap_or(UNKNOWN), + team_id: UNKNOWN, + user_id: UNKNOWN, + user_name: UNKNOWN, + } + } +} + +/// What the handler resolved about the upstream it reached, or tried to. +/// [`Upstream::default()`] is the shape of a request that failed before +/// resolution; a handler fills in only the fields its endpoint has. +#[derive(Clone, Copy)] +pub(crate) struct Upstream<'a> { + pub provider: &'a str, + /// MUST be bounded: a name that already resolved against the snapshot, + /// or `usage_attr::metric_model_label()` output on any path that can + /// fire before resolution. The raw client-supplied `model` is + /// attacker-controlled cardinality (#451). + pub model: &'a str, + pub upstream_model: &'a str, + pub provider_key_id: &'a str, + pub stream: bool, + pub is_fallback: bool, +} + +impl Default for Upstream<'_> { + fn default() -> Self { + Self { + provider: UNKNOWN, + model: UNKNOWN, + upstream_model: UNKNOWN, + provider_key_id: UNKNOWN, + stream: false, + is_fallback: false, + } + } +} + +/// Endpoints whose requests belong in the `aisix_llm_*` families on top of +/// the `aisix_proxy_*` ones — the model-inference routes. +/// +/// Values are `normalize_endpoint_label` outputs; `llm_endpoints_are_reachable` +/// pins that, because a typo here fails silently (the entry simply never +/// matches, and the endpoint quietly drops out of every LLM query). +/// +/// Deliberately absent, and why: +/// - `/mcp`, `/mcp/{server}`, `/a2a` — tool and agent calls, no model. +/// - `/passthrough/:provider/*rest` — an opaque tunnel; the gateway parses +/// nothing and cannot attribute a model. +/// - `/v1/files`, `/v1/batches`, `/v1/fine_tuning/jobs` — management calls. +/// - `/v1/realtime` — does reach a model, but feeds none of the +/// `aisix_llm_*_tokens_total` families (still chat + messages only), so +/// counting it here would inflate the denominator of every +/// tokens-per-request query. +const LLM_ENDPOINTS: &[&str] = &[ + "/v1/chat/completions", + "/v1/completions", + "/v1/embeddings", + "/v1/images/generations", + "/v1/messages", + "/v1/messages/count_tokens", + "/v1/rerank", + "/v1/responses", + "/v1/audio/transcriptions", + "/v1/audio/translations", + "/v1/audio/speech", + "/v1/videos", + "/v1/videos/:id", +]; + +/// Whether this endpoint's requests are model inference. +/// +/// Keyed off the route, not the call site, so a request lands in the same +/// families however it ended — a 413 refused before dispatch has to sit in +/// the same denominator as the model-not-found 404 the handler itself +/// records, or a success rate over the endpoint silently omits one of them. +/// +/// Anything unlisted is proxy-only, the safe default: a wrong `false` loses +/// a row from an LLM query, a wrong `true` corrupts every per-request token +/// and cost average built on these counters. +fn is_llm_endpoint(endpoint: &str) -> bool { + LLM_ENDPOINTS.contains(&endpoint) +} + +/// Terminal request-metric emit, shared by every handler. +/// +/// `endpoint` must be a bounded route template — a literal for the fixed +/// routes, or [`crate::normalize_endpoint_label`] output for the `:param` / +/// wildcard ones. Never a raw request path (#451). +pub(crate) fn record( + state: &ProxyState, + endpoint: &'static str, + caller: Caller<'_>, + upstream: Upstream<'_>, + status: u16, + elapsed: Duration, +) { + let outcome = RequestOutcome::from_status(status); + state + .metrics + .record_request(upstream.provider, upstream.model, status, outcome, elapsed); + // Held in a binding: `RequestLabels` borrows it. + let provider_key_name = { + let snap = state.snapshot.load(); + provider_key_metric_name(&snap, upstream.provider_key_id) + }; + let labels = RequestLabels { + endpoint, + // Derived from the endpoint rather than passed in, so the detailed + // families can't disagree with `aisix_proxy_in_flight_requests` + // about which protocol a route speaks. + inbound_protocol: crate::inbound_protocol_for_endpoint(endpoint), + provider: upstream.provider, + model: upstream.model, + upstream_model: upstream.upstream_model, + provider_key_id: upstream.provider_key_id, + provider_key_name: &provider_key_name, + api_key_id: caller.api_key_id, + team_id: caller.team_id, + user_id: caller.user_id, + user_name: caller.user_name, + stream: upstream.stream, + is_fallback: upstream.is_fallback, + status, + outcome, + }; + if is_llm_endpoint(endpoint) { + state.metrics.record_proxy_and_llm_request(labels, elapsed); + } else { + state.metrics.record_proxy_request(labels, elapsed); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Every registered proxy route, as its raw request path. Adding a route + /// to `build_router` without adding it here leaves the tests below + /// unable to see it — which is the point: the two assertions that follow + /// are what force a new endpoint's `endpoint` label and LLM-vs-proxy + /// tier to be decided rather than defaulted. + const ROUTES: &[&str] = &[ + "/v1/chat/completions", + "/v1/completions", + "/v1/embeddings", + "/v1/images/generations", + "/v1/messages", + "/v1/messages/count_tokens", + "/v1/rerank", + "/v1/responses", + "/v1/audio/transcriptions", + "/v1/audio/translations", + "/v1/audio/speech", + "/v1/videos", + "/v1/videos/vid_abc123", + "/v1/videos/vid_abc123/content", + "/v1/realtime", + "/v1/files", + "/v1/files/file_abc123", + "/v1/files/file_abc123/content", + "/v1/batches", + "/v1/batches/batch_abc123", + "/v1/batches/batch_abc123/cancel", + "/v1/fine_tuning/jobs", + "/v1/fine_tuning/jobs/ft_abc123", + "/mcp", + "/mcp/some-server", + "/a2a/some-agent", + "/passthrough/openai/v1/anything", + ]; + + /// No proxy route may fall through to the `"other"` bucket. A route that + /// does is invisible per-endpoint in every request series — which is how + /// `/v1/videos` shipped (AISIX-Cloud#1234): it was registered in + /// `build_router` but missing from the normalizer's allowlist, so all + /// video traffic reported `endpoint="other"`. + #[test] + fn every_route_has_its_own_endpoint_label() { + for route in ROUTES { + assert_ne!( + crate::normalize_endpoint_label(route), + "other", + "route {route} is missing from normalize_endpoint_label" + ); + } + } + + /// Guards against a typo in [`LLM_ENDPOINTS`]. An entry that no route + /// normalizes to can never match, and the failure is silent: the + /// endpoint just stops appearing in `aisix_llm_requests_total`, which is + /// indistinguishable from having no traffic. + #[test] + fn llm_endpoints_are_reachable() { + let reachable: Vec<&str> = ROUTES + .iter() + .map(|r| crate::normalize_endpoint_label(r)) + .collect(); + for endpoint in LLM_ENDPOINTS { + assert!( + reachable.contains(endpoint), + "no route normalizes to {endpoint} — dead entry in LLM_ENDPOINTS" + ); + } + } + + /// The tier split itself: the inference routes carry the LLM series, the + /// tool / management / tunnel surfaces carry only the proxy series. + #[test] + fn tiers_split_inference_from_the_rest() { + for route in [ + "/v1/chat/completions", + "/v1/responses", + "/v1/messages/count_tokens", + "/v1/embeddings", + "/v1/audio/speech", + "/v1/videos/vid_abc123/content", + ] { + assert!( + is_llm_endpoint(crate::normalize_endpoint_label(route)), + "{route} should count as an LLM request" + ); + } + for route in [ + "/mcp/some-server", + "/a2a/some-agent", + "/v1/realtime", + "/v1/batches/batch_abc123", + "/passthrough/openai/v1/anything", + "/livez", + ] { + assert!( + !is_llm_endpoint(crate::normalize_endpoint_label(route)), + "{route} must not count as an LLM request" + ); + } + } +} diff --git a/crates/aisix-proxy/src/rerank.rs b/crates/aisix-proxy/src/rerank.rs index 3c4128e1..ccc15a1a 100644 --- a/crates/aisix-proxy/src/rerank.rs +++ b/crates/aisix-proxy/src/rerank.rs @@ -10,7 +10,7 @@ //! The gateway appends `/v1/rerank`. use aisix_core::AppliedGuardrail; -use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, RequestOutcome, UsageEvent}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, UsageEvent}; use axum::extract::State; use axum::http::HeaderValue; use axum::response::{IntoResponse, Response}; @@ -35,6 +35,9 @@ struct RerankDispatchSuccess { /// Resolved ProviderKey UUID — feeds per-PK telemetry attribution /// (AISIX-Cloud#867 parity). provider_key_id: String, + /// Provider-side model name, for the `upstream_model` metric label + /// (AISIX-Cloud#1234 parity with chat / messages / responses). + upstream_model: String, /// The `{kind, hook}` set of guardrails that governed this request (#379 /// parity) — surfaced on the emitted UsageEvent. applied_guardrails: Vec, @@ -115,11 +118,18 @@ pub async fn rerank( &request_id, None, ); - state.metrics.record_request( - &success.provider, - &model_name, + crate::request_metrics::record( + &state, + "/v1/rerank", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Issue #405: emit UsageEvent so cp-api's budget ledger @@ -162,11 +172,15 @@ pub async fn rerank( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + crate::request_metrics::record( + &state, + "/v1/rerank", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655 parity: surface the failed request in Logs with a @@ -546,6 +560,7 @@ async fn dispatch( provider: provider_label, model_id: model_entry.id.to_string(), provider_key_id: pk_entry.id.to_string(), + upstream_model, applied_guardrails: applied_guardrails.clone(), usage, redactions, diff --git a/crates/aisix-proxy/src/responses.rs b/crates/aisix-proxy/src/responses.rs index 6e8f7d1c..92899b7f 100644 --- a/crates/aisix-proxy/src/responses.rs +++ b/crates/aisix-proxy/src/responses.rs @@ -13,9 +13,7 @@ //! 400 with an explanatory message. use aisix_gateway::{ChatFormat, ChatMessage, ChatResponse, FinishReason, UsageStats}; -use aisix_obs::{ - content_capture_cap, AccessLog, CapturedContent, LatencyLabels, RequestOutcome, UsageEvent, -}; +use aisix_obs::{content_capture_cap, AccessLog, CapturedContent, LatencyLabels, UsageEvent}; use axum::extract::State; use axum::http::{HeaderName, HeaderValue}; use axum::response::{IntoResponse, Response}; @@ -58,6 +56,11 @@ struct ResponseDispatchSuccess { /// pk_label / …) on the emitted UsageEvent (AISIX-Cloud#867). Empty when /// the target carried no provider_key_id. provider_key_id: String, + /// The provider-side model name the winning attempt actually called, + /// for the `upstream_model` metric label (AISIX-Cloud#1234). Same value + /// chat + messages report, so a query can group all three endpoints by + /// the model the provider was billed for rather than the alias. + upstream_model: String, /// Per-attempt routing telemetry (#655): the failed attempts that /// preceded the winner plus the winning attempt itself. routing: RoutingTelemetry, @@ -188,6 +191,13 @@ pub async fn responses( .and_then(|v| v.as_str()) .unwrap_or("") .to_string(); + // Read once here rather than off `body` at each terminal emit: dispatch + // never rewrites the field, and the failure paths must label the request + // with what the caller asked for. + let stream_requested = body + .get("stream") + .and_then(|v| v.as_bool()) + .unwrap_or(false); // Filled by `dispatch` with per-detector PII mask counts (#932); attached // to the terminal usage event on both the success and failure paths. @@ -224,11 +234,19 @@ pub async fn responses( &success.routing, None, ); - state.metrics.record_request( - &success.provider, - &model_name, + crate::request_metrics::record( + &state, + "/v1/responses", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + provider: &success.provider, + model: &model_name, + upstream_model: &success.upstream_model, + provider_key_id: &success.provider_key_id, + stream: stream_requested, + is_fallback: success.routing.fallback_count() > 0, + }, status, - RequestOutcome::from_status(status), elapsed, ); // Per #655: one zero-token UsageEvent per failed attempt that @@ -267,10 +285,7 @@ pub async fn responses( model: &model_name, provider: &success.provider, status, - streaming: body - .get("stream") - .and_then(|v| v.as_bool()) - .unwrap_or(false), + streaming: stream_requested, }, elapsed, ); @@ -327,11 +342,21 @@ pub async fn responses( ); let snap = state.snapshot.load(); let metric_model = crate::usage_attr::metric_model_label(&snap, &model_name); - state.metrics.record_request( - "unknown", - metric_model, + // The failed request counts on the detailed families too, so a + // success rate over /v1/responses has the failures in its + // denominator. Provider / upstream / provider-key never + // resolved on this path. + crate::request_metrics::record( + &state, + "/v1/responses", + crate::request_metrics::Caller::new(&auth), + crate::request_metrics::Upstream { + model: metric_model, + stream: stream_requested, + is_fallback: routing.fallback_count() > 0, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); state.metrics.record_request_e2e_latency( @@ -340,10 +365,7 @@ pub async fn responses( model: metric_model, provider: "unknown", status, - streaming: body - .get("stream") - .and_then(|v| v.as_bool()) - .unwrap_or(false), + streaming: stream_requested, }, elapsed, ); @@ -1311,6 +1333,7 @@ async fn responses_to_target( usage, model_id: model_id.to_string(), provider_key_id: provider_key_id.clone(), + upstream_model: upstream_model.clone(), routing: RoutingTelemetry::default(), guardrail_blocked: false, usage_handled_by_stream: false, @@ -1530,6 +1553,7 @@ async fn responses_to_target( usage: None, model_id: model_id.to_string(), provider_key_id, + upstream_model: upstream_model.clone(), routing: RoutingTelemetry::default(), guardrail_blocked: false, usage_handled_by_stream: true, @@ -1631,6 +1655,7 @@ async fn responses_to_target( usage, model_id: model_id.to_string(), provider_key_id: provider_key_id.clone(), + upstream_model: upstream_model.clone(), routing: RoutingTelemetry::default(), guardrail_blocked: true, usage_handled_by_stream: false, @@ -1672,6 +1697,7 @@ async fn responses_to_target( usage, model_id: model_id.to_string(), provider_key_id, + upstream_model, routing: RoutingTelemetry::default(), guardrail_blocked: false, usage_handled_by_stream: false, @@ -2007,6 +2033,7 @@ async fn responses_cross_provider_to_target( usage: None, model_id: model_id.to_string(), provider_key_id, + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), routing: RoutingTelemetry::default(), guardrail_blocked: false, usage_handled_by_stream: true, @@ -2110,6 +2137,7 @@ async fn responses_cross_provider_to_target( usage: Some(usage), model_id: model_id.to_string(), provider_key_id: provider_key_id.clone(), + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), routing: RoutingTelemetry::default(), guardrail_blocked: true, usage_handled_by_stream: false, @@ -2162,6 +2190,7 @@ async fn responses_cross_provider_to_target( usage: Some(usage), model_id: model_id.to_string(), provider_key_id, + upstream_model: model.upstream_model().unwrap_or("unknown").to_string(), routing: RoutingTelemetry::default(), guardrail_blocked: false, usage_handled_by_stream: false, diff --git a/crates/aisix-proxy/src/videos.rs b/crates/aisix-proxy/src/videos.rs index 63eca549..566f28f9 100644 --- a/crates/aisix-proxy/src/videos.rs +++ b/crates/aisix-proxy/src/videos.rs @@ -70,7 +70,7 @@ //! design (poll traffic would flood /logs with no billing signal). use aisix_core::AppliedGuardrail; -use aisix_obs::{AccessLog, RequestOutcome, UsageEvent}; +use aisix_obs::{AccessLog, UsageEvent}; use axum::extract::State; use axum::http::{header, StatusCode}; use axum::response::{IntoResponse, Response}; @@ -1424,7 +1424,12 @@ struct Telemetry<'a> { state: &'a ProxyState, method: &'static str, path: String, - api_key_id: String, + /// The `endpoint` metric label — the same collapsed route template + /// `normalize_endpoint_label` produces, so the request families agree + /// with `aisix_proxy_in_flight_requests`. Coarser than `path`, which + /// keeps `/content` distinct for the access log. + endpoint: &'static str, + auth: &'a AuthenticatedKey, request_id: String, started: Instant, } @@ -1446,7 +1451,7 @@ impl Telemetry<'_> { latency: elapsed, provider: Some(provider).filter(|p| !p.is_empty()), model: Some(model_label), - api_key_id: Some(&self.api_key_id), + api_key_id: Some(&self.auth.entry.id), prompt_tokens: None, completion_tokens: None, total_tokens: None, @@ -1458,11 +1463,16 @@ impl Telemetry<'_> { error: error.as_deref(), } .emit(); - self.state.metrics.record_request( - provider, - model_label, + crate::request_metrics::record( + self.state, + self.endpoint, + crate::request_metrics::Caller::new(self.auth), + crate::request_metrics::Upstream { + provider, + model: model_label, + ..Default::default() + }, status, - RequestOutcome::from_status(status), elapsed, ); } @@ -1481,7 +1491,8 @@ pub async fn create_video( state: &state, method: "POST", path: "/v1/videos".to_string(), - api_key_id: auth.entry.id.clone(), + endpoint: "/v1/videos", + auth: &auth, request_id: client.request_id.clone(), started, }; @@ -1769,7 +1780,8 @@ pub async fn get_video( state: &state, method: "GET", path: "/v1/videos/:id".to_string(), - api_key_id: auth.entry.id.clone(), + endpoint: "/v1/videos/:id", + auth: &auth, request_id: client.request_id.clone(), started: Instant::now(), }; @@ -1820,7 +1832,10 @@ pub async fn video_content( state: &state, method: "GET", path: "/v1/videos/:id/content".to_string(), - api_key_id: auth.entry.id.clone(), + // Collapsed with the metadata route, matching how + // `normalize_endpoint_label` treats `/v1/files/:id/content`. + endpoint: "/v1/videos/:id", + auth: &auth, request_id: client.request_id.clone(), started: Instant::now(), }; diff --git a/tests/e2e/src/cases/request-metrics-endpoint-coverage-e2e.test.ts b/tests/e2e/src/cases/request-metrics-endpoint-coverage-e2e.test.ts new file mode 100644 index 00000000..5d5136b6 --- /dev/null +++ b/tests/e2e/src/cases/request-metrics-endpoint-coverage-e2e.test.ts @@ -0,0 +1,248 @@ +import { createHash } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + EtcdClient, + SeedClient, + spawnApp, + startOpenAiUpstream, + waitConfigPropagation, + type OpenAiUpstream, + type SpawnedApp, +} from "../harness/index.js"; + +// AISIX-Cloud#1234: `aisix_proxy_requests_total` / `aisix_llm_requests_total` +// and their duration histograms were emitted by the chat and messages +// handlers ONLY. Every other endpoint recorded just the legacy +// `aisix_requests_total`, so `/v1/responses` traffic (Codex and friends) was +// missing from every request-count and success-rate query built on the +// detailed families — while still appearing in the legacy one, which made +// the gap look like a query mistake rather than absent instrumentation. +// +// These specs pin the two halves of the fix: the inference endpoints reach +// the LLM families, and the non-inference proxy surfaces reach the proxy +// families WITHOUT being counted as LLM requests. + +const CALLER_PLAINTEXT = "sk-request-metrics-caller"; +const CALLER_KEY_HASH = createHash("sha256") + .update(CALLER_PLAINTEXT) + .digest("hex"); + +const MODEL = "reqmetrics-gpt"; + +describe("request metrics endpoint coverage e2e", () => { + let app: SpawnedApp | undefined; + let upstream: OpenAiUpstream | undefined; + let etcdReachable = false; + + beforeAll(async () => { + const etcd = new EtcdClient(); + etcdReachable = await etcd.ping(); + if (!etcdReachable) return; + + upstream = await startOpenAiUpstream({ nonStreamBody: responsesBody() }); + app = await spawnApp(); + const seed = new SeedClient(etcd, app.etcdPrefix); + + const pk = await seed.createProviderKey({ + display_name: `${MODEL}-pk`, + secret: "sk-mock", + api_base: `${upstream.baseUrl}/v1`, + }); + await seed.createModel({ + display_name: MODEL, + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pk.id, + }); + await seed.createApiKey({ + key_hash: CALLER_KEY_HASH, + allowed_models: [MODEL], + }); + }); + + afterAll(async () => { + await app?.exit(); + await upstream?.close(); + }); + + test("/v1/responses reaches the LLM request families", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + await waitConfigPropagation(async () => { + const probe = await postResponses(app!, { model: MODEL, input: "ready" }); + return probe.status === 200; + }); + + const { status } = await postResponses(app, { model: MODEL, input: "hi" }); + expect(status).toBe(200); + + const text = await scrape(app); + + // The reported bug: this series did not exist for /v1/responses at all. + expect( + seriesFor(text, "aisix_llm_requests_total", "/v1/responses"), + ).toContainEqual( + expect.stringContaining(`model="${MODEL}"`), + ); + expect( + seriesFor(text, "aisix_llm_requests_total", "/v1/responses").join("\n"), + ).toContain('status="200"'); + + // Same gap on the proxy tier and on both duration histograms. + expect( + seriesFor(text, "aisix_proxy_requests_total", "/v1/responses"), + ).not.toHaveLength(0); + expect( + seriesFor(text, "aisix_llm_request_duration_seconds_count", "/v1/responses"), + ).not.toHaveLength(0); + expect( + seriesFor( + text, + "aisix_proxy_request_duration_seconds_count", + "/v1/responses", + ), + ).not.toHaveLength(0); + + // The detailed label set is filled in, not left at the defaults — an + // `upstream_model="unknown"` here would mean the handler emitted the + // series without threading what it actually called. + const line = seriesFor( + text, + "aisix_llm_requests_total", + "/v1/responses", + )[0]; + expect(line).toContain('inbound_protocol="openai"'); + expect(line).toContain('provider="openai"'); + expect(line).toContain('upstream_model="gpt-4o-mini"'); + expect(line).toContain('outcome="success"'); + }, 30_000); + + test("a failed /v1/responses request lands in the same denominator", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + const { status } = await postResponses(app, { + model: "reqmetrics-no-such-model", + input: "hi", + }); + expect(status).toBeGreaterThanOrEqual(400); + + const text = await scrape(app); + const failed = seriesFor( + text, + "aisix_llm_requests_total", + "/v1/responses", + ).filter((l) => !l.includes('outcome="success"')); + expect(failed).not.toHaveLength(0); + // A model that never resolved must not put caller-supplied text into a + // label (#451) — it collapses to the fixed sentinel. + expect(failed.join("\n")).toContain('model="unresolved"'); + expect(failed.join("\n")).not.toContain("reqmetrics-no-such-model"); + + // Failures also raise the proxy-side failure counter the success-rate + // query divides by. + expect( + seriesFor(text, "aisix_proxy_failed_requests_total", "/v1/responses"), + ).not.toHaveLength(0); + }, 30_000); + + test("passthrough is counted as a proxy request but never as an LLM one", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + // An unconfigured provider: the request fails, which is the path that + // used to put the caller-supplied `:provider` segment straight into the + // `provider` label. + const res = await fetch( + `${app.proxyUrl}/passthrough/reqmetrics-bogus-provider/v1/anything`, + { + method: "POST", + headers: { + authorization: `Bearer ${CALLER_PLAINTEXT}`, + "content-type": "application/json", + }, + body: JSON.stringify({ hello: "world" }), + }, + ); + expect(res.status).toBeGreaterThanOrEqual(400); + + const text = await scrape(app); + const endpoint = "/passthrough/:provider/*rest"; + + expect(seriesFor(text, "aisix_proxy_requests_total", endpoint)).not.toHaveLength( + 0, + ); + // The tier split: a tunnelled request reaches no model, so it must stay + // out of the LLM families or every tokens-per-request average is wrong. + expect(seriesFor(text, "aisix_llm_requests_total", endpoint)).toHaveLength(0); + + // Cardinality: the bogus provider name must not have become a label. + expect(text).not.toContain("reqmetrics-bogus-provider"); + expect( + seriesFor(text, "aisix_proxy_requests_total", endpoint).join("\n"), + ).toContain('provider="unresolved"'); + }, 30_000); +}); + +async function postResponses( + app: SpawnedApp, + body: unknown, +): Promise<{ status: number }> { + const res = await fetch(`${app.proxyUrl}/v1/responses`, { + method: "POST", + headers: { + authorization: `Bearer ${CALLER_PLAINTEXT}`, + "content-type": "application/json", + }, + body: JSON.stringify(body), + }); + await res.text(); + return { status: res.status }; +} + +async function scrape(app: SpawnedApp): Promise { + const res = await fetch(`${app.metricsUrl}/metrics`); + expect(res.status).toBe(200); + return res.text(); +} + +/** Every sample line of `metric` carrying `endpoint=""`. */ +function seriesFor( + scrapeText: string, + metric: string, + endpoint: string, +): string[] { + return scrapeText + .split("\n") + .filter( + (line) => + line.startsWith(`${metric}{`) && + line.includes(`endpoint="${endpoint}"`), + ); +} + +function responsesBody() { + return { + id: "resp_reqmetrics", + object: "response", + created_at: Math.floor(Date.now() / 1000), + status: "completed", + model: "gpt-4o-mini", + output: [ + { + id: "msg_reqmetrics", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "hello" }], + }, + ], + usage: { input_tokens: 11, output_tokens: 13, total_tokens: 24 }, + }; +}