feat: add AISIX-native Prometheus metrics - #331
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR expands AISIX AI Gateway observability by introducing Prometheus metrics configuration, new AISIX-native metric families, and request-level instrumentation across chat, Anthropic, and quota handlers. The ChangesPrometheus Metrics System Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-obs/src/metrics.rs`:
- Around line 194-212: The function record_proxy_request is recording proxy
latency into the LLM histogram constant M_LLM_REQUEST_DURATION; change it to use
the proxy-specific histogram constant M_PROXY_REQUEST_DURATION instead (in the
metrics::histogram! call and where .record(...) is applied) so proxy latencies
are emitted to the aisix_proxy_* metric family; update any references inside
record_proxy_request (labels and .record(duration.as_secs_f64())) to point to
M_PROXY_REQUEST_DURATION and ensure the histogram label set remains identical.
- Around line 165-191: The shared atomic inner.proxy_in_flight used in
increment_proxy_in_flight and decrement_proxy_in_flight causes cross-label
interference and can underflow the stored counter; replace it with a per-label
counter map (e.g., a Mutex<HashMap<(String,String), AtomicIsize>> or a
concurrent map like DashMap) keyed by (endpoint, inbound_protocol) and update
that specific counter in increment_proxy_in_flight/decrement_proxy_in_flight,
publishing its value with metrics::gauge!(M_PROXY_IN_FLIGHT, "endpoint"=>...,
"inbound_protocol"=>...). In decrement_proxy_in_flight ensure atomic decrement
is done safely (use fetch_update or a CAS loop on AtomicIsize to never drop
below zero, or remove the map entry when it reaches zero) so the stored state
cannot go negative and future increments/metrics remain correct.
In `@crates/aisix-proxy/src/lib.rs`:
- Around line 117-121: The in-flight gauge is decremented immediately after
calling next.run(request).await, but for streaming responses you must decrement
only when the response body finishes or is dropped; wrap the response body with
an RAII/body-wrapper that calls
state.metrics.decrement_proxy_in_flight(&endpoint, &inbound_protocol) from its
Drop (or when the body stream completes) so the gauge remains incremented for
the full lifetime of the SSE/stream; locate where next.run(request).await
returns the Response and replace its body with the wrapper (use the same
response variable name) so the decrement is tied to the wrapped body lifecycle
instead of executing immediately after next.run.
In `@tests/e2e/src/cases/prometheus-metrics-e2e.test.ts`:
- Around line 70-104: Add two additional e2e cases around the existing "scrape
contains AISIX-native request and token metrics" test: one that sets
observability.metrics.prometheus.enabled = false and asserts the /metrics
endpoint returns 404 (or is absent) and that scrape text does NOT contain the
aisix_* metrics, and another that sets observability.metrics.prometheus.path to
a custom path (e.g., /custom-metrics), uses waitConfigPropagation and
ProxyClient/chat probe exactly as the original test to ensure config propagated,
then fetches app.adminUrl + custom path and asserts status 200 and the same
metric names and labels are present; use the same helpers (ProxyClient,
waitConfigPropagation, app.adminUrl) and the existing expectation patterns so
these new tests cover the disabled and custom-path boundary/combination
scenarios.
- Around line 26-27: The test currently short-circuits when etcd/app/upstream
are unreachable by returning early (using etcdReachable from new
EtcdClient().ping()), which hides failures; remove the conditional skips and
instead make the availability checks fail fast: replace the early return that
uses etcdReachable with an assertion or thrown Error (e.g.,
assert(etcdReachable) or throw new Error(...)) that includes diagnostic info
from EtcdClient().ping(), and apply the same treatment to the other environment
checks referenced around the suite (the app/upstream reachability checks) so the
suite fails deterministically rather than silently skipping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e19aafb-1f98-49b3-bb4a-0b6c5b86b8ca
📒 Files selected for processing (13)
crates/aisix-admin/src/lib.rscrates/aisix-admin/src/state.rscrates/aisix-obs/src/lib.rscrates/aisix-obs/src/metrics.rscrates/aisix-proxy/src/budget.rscrates/aisix-proxy/src/chat.rscrates/aisix-proxy/src/lib.rscrates/aisix-proxy/src/messages.rscrates/aisix-proxy/src/quota.rscrates/aisix-server/src/main.rsdocs/configuration/bootstrap-config.mddocs/operations/metrics-and-logs.mdtests/e2e/src/cases/prometheus-metrics-e2e.test.ts
There was a problem hiding this comment.
Pull request overview
Adds a new set of AISIX-native Prometheus metric families (aisix_proxy_*, aisix_llm_*, etc.) while keeping the existing compatibility series, and makes the admin scrape endpoint configurable/disable-able via bootstrap config. The PR also wires additional request-path instrumentation (usage, TTFT, in-flight, rate-limit remaining, and optional budget gauges) and adds both unit and E2E coverage plus operator docs for the new metrics.
Changes:
- Introduces AISIX-native metric families in
aisix-obs, plus additional proxy instrumentation for usage, TTFT, in-flight requests, rate-limit remaining, and optional budget gauges. - Makes the admin Prometheus scrape endpoint configurable (
observability.metrics.prometheus.enabled/path) and adds admin router tests for path/disable behavior. - Adds operator documentation for metric names/labels and an E2E scrape test that drives real traffic and checks representative
aisix_*output.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/src/cases/prometheus-metrics-e2e.test.ts | New E2E test that drives traffic and validates representative Prometheus output. |
| docs/operations/metrics-and-logs.md | Documents Prometheus endpoint configurability and the new AISIX-native metric names/labels. |
| docs/configuration/bootstrap-config.md | Documents bootstrap config keys for Prometheus scrape endpoint enable/path. |
| crates/aisix-server/src/main.rs | Wires Prometheus config into AdminState. |
| crates/aisix-proxy/src/quota.rs | Emits optional budget gauges during quota enforcement when CP returns budget details. |
| crates/aisix-proxy/src/messages.rs | Records AISIX-native LLM usage metrics for the Anthropic /v1/messages path. |
| crates/aisix-proxy/src/lib.rs | Adds middleware to track proxy in-flight requests by endpoint/protocol. |
| crates/aisix-proxy/src/chat.rs | Records AISIX-native usage + TTFT for streaming and non-streaming chat; emits remaining RL gauges; emits optional budget gauges. |
| crates/aisix-proxy/src/budget.rs | Extends CP budget-check parsing to include optional budget detail fields (with tests). |
| crates/aisix-obs/src/metrics.rs | Implements new metric families, label structs, and recording helpers; adds unit tests for rendering. |
| crates/aisix-obs/src/lib.rs | Re-exports new metrics label/value structs. |
| crates/aisix-admin/src/state.rs | Adds Prometheus config to admin state and builder method. |
| crates/aisix-admin/src/lib.rs | Makes metrics endpoint mount conditional/configurable; adds router tests. |
Comments suppressed due to low confidence (6)
crates/aisix-obs/src/metrics.rs:138
record_requestforwards the same end-to-enddurationintorecord_llm_request, which recordsaisix_llm_api_latency_seconds. That makesaisix_llm_api_latency_secondsidentical to gateway request latency rather than upstream API latency. Consider recordingaisix_llm_api_latency_secondsonly when you have the upstream timing available (or rename the metric to reflect what’s actually measured).
self.record_proxy_request(labels, duration);
self.record_llm_request(labels, duration);
crates/aisix-obs/src/metrics.rs:173
proxy_in_flightis tracked with a single globalAtomicI64but exported withendpoint/inbound_protocollabels. As soon as multiple endpoints/protocols are in flight concurrently, each labeled gauge will be set to the global count (and will be wrong for all but the last-updated label set). Track in-flight counts per label (e.g., a DashMap keyed by(endpoint,inbound_protocol)), or drop the labels if you only want a process-wide gauge.
pub fn increment_proxy_in_flight(&self, endpoint: &str, inbound_protocol: &str) {
let value = self.inner.proxy_in_flight.fetch_add(1, Ordering::Relaxed) + 1;
metrics::with_local_recorder(&self.inner.recorder, || {
metrics::gauge!(
M_PROXY_IN_FLIGHT,
"endpoint" => endpoint.to_string(),
"inbound_protocol" => inbound_protocol.to_string(),
)
.set(value as f64);
crates/aisix-obs/src/metrics.rs:183
decrement_proxy_in_flightalways does anAtomicI64::fetch_sub(1), which can drive the stored counter negative if it’s ever decremented when already at 0 (e.g., cancellation/early-drop paths). Even though the exported value is clamped, the atomic itself remains negative and future increments will be incorrect. Consider using an unsigned atomic plus a saturatingfetch_update/CAS loop (or a guard that guarantees 1:1 inc/dec).
pub fn decrement_proxy_in_flight(&self, endpoint: &str, inbound_protocol: &str) {
let value = self
.inner
.proxy_in_flight
.fetch_sub(1, Ordering::Relaxed)
.saturating_sub(1)
.max(0);
crates/aisix-obs/src/metrics.rs:510
aisix_llm_spend_totalis emitted viametrics::gauge!(...).increment(...)(seerecord_counter_f64), so it will be a Prometheus Gauge even though the name ends with_totaland is described as a cumulative series. If you need a true counter type, consider exporting spend in an integer unit (e.g., micro-USD) viacounter!, or rename the metric to avoid Prometheus counter semantics.
fn record_counter_f64(&self, metric: &'static str, value: f64) {
metrics::gauge!(
metric,
"endpoint" => self.endpoint.to_string(),
"inbound_protocol" => self.inbound_protocol.to_string(),
"provider" => self.provider.to_string(),
"model" => self.model.to_string(),
"upstream_model" => self.upstream_model.to_string(),
"provider_key_id" => self.provider_key_id.to_string(),
"api_key_id" => self.api_key_id.to_string(),
"team_id" => self.team_id.to_string(),
"owner_id" => self.owner_id.to_string(),
)
.increment(value);
}
crates/aisix-obs/src/metrics.rs:344
set_rate_limit_remainingonly sets gauges whenrequests/tokensareSome. If a key/model ever transitions from having a limit to unlimited (or the limiter can’t compute a value), the previous gauge sample will remain exported and become stale. Consider explicitly setting a sentinel value (e.g., -1) when the limit is not applicable, or adding a separateconfigured/presentgauge so scrapes can distinguish “unlimited/unknown” from “stale”.
pub fn set_rate_limit_remaining(
&self,
api_key_id: &str,
model: &str,
requests: Option<u64>,
tokens: Option<u64>,
) {
metrics::with_local_recorder(&self.inner.recorder, || {
if let Some(value) = requests {
metrics::gauge!(
M_RATELIMIT_REMAINING_REQUESTS,
"api_key_id" => api_key_id.to_string(),
"model" => model.to_string(),
)
.set(value as f64);
}
if let Some(value) = tokens {
metrics::gauge!(
M_RATELIMIT_REMAINING_TOKENS,
"api_key_id" => api_key_id.to_string(),
"model" => model.to_string(),
)
.set(value as f64);
}
});
crates/aisix-obs/src/metrics.rs:360
set_budget_gaugesonly updates the gauges for fields that areSome. If the control plane intermittently omits some optional budget fields (or they become absent after being present), previously-exported gauge values for those fields will remain and can become stale. Consider defining a clear behavior for “missing” (e.g., set to NaN/-1 or emit abudget_detail_presentgauge) so dashboards don’t silently show outdated values.
pub fn set_budget_gauges(&self, labels: BudgetLabels<'_>, budget: BudgetGauges) {
metrics::with_local_recorder(&self.inner.recorder, || {
if let Some(value) = budget.limit_usd {
labels.record_gauge(M_BUDGET_LIMIT_USD, value);
}
if let Some(value) = budget.spent_usd {
labels.record_gauge(M_BUDGET_SPENT_USD, value);
}
if let Some(value) = budget.remaining_usd {
labels.record_gauge(M_BUDGET_REMAINING_USD, value);
}
if let Some(value) = budget.reset_seconds {
labels.record_gauge(M_BUDGET_RESET_SECONDS, value as f64);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
crates/aisix-proxy/src/messages.rs:677
record_llm_usagefor/v1/messagesuses..UsageLabels::default(), soupstream_modelandprovider_key_idwill always be"unknown"in usage/token/spend metrics even thoughUsageEventalready carriesprovider_model_version(and the handler resolves the provider key). Consider passing those through intoUsageLabelsso the AISIX-native usage series can be grouped the same way as the rest of the request metrics.
state.metrics.record_llm_usage(
UsageLabels {
endpoint: "/v1/messages",
inbound_protocol: "anthropic",
provider,
model,
api_key_id,
team_id: team_id.unwrap_or("unknown"),
owner_id: owner_id.unwrap_or("unknown"),
..UsageLabels::default()
},
…s fields PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and `metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159` plus the `normalized_prometheus_path` helper at `:164-174`, and exercised both paths end-to-end in three new unit tests (`metrics_endpoint_uses_configured_path`, `metrics_endpoint_normalizes_configured_path`, `metrics_endpoint_can_be_disabled`). The two rows in this PR's observability field table that called those fields "reserved (not yet consulted)" are now factually stale. Update both rows to `wired` and align the row descriptions with #331's runtime behaviour. Also adjust the introductory paragraph above the table so it no longer asserts that only `service_name` and `log_level` are consulted at runtime. This is the consumer-trace discipline locked in during round 2 of this PR, applied prospectively to a moving target: PR #331 changed the consumer site after round 2's verification window, so the doc text needs to update.
Adds AISIX-native Prometheus metrics for the LiteLLM-style categories AISIX can support from DP data today, while keeping the existing compatibility metrics.
What changed:
aisix_proxy_*,aisix_llm_*, deployment, routing, quota, budget, Redis, usage-event, and OTLP fan-out metric familiesaisix_requests_total,aisix_request_duration_seconds,aisix_ratelimit_rejections_total, andaisix_tokens_consumed_totalobservability.metrics.prometheus.enabled/pathcontrol the admin scrape endpoint instead of always mounting/metrics; path values are normalized sointernal/prommounts as/internal/promaisixand checks representativeaisix_*metricsBudget gauges are emitted only when the CP budget-check response includes optional budget detail fields. Team/member labels use
team_idandowner_idwhen those fields are present in DP config; otherwise they render asunknownrather than inventing aliases or emails.No database changes.
Tested:
cargo clippy --workspace --all-targets -- -D warningscargo build -p aisix-server --bin aisixcargo test -p aisix-obs --libcargo test -p aisix-admin metrics_endpoint --libcd tests/e2e && pnpm exec vitest run src/cases/prometheus-metrics-e2e.test.tscargo test --workspace,cd tests/e2e && pnpm exec vitest runSummary by CodeRabbit
New Features
Bug Fixes / Tests
Documentation