From de3f263faaf62feba727f04a2ee8de87483a9306 Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Wed, 28 Oct 2026 05:46:29 -0700 Subject: [PATCH 1/6] docs(agents): add metrics guidelines for authoring metrics, logs, and traces Add .ai/metrics-guidelines.md, the canonical rules an agent or contributor follows when adding or changing a metric, a log event, or a trace span: choosing between them, the questions a new metric must answer in the pull request, naming, labels, registration, stability and deprecation, catalog documentation, and where each piece lives in Rust, Python, and Go. Each rule has an ID so reviews can cite it. Point agents at it from the root AGENTS.md, and align the two in-repo sources that taught the opposite: the naming conventions in prometheus_names.rs (_ms, _percent, _per_second) and the Metrics Developer Guide's labeled-counter example (requests_by_model). Route the guideline to the observability codeowners as well as process. Signed-off-by: Yuewei Na --- .ai/metrics-guidelines.md | 331 ++++++++++++++++++ .github/codeowners/areas.yaml | 4 + AGENTS.md | 6 +- CODEOWNERS | 1 + .../metrics-developer-guide.md | 14 +- lib/runtime/src/metrics/prometheus_names.rs | 29 +- 6 files changed, 366 insertions(+), 19 deletions(-) create mode 100644 .ai/metrics-guidelines.md diff --git a/.ai/metrics-guidelines.md b/.ai/metrics-guidelines.md new file mode 100644 index 000000000000..50d9b0441a72 --- /dev/null +++ b/.ai/metrics-guidelines.md @@ -0,0 +1,331 @@ + + + +# Metrics Guidelines + +Rules for adding or changing a metric, a log event, or a trace span in Dynamo. They keep +metrics useful, cheap to run, and stable for the people who build dashboards and alerts on +them. + +The rules apply to every new or changed metric, log event, and span, in Rust, Python, and Go. +Existing code that breaks a rule is not a precedent: do not copy it, and do not rename an +existing metric as a side effect of another change (see +[Stability and deprecation](#stability-and-deprecation)). Engine metrics that Dynamo passes +through unchanged (`vllm:*`, `sglang:*`, `lmcache:*`, and the TensorRT-LLM `trtllm_*` metrics) +keep their engine's names and are out of scope. A metric that Dynamo computes is a Dynamo +metric, even when an engine prefix filter passes it through. + +Each rule has an ID, such as **N2**, so that a review can cite it. An exception to a rule +needs a written reason in the pull request. + +## Metric, log, or trace + +**C1.** A metric exists only if something reads it continuously (an alert, an SLO, a +dashboard, or another Dynamo component) and its labels have a small, known set of values. +Everything else is a structured log event. + +| If | Then | +|----|------| +| You can name the dashboard, alert, or component that reads it, and the labels are few and bounded | Metric | +| You need a request, tenant, session, or user ID to make it useful | Log event | +| The reader is a report or an investigation (billing, audit, post-incident) | Log event | +| You need a live signal and the detail behind it | One small metric for the aggregate, and one log event for the detail | +| You want to see where one request spent its time across components | Trace span (sampled) | + +For example, failed requests are counted in +`dynamo_frontend_requests_total{model, endpoint, request_type, status, error_type}`, which +feeds an error-rate alert. The request ID and tenant of each failure go in a log event. +`record_unsupported_residency_event` in `lib/kv-router/src/indexer/metrics.rs` shows the +pair in code: it increments a counter whose labels have a fixed set of values, and it logs +the worker and rank as fields of a rate-limited event. + +## Before you add a metric + +Answer these questions in the pull request description. If an answer rules the metric out, +use a log event instead. + +**A1. Reader.** Name the alert, SLO, dashboard panel, or component that reads the metric. +"It might be useful" is not a reader. In-repo readers include the Grafana dashboards under +`dev/observability/grafana_dashboards/` and `deploy/observability/`, and the planner's +queries in `components/src/dynamo/planner/monitoring/`. + +**A2. Not a query.** If PromQL over existing metrics gives the same answer, do not add the +metric. Name the PromQL in the pull request when you rule it out. + +**A3. Not an engine metric.** Do not re-export an engine metric under a `dynamo_` name; +Dynamo already passes engine metrics through. Add a Dynamo metric only when it measures +something the engine cannot see (the frontend's or the router's view), or when Dynamo +components need the same signal from every engine under one name. In that case, normalize +it across engines and document the mapping from each engine. + +**A4. Not a sibling.** If the metric differs from an existing one by one dimension (a +modality, a direction, a role), add a label instead of a new name. When the existing metric +is stable, adding a label is a breaking change (**S1**): add the labeled metric under a new +name and deprecate the old one. + +**A5. Static configuration.** Values that never change while the process runs (context +length, block size) go in one `*_info` metric with value 1 and each parameter as a label, +for example `dynamo_model_info{model, context_length, kv_cache_block_size}`. + +**A6. Series count.** Each distinct combination of label values is one time series, and +each series costs memory in Prometheus. Multiply the number of values of every label: +`model` (10 values) and `status` (3 values) make 30 series, so a counter with five small +labels can still create hundreds. A histogram adds one series per bucket plus `+Inf`, +`_sum`, and `_count`, so 10 buckets make 13 series per label combination. Write the total +in the pull request. The count is per process; the cost in Prometheus is that number times +the number of scraped processes. Python `prometheus_client` also exports a `_created` series +for every counter and histogram label combination: count 2 series per counter combination +and buckets + 4 per histogram combination. + +**A7. Documentation.** Document the metric in the same pull request (**D1**, **D2**). + +Write the answers as one block per metric in the pull request description: + +```text +Metric: dynamo_frontend_prefill_queue_wait_seconds (histogram) +Reader: "Prefill queue wait p95" panel in the router dashboard; alert at 2 s +Labels: model (10 values) +Series: 10 x (10 buckets + 3) = 130 per frontend +Query?: No. Existing stage_duration_seconds does not split out queue wait. +Docs: Added to the Metrics Catalog page +``` + +## Naming + +Apply these rules to the exported name, which is what dashboards and alerts see. The APIs +assemble it from parts (see [Where things live](#where-things-live)). + +**N1.** The exported name is snake_case and starts with `dynamo__`. + +**N2.** The name ends with its unit in base form: `seconds` (not `ms`, `us`, or `ns`), `bytes` +(not `KB` or `GB`), `tokens`, or `ratio` for a value from 0 to 1 (not a percentage). For +example, `request_duration_seconds` and `kv_cache_usage_ratio`. + +**N3.** A counter only goes up (requests, tokens, errors), ends in `_total`, and is charted +with `rate()`. A counter with a unit puts the unit before `_total`, as in +`detokenize_seconds_total`. + +**N4.** A gauge can go up and down (in-flight requests, cache usage) and does not end in +`_total`. + +**N5.** Do not put a label name or a label value in the metric name. Use a label. + +**N6.** The help text says what is measured and matches the value. If the code measures +something different from what the name says, fix the name. Changing what a metric measures +means a new name. + +```text +# BAD -- unit not in base form, percentage, counter without _total, label value in the name +dynamo_frontend_queue_wait_ms +dynamo_component_cache_usage_percent +dynamo_frontend_rejected_requests +dynamo_frontend_images_per_request + +# GOOD +dynamo_frontend_queue_wait_seconds +dynamo_component_cache_usage_ratio +dynamo_frontend_rejected_requests_total +dynamo_frontend_multimodal_items_per_request{modality} +``` + +## Labels + +**L1.** Aim for fewer than 10 values per label. A label that can reach about 100 values +does not belong on a metric; use a log event or a trace. + +**L2.** Never use a request, session, user, or tenant ID, a prompt, a URL path, or a raw +client-supplied value as a label. Map a client-supplied value to a known, bounded set +first, as `categorizeError` does in `deploy/operator/internal/observability/metrics.go`. + +**L3.** The `model` label comes from the set of served models, never from the raw value in +the request. A request that names a model that is not served uses one fixed value. In the +frontend, resolve the label with `ModelManager::metric_model_for`, which returns the +existing sentinel `unknown_model`, so that all frontend metrics agree. A component with no +existing sentinel uses `unknown`, as the endpoint picker's `set_served_model` does. + +**L4.** Summing over a label must give a number that means something. A label that mixes +different quantities, such as `kind` with the values `requests` and `bytes`, should be +separate metrics. Static `*_info` metrics are the exception. + +**L5.** Do not add per-worker labels (`worker_id`, `dp_rank`) to a new metric unless an +autoscaler or the router needs them. A per-worker series must stop being exported when its +worker is gone, as `lib/llm/src/discovery/worker_monitor.rs` does with +`remove_label_values`. This rule is about labels that you declare. The `worker_id` that the +Rust runtime adds to a process's own metrics identifies that process and is not affected. + +**L6.** Pre-create the expected values of bounded labels (such as `status` and `endpoint`) +at startup, so that `rate()` and `absent()` alerts work from the first scrape. Create the +`model` values when the model registers, because they are not known at startup. + +**L7.** Histograms are the main cost. Use about 10 buckets, put a bucket edge at every alert +threshold, and use `model` as the only label that you declare. + +## Registration + +**R1.** Register a metric at startup in every process that updates it, not on first use. +A labeled family has no output until it has at least one child, so registration alone does +not make it visible: pre-create its children as well (**L6**). + +**R2.** Do not add a metric that no production code updates. + +**R3.** When you add, rename, or remove a name in `lib/runtime/src/metrics/prometheus_names.rs`, +regenerate its Python mirror in the same pull request (see [Where things live](#where-things-live)). +Never edit the mirror by hand. + +## Patterns to follow + +The right-hand column shows the shape to aim for in new metrics. It is not a list of metrics +that exist today. + +| Instead of | Do this | +|------------|---------| +| `images_per_request`, `videos_per_request`, `audio_per_request` | One `multimodal_items_per_request{modality}` | +| `model_context_length`, `model_kv_cache_block_size`, `model_max_num_seqs` | One `model_info{model, context_length, kv_cache_block_size, max_num_seqs}` with value 1 | +| `num_prefill_replicas`, `num_decode_replicas` | One `num_replicas{sub_component_type}` | +| `router_overhead_total_ms`, or a "percent" gauge that holds 0 to 1 | `_seconds` and `_ratio` names | +| A counter of events above a threshold, next to a histogram with that bucket edge | `increase(x_count[5m]) - increase(x_bucket{le="T"}[5m])` in PromQL, where `T` is an existing bucket edge as exported | + +## Stability and deprecation + +Renaming or reshaping a metric breaks dashboards and alerts, often silently: a panel just +shows no data. So every metric has one of two stability levels. + +- **Experimental:** marked as experimental in the Metrics Catalog. It can change or + disappear in any release, with a release note. +- **Stable:** documented in the Metrics Catalog without an experimental mark. A metric + becomes stable after at least one release as experimental, once a maintainer has + reviewed its name, unit, type, and labels, and a test checks that the code updates it. + +A new metric therefore starts as experimental. A catalog entry with no experimental mark is +stable, including an entry whose name breaks these rules. + +**S1.** Do not rename a stable metric, change its type or unit, or add or remove labels. Add +a new metric and deprecate the old one. Adding a new value to an existing label is allowed +when existing queries stay correct (for example, `sum()` over the label still means the +same thing). + +**S2.** To deprecate a stable metric, mark it deprecated in the Metrics Catalog and in its +help text, name its replacement or a PromQL recipe, and name the release that removes it. +Move in-repo readers (**A1**) to the replacement, and state the deprecation in the pull +request description so that it can go into the release notes. + +**S3.** Keep a deprecated metric for at least two releases after the announcement, then +remove it. Never remove it in a patch release. + +**S4.** Remove a metric only if every question it answers can still be answered from what +remains, with the same meaning, measurement point, label granularity, scrape target, +timeliness, and availability. A metric that was never registered or never updated can be +removed without deprecation. + +Not promised as stable: engine metric names and labels (vLLM, SGLang, and TensorRT-LLM +metrics pass through under their own names), experimental metrics, per-worker debugging +gauges, and metrics that are not in the Metrics Catalog. + +## Documentation + +**D1.** The [Metrics Catalog](../docs/fern/pages/reference/observability/metrics-catalog.mdx) +is the source of truth for metric documentation, and a metric that is not in it is +unsupported. In the same pull request, add or update the metric's entry: its type, unit, +labels and their expected values, and stability level. Operator metrics go in +[Operator Metrics](../docs/fern/pages/reference/observability/operator-metrics.mdx) instead. +If no section fits the metric's family, add one. If the Metrics Inventory page +(`metrics-inventory.md` in the same directory) exists, add or update the metric's row there +as well. + +- Mark a new metric experimental: start its description with `**Experimental.**`. +- Mark a deprecated metric with `deprecated={true}` on its `ParamField`, and name the + replacement and the removal release in its description. + +**D2.** Update the `AGENTS.md` nearest to the code that owns the metric to say what the +metric is and how it can be used. The "Frontend Serving-Load Metrics" section of +`lib/llm/AGENTS.md` is an example. + +## Logs and traces + +**T1.** Logs are structured JSON, one event per line. Write each event so that its details +are separate fields when the JSONL console format is on +(`DYN_LOGGING_CONSOLE_FORMAT=jsonl`; the default is readable text). + +- Rust: use `tracing` macros with `key = value` fields and a fixed message, for example + `tracing::warn!(worker_id, dp_rank, "Ignoring KV event")`. +- Python: the log bridge forwards only the formatted message to Rust, so fields passed with + `extra=` are lost. Put the values in lazy `%s` arguments, as `.ai/python-guidelines.md` + requires. + +**T2.** Request-path log events carry `x_request_id`, `trace_id`, and `span_id`. + +- Rust: emit the event inside the request's span; the JSONL formatter takes these IDs from + the span. Attach the span to spawned tasks with `.instrument(...)` or `in_current_span()`. +- Python: include the request ID with `context.id()`; `context.trace_id` and + `context.trace_headers()` give the trace context. + +**T3.** Use a trace span, which is sampled, to show where one request spent its time across +components. Do not add a metric label to get that view. + +## Where things live + +### Rust + +- Add each name as a `pub const` suffix in its subsystem's `pub mod` in + `lib/runtime/src/metrics/prometheus_names.rs`. Then regenerate the Python mirror with + `cargo run -p dynamo-codegen --bin gen-python-prometheus-names`. Do not edit + `lib/bindings/python/src/dynamo/prometheus_names.py` by hand; the + `committed_python_matches_generated` test fails when it is stale. +- Create metrics with `.metrics().create_*` on the runtime, namespace, component, or + endpoint (`lib/runtime/src/metrics.rs`). Pass only the suffix: the API prepends + `dynamo_component_`, or the prefix given to `create_metric_with_options`. A full + `dynamo_...` name gets the prefix twice. +- The API adds the `dynamo_namespace`, `dynamo_component`, `dynamo_endpoint`, and + `worker_id` labels and rejects them as your own labels. In the `*vec` methods, the + argument named `const_labels` holds the variable label names. +- A histogram created with `None` buckets gets the `prometheus` crate defaults: 11 edges from + 0.005 to 10, sized for seconds. Pass buckets that fit the unit and the alert thresholds + (**L7**). +- The frontend's own families are in `lib/llm/src/http/service/metrics.rs`. Use the bounded + enums there (`Status`, `ErrorType`, `Endpoint`) for label values. +- Test that the code updates a metric by rendering the registry with `prometheus_expfmt()`, + as `lib/llm/tests/admission_dp_rank.rs` does. + +### Python + +- Python cannot create runtime-registered metrics. Create `prometheus_client` metrics in a + dedicated `CollectorRegistry`, and expose it with the helpers in + `components/src/dynamo/common/utils/prometheus.py`. Register it on both the exposition + callback and the typed (OTLP) callback; registering only one leaves the metrics out of + OTLP export. +- Name constants shared with Rust are in `dynamo.prometheus_names`, which is generated from + the Rust file. Add a shared name in Rust first. +- `prometheus_client` adds `_total` to a counter's exported name. Apply **N3** to the + exported name. +- Where SGLang's multiprocess mode needs it, import `prometheus_client` inside the function, + with a comment that says why. +- Test end to end with `find_metric_samples` in `tests/utils/prometheus.py`, which tells an + absent series from a zero one. + +### Go (operator) + +- Operator metrics are `client_golang` vectors with `Namespace: "dynamo_operator"` in + `deploy/operator/internal/observability/metrics.go`, registered once in `InitMetrics`. + +## Existing names that break these rules + +These metrics break a naming, type, or registration rule. Their owners have not yet decided +between a deprecating rename and a recorded exception. Do not copy them, and do not rename +them as a side effect of another change. The exemption covers these existing names only: a new +metric that repeats one of their defects must follow the rules. + +| Metric | Rule | +|--------|------| +| `dynamo_tokio_worker_mean_poll_time_ns` | N2 | +| `dynamo_frontend_detokenize_total_us` | N2, N3 | +| `dynamo_frontend_detokenize_token_count` | N3 | +| `dynamo_component_kv_cache_events_applied`, `dynamo_kvrouter_kv_cache_events_applied` | N3 | +| `dynamo_component_kv_cache_event_warnings`, `dynamo_kvrouter_kv_cache_event_warnings` | N3 | +| `dynamo_router_overhead_*_ms` | N2 | +| `kvbm_matched_tokens` | N1, N3 | +| `dynamo_component_gpu_cache_usage_percent` | N2 | +| `dynamo_component_kv_cache_hit_rate` | R2 | +| `dynamo_operator_resources_total` | N4 | + +Other existing names in `prometheus_names.rs` and elsewhere also break these rules. They +are not precedents either. diff --git a/.github/codeowners/areas.yaml b/.github/codeowners/areas.yaml index 9c0b0619c195..10b3e3764d5d 100644 --- a/.github/codeowners/areas.yaml +++ b/.github/codeowners/areas.yaml @@ -618,6 +618,10 @@ shared: owners: - process - agents +- glob: .ai/metrics-guidelines.md + owners: + - process + - observability - glob: .github/workflows/fern-docs.yml owners: - ops diff --git a/AGENTS.md b/AGENTS.md index d6dbdf5e315e..d96da4a5c0b6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,10 @@ When reviewing frontend or runtime changes, also read the corresponding [frontend review prompt](.github/review-prompts/frontend.md) or [runtime review prompt](.github/review-prompts/runtime.md) for additional CODEOWNERS guidance. +Before adding or changing a metric, a log event, or a trace span, read +[`.ai/metrics-guidelines.md`](.ai/metrics-guidelines.md). It decides between a metric, a log, +and a trace, and sets the rules for names, labels, registration, stability, and documentation. + Before changing request or response fields, validation, preprocessing, backend translation, or response projection, read the [protocol field handling contract](docs/fern/pages/developer-guide/knowledge-base/modular-components/frontend/protocol-field-handling.md). @@ -182,7 +186,7 @@ Sibling repositories this repo integrates with: | `docs/fern/` | Fern docs site: `pages/` holds every page, the rest is site config (`index.yml`, `docs.yml`, `main.css`, `components/`, `scripts/`, `translations/`). Read [`docs/fern/AGENTS.md`](docs/fern/AGENTS.md) before editing, and [`docs/fern/pages/AGENTS.md`](docs/fern/pages/AGENTS.md) before adding a page | | `examples/`, `recipes/` | Runnable examples and deployment recipes — also covered by [`docs/fern/AGENTS.md`](docs/fern/AGENTS.md) | | `benchmarks/`, `tests/` | Benchmark harnesses and the top-level pytest suite | -| `.ai/` | Agent topic guidelines: `ais-perf-model-guidelines.md` (canonical AISimulate interface), `bash-launch-guidelines.md`, `ci-guidelines.md`, `linear-ticket-refs.md`, `pytest-guidelines.md`, `python-guidelines.md`, `test-model-size-guardrails.md` | +| `.ai/` | Agent topic guidelines: `ais-perf-model-guidelines.md` (canonical AISimulate interface), `bash-launch-guidelines.md`, `ci-guidelines.md`, `linear-ticket-refs.md`, `metrics-guidelines.md`, `pytest-guidelines.md`, `python-guidelines.md`, `test-model-size-guardrails.md` | | `.agents/skills/` | Agent skills (see [Skills](#skills)) | ## Build diff --git a/CODEOWNERS b/CODEOWNERS index 3b7c025b993c..e15672777a55 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -640,6 +640,7 @@ requirements.txt /recipes/**/tokenspeed/** @ai-dynamo/dynamo-performance-codeowners @ai-dynamo/dynamo-tokenspeed-codeowners /recipes/**/vllm/**/*xpu* @ai-dynamo/dynamo-performance-codeowners @ai-dynamo/dynamo-backend-vllm-codeowners @ai-dynamo/dynamo-xpu-codeowners /tests/serve/test_vllm.py @ai-dynamo/dynamo-runtime-codeowners @ai-dynamo/dynamo-backend-vllm-codeowners +/.ai/metrics-guidelines.md @ai-dynamo/dynamo-process-codeowners @ai-dynamo/dynamo-observability-codeowners /deploy/operator/Dockerfile @ai-dynamo/dynamo-operator-codeowners @ai-dynamo/dynamo-ops-codeowners /lib/llm/src/request_trace/ @ai-dynamo/dynamo-runtime-codeowners @ai-dynamo/dynamo-agents-codeowners @ai-dynamo/dynamo-observability-codeowners /lib/mocker/servers/sglang/ @ai-dynamo/dynamo-performance-codeowners @ai-dynamo/dynamo-backend-sglang-codeowners @ai-dynamo/dynamo-sidecar-codeowners diff --git a/docs/fern/pages/developer-guide/advanced-customizations/metrics-developer-guide.md b/docs/fern/pages/developer-guide/advanced-customizations/metrics-developer-guide.md index 28444d163874..d565e65e2a13 100644 --- a/docs/fern/pages/developer-guide/advanced-customizations/metrics-developer-guide.md +++ b/docs/fern/pages/developer-guide/advanced-customizations/metrics-developer-guide.md @@ -7,6 +7,8 @@ subtitle: Reference for creating counters, gauges, and histograms with Dynamo's This guide explains how to create and use custom metrics in Dynamo components using the Dynamo metrics API. +Before you add or change a metric, read the [metrics guidelines](https://github.com/ai-dynamo/dynamo/blob/main/.ai/metrics-guidelines.md). They cover when to use a metric instead of a log or a trace, and the rules for names, labels, registration, stability, and documentation. + ## Metrics Exposure All metrics created via the Dynamo metrics API are automatically exposed on the `/metrics` HTTP endpoint in Prometheus Exposition Format text when the following environment variable is set: @@ -87,10 +89,10 @@ latency.observe(0.023); // 23ms ```rust // Create vector metrics with label names -let requests_by_model = endpoint.metrics().create_countervec( - "requests_by_model", - "Requests by model", - &["model_type", "model_size"], +let model_requests_total = endpoint.metrics().create_countervec( + "model_requests_total", + "Requests handled, by served model", + &["model"], &[] )?; @@ -101,8 +103,8 @@ let memory_by_gpu = endpoint.metrics().create_gaugevec( &[] )?; -// Use with specific label values -requests_by_model.with_label_values(&["llama", "7b"]).inc(); +// Use with specific label values; take the model from the served models, not the request +model_requests_total.with_label_values(&["Qwen/Qwen3-0.6B"]).inc(); memory_by_gpu.with_label_values(&["0", "allocated"]).set(8192.0); ``` diff --git a/lib/runtime/src/metrics/prometheus_names.rs b/lib/runtime/src/metrics/prometheus_names.rs index bac7e885b18f..3bb59c47c47d 100644 --- a/lib/runtime/src/metrics/prometheus_names.rs +++ b/lib/runtime/src/metrics/prometheus_names.rs @@ -15,13 +15,19 @@ //! //! ## Naming Conventions //! +//! Before adding or changing a name here, read `.ai/metrics-guidelines.md`, which also covers +//! labels, registration, stability, and documentation. Existing names that break these +//! conventions are kept for compatibility; do not copy them, and do not rename them as a +//! side effect of another change. +//! //! All metric names should follow: `{prefix}_{name}_{suffix}` //! //! **Prefix**: Component identifier (`dynamo_component_`, `dynamo_frontend_`, etc.) //! **Name**: Descriptive snake_case name indicating what is measured //! **Suffix**: -//! - Units: `_seconds`, `_bytes`, `_ms`, `_percent`, `_messages`, `_connections` -//! - Counters: `_total` (not `total_` prefix) - for cumulative metrics that only increase +//! - Units, in base form: `_seconds` (not `_ms`), `_bytes`, `_tokens`, and `_ratio` for a +//! value from 0 to 1 (not `_percent`) +//! - Counters: `_total` (not `total_` prefix), after the unit - for cumulative metrics that only increase //! - Gauges: No `_total` suffix - for current state metrics that can go up and down //! - Note: Do not use `_counter`, `_gauge`, `_time`, or `_size` in Prometheus names (too vague) //! @@ -29,14 +35,13 @@ //! - ❌ `_counter` → ✅ `_total` //! - ❌ `_sum` → ✅ `_total` //! - ❌ `_gauge` → ✅ (no suffix needed for current values) -//! - ❌ `_time` → ✅ `_seconds`, `_ms`, `_hours`, `_duration_seconds` -//! - ❌ `_time_total` → ✅ `_seconds_total`, `_ms_total`, `_hours_total` -//! - ❌ `_total_time` → ✅ `_seconds_total`, `_ms_total`, `_hours_total` +//! - ❌ `_time`, `_ms` → ✅ `_seconds`, `_duration_seconds` +//! - ❌ `_time_total`, `_total_time`, `_ms_total` → ✅ `_seconds_total` //! - ❌ `_total_time_seconds` → ✅ `_seconds_total` -//! - ❌ `_average_time` → ✅ `_seconds_avg`, `_ms_avg` -//! - ❌ `_size` → ✅ `_bytes`, `_total`, `_length` -//! - ❌ `_some_request_size` → ✅ `_some_request_bytes_avg` -//! - ❌ `_rate` → ✅ `_per_second`, `_per_minute` +//! - ❌ `_average_time` → ✅ a `_seconds` histogram; its average is `rate(x_sum) / rate(x_count)` +//! - ❌ `_size` → ✅ `_bytes`, `_tokens` +//! - ❌ `_percent` → ✅ `_ratio` (0 to 1) +//! - ❌ `_rate`, `_per_second` → ✅ a `_total` counter, charted with `rate()` //! - ❌ `disconnected_clients_total` → ✅ `disconnected_clients` (gauge, not counter) //! - ❌ `inflight_requests_total` → ✅ `inflight_requests` (gauge, not counter) //! - ❌ `connections_total` → ✅ `current_connections` (gauge, not counter) @@ -47,9 +52,9 @@ //! - ✅ `dynamo_component_errors_total` - Total error counter (not `total_errors`) //! - ✅ `dynamo_component_memory_usage_bytes` - Memory usage gauge //! - ✅ `dynamo_frontend_active_requests` - Current active requests gauge -//! - ✅ `dynamo_component_cpu_usage_percent` - CPU usage percentage -//! - ✅ `dynamo_frontend_tokens_per_second` - Token generation rate -//! - ✅ `dynamo_messaging_client_connection_duration_ms` - Connection time in milliseconds +//! - ✅ `dynamo_component_cpu_usage_ratio` - CPU usage from 0 to 1 +//! - ✅ `dynamo_frontend_output_tokens_total` - Output token counter; `rate()` gives tokens per second +//! - ✅ `dynamo_messaging_client_connection_duration_seconds` - Connection time histogram //! - ✅ `dynamo_messaging_client_current_connections` - Current active connections gauge //! - ✅ `dynamo_messaging_client_in_messages_total` - Total messages received counter //! From c17c654afa9b564fd45e7f502e45802c952958df Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Wed, 28 Oct 2026 05:49:22 -0700 Subject: [PATCH 2/6] docs(review): add a metrics review prompt and apply it in CodeRabbit and pr-checklist Add .github/review-prompts/metrics.md, which tells a review agent how to find the metrics, log events, and trace spans a change adds or modifies, build a per-metric table (exported name, type, declared labels, series count, registration and update sites, catalog entry, stability), and check each one against .ai/metrics-guidelines.md, citing the rule IDs. It defines what a finding must contain (location, rule, problem, consequence, exact correction), keeps author-only questions (reader, series count) apart from defects, and excludes pre-existing violations, engine pass-through metrics, and runtime-injected labels. Apply it from every review surface: a CodeRabbit path instruction for Rust, Python, and Go files and for the metrics reference pages, with the prompt added to CodeRabbit's code guidelines; the pr-checklist area guidance and PR description section; the root AGENTS.md review pointer; and the review prompts README. Route the prompt to the ops and observability codeowners. Signed-off-by: Yuewei Na --- .agents/skills/pr-checklist/SKILL.md | 3 + .coderabbit.yaml | 5 + .github/codeowners/areas.yaml | 4 + .github/review-prompts/README.md | 11 ++ .github/review-prompts/metrics.md | 209 +++++++++++++++++++++++++++ AGENTS.md | 2 + CODEOWNERS | 1 + 7 files changed, 235 insertions(+) create mode 100644 .github/review-prompts/metrics.md diff --git a/.agents/skills/pr-checklist/SKILL.md b/.agents/skills/pr-checklist/SKILL.md index 42c1c4e69058..66bdaea2808f 100644 --- a/.agents/skills/pr-checklist/SKILL.md +++ b/.agents/skills/pr-checklist/SKILL.md @@ -55,6 +55,7 @@ Exclude tests, documentation, comments, and whitespace from this count, as they - Rust: the `graham-code-review` skill - Python: [Python guidelines](../../../.ai/python-guidelines.md) - Tests: [pytest guidelines](../../../.ai/pytest-guidelines.md) and [test model size guardrails](../../../.ai/test-model-size-guardrails.md) +- Metrics, logs, and traces: [metrics guidelines](../../../.ai/metrics-guidelines.md) and the [metrics review prompt](../../../.github/review-prompts/metrics.md) - Docs, examples, and recipes: the `dynamo-docs` skill - Any directory: its nearest `AGENTS.md` @@ -92,4 +93,6 @@ Fill in every section of `.github/pull_request_template.md`, and title the PR pe **Validation.** Back each claim with evidence, ideally a table with one row per behavior: the case, what happened before, and what happens now. For bug fixes, explain what caused the problem and why the fix addresses it. Include performance data when the change claims or risks a performance impact. Name each key test added and the regression it catches. Do not list routine test runs, clippy or linter checks, or unnecessary detail. +**Metrics.** For each new metric, include the block that the [metrics guidelines](../../../.ai/metrics-guidelines.md#before-you-add-a-metric) ask for: its reader, labels, series count, why existing metrics or PromQL do not answer the question, and its catalog entry. + **Issues and Ongoing Work.** Reference the GitHub or Linear issue the PR implements, for example `Closes #8480` or `Closes DYN-1234`. List any similar PRs, issues, or code on `main` that the Existing work check found, each with one line on how this PR differs; if the search found nothing, say so. As the final step, run the `dynamo-issue-first` skill to validate the issue and its reference. diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 79027a5f98f9..bc369791e4f4 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -37,9 +37,14 @@ reviews: instructions: "Review against .ai/bash-launch-guidelines.md" - path: "**/*.{py,rs,go,ts,tsx,js,mjs,cjs,sh,yaml,yml,toml}" instructions: "Flag internal Linear ticket references (e.g. DIS-XXXX, DYN-XXXX, OPS-XXXX, DEP-XXXX, or any -NNNN ID) in the added lines; see .ai/linear-ticket-refs.md for what to recommend." + - path: "**/*.{rs,py,go}" + instructions: "If the change adds or modifies a metric, a log event, or a trace span, review it against .ai/metrics-guidelines.md, using the checks and finding format in .github/review-prompts/metrics.md." + - path: "docs/fern/pages/reference/observability/*.mdx" + instructions: "Review metric entries that the change adds, deprecates, or removes against the Stability and Documentation rules in .ai/metrics-guidelines.md." knowledge_base: code_guidelines: filePatterns: - ".ai/**/*.md" + - ".github/review-prompts/metrics.md" chat: auto_reply: true diff --git a/.github/codeowners/areas.yaml b/.github/codeowners/areas.yaml index 10b3e3764d5d..1e8f70141662 100644 --- a/.github/codeowners/areas.yaml +++ b/.github/codeowners/areas.yaml @@ -582,6 +582,10 @@ shared: owners: - ops - runtime +- glob: .github/review-prompts/metrics.md + owners: + - ops + - observability # CODEOWNERS stays on the process area alone. GitHub requests every team on # the matching line, even when one approval satisfies the gate, so listing # every area here pages every codeowner group for a generated-file edit. diff --git a/.github/review-prompts/README.md b/.github/review-prompts/README.md index ae0520fdc64f..d2de16f13a90 100644 --- a/.github/review-prompts/README.md +++ b/.github/review-prompts/README.md @@ -53,3 +53,14 @@ Keep these prompts in plain English and usable directly when updating them. Add concern when multiple source discussions substantiate it, retain the concrete behavioral distinction, and avoid turning a historical fix into an unconditional requirement for unrelated changes. + +## Metrics review prompt + +The [metrics](metrics.md) prompt is not a codeowner prompt. It applies the +[metrics guidelines](../../.ai/metrics-guidelines.md) to the metrics, log events, and +trace spans that a change adds or modifies, in any area of the repository. Its checks +come from that written policy rather than from review history, and each one cites the +guideline rule it enforces. The guideline owns the rules: change a rule there first, +then update the check that cites it. The prompt also defines what a finding must +contain, so that each one names the correction, and it keeps questions that only the +author can answer, such as a metric's reader, separate from defects. diff --git a/.github/review-prompts/metrics.md b/.github/review-prompts/metrics.md new file mode 100644 index 000000000000..300617a0c15b --- /dev/null +++ b/.github/review-prompts/metrics.md @@ -0,0 +1,209 @@ + + +# Metrics review + +Review the metrics, log events, and trace spans that the proposed change adds or modifies, and +check them against the [metrics guidelines](../../.ai/metrics-guidelines.md). The guidelines own +the rules; cite their IDs, such as **N2**, in every finding. Read the diff and enough surrounding +code to establish each metric's exported name, type, labels, and the code that registers and +updates it. Apply the checks to any area of the repository that the change touches. + +A metric's name, type, unit, and labels are a contract that dashboards and alerts depend on. A +rule violation is a contract defect, not a style preference. + +## Find the telemetry + +1. Look for metric definitions and changes: + - Rust: new `pub const` names in `lib/runtime/src/metrics/prometheus_names.rs`; + `.metrics().create_*` and `create_metric_with_options` calls; `prometheus` crate + constructors such as `IntCounterVec::new`; custom `Collector` implementations. + - Python: `prometheus_client` `Counter`, `Gauge`, `Histogram`, `Info`, and `Enum`; registry + bridges such as `register_engine_metrics_callback`, + `register_prometheus_expfmt_callback`, and `register_prometheus_typed_callback`. + - Go: `client_golang` constructors such as `prometheus.NewGaugeVec`, and registration in + `InitMetrics`. + - Docs: `metrics-catalog.mdx`, `operator-metrics.mdx`, and `metric-labels.mdx` under + `docs/fern/pages/reference/observability/`. + +2. Look for added log events (`tracing::` macros, Python `logging` and `logger` calls) on request + paths, and for added spans (`tracing` span macros, `#[instrument]`, `.instrument(...)`, and + `dynamo.common.backend.telemetry`). + +3. Skip test-only code (`tests/`, `#[cfg(test)]`, and code behind the `bench` feature) and the + generated `lib/bindings/python/src/dynamo/prometheus_names.py`; review its Rust source + instead. When the change adds, renames, or removes a constant in `prometheus_names.rs` + without regenerating that file, report it under **R3** with the regenerate command. If the + change adds or modifies no metric, log event, or span, say so and stop. + +4. A metric that the change registers, exports, or updates for the first time is new for this + review. Check its whole definition (name, type, unit, labels, and buckets), even where those + lines are unchanged. + +## Build the metric table + +5. For each new or changed metric, record its exported name, type, declared labels with the + source of each label's values, series per process, registration site, production update + site, catalog entry, and stability level. Resolve the exported name the way the API builds + it: + - Rust `create_*` prepends `dynamo_component_`, or the prefix given in `MetricOptions`. A + full `dynamo_...` name passed in gets the prefix twice. + - Go joins `Namespace` and `Name` with `_`. + - Python `prometheus_client` adds `_total` to a counter's exported name. + +6. Count only the labels that the author declares. The runtime adds `dynamo_namespace`, + `dynamo_component`, `dynamo_endpoint`, and `worker_id`, and the Python bridges add `model` + and `model_name`; do not report those against **L1**, **L5**, or **L7**. + +## Checks + +### Metric or log + +7. **C1, L2.** Report a label whose value is a request, session, user, or tenant ID, a prompt, a + URL path, or a value copied from a request or protocol field. The correction moves the detail + to a log event with fields and keeps, at most, an aggregate counter with bounded labels. + +### Justification + +8. **A1-A6.** Check the pull request description for the Metric, Reader, Labels, Series, + Query?, and Docs block for each new metric. When the block or an answer is missing, ask for + it under "Questions for the author"; do not report it as a defect. When you cannot see the + description, ask the questions anyway. Check that the stated series arithmetic matches the + declared labels and buckets: buckets + 3 per histogram label combination in Rust and Go, and + buckets + 4 in Python, where each counter label combination is also 2 series. + +9. **A2, A4, A5.** Search `prometheus_names.rs`, the Metrics Catalog, and + `metrics-comparison.md` for an existing metric with the same measurement point, or one that + differs only by a modality, a direction, or a role. Report only a concrete match: name both + metrics, and give the PromQL that replaces the new one or the label to add. Report static + configuration exported as separate gauges, and give the `*_info` metric that replaces them. + +10. **A3.** Report a new `dynamo_` metric that copies a value the engine already exports, unless + the pull request says which engines it normalizes and documents the mapping. + +### Naming + +11. **N1-N5.** Check the exported name: the `dynamo__` prefix; a unit in base form + (not `_ms`, `_us`, `_ns`, `_percent`, `_kb`, or `_gb`); `_total` on a counter, after the + unit; no `_total` on a gauge; no label name or value in the name. The correction gives the + exact replacement name and, in Rust, the constant to change. + +12. **N6.** Read the update site and confirm that the value matches the name and help text. For + example, a `_seconds` histogram that observes milliseconds is a defect. N6 concerns the + quantity, unit, and measurement point. A change that only alters when the value is published, + such as batching or flushing updates, is not an N6 defect; raise stale wording on unchanged + lines as a question, not a finding. + +### Labels + +13. **L1, L3, L4.** Confirm that each declared label takes its values from a bounded set: an + enum, a constant, or a mapping with a fallback value. The frontend resolves `model` with + `ModelManager::metric_model_for`; other components use a fixed `unknown` value. Report a + label that mixes different quantities. + +14. **L5.** For a new declared `worker_id` or `dp_rank` label, check that the pull request names + the router or autoscaler that needs it, and find the code that removes the series when the + worker leaves (`remove_label_values`, `clear()`, or a collector that rebuilds at scrape + time). Ask when the consumer is not named; report when no removal path exists in a + long-lived process. + +15. **L6, R1.** Report a new labeled family whose children are created only in the request path, + or whose registration happens on first use, for example inside `OnceLock::get_or_init` on + the update path. Before reporting, search the code at the pull request head for a seeding + call at startup or at model registration. A registered family with no children does not + appear on `/metrics`. + +16. **L7.** For a new histogram, check for about 10 buckets, an edge at each named alert + threshold, and `model` as the only declared label. A Rust histogram created with `None` + buckets gets the default edges for seconds (0.005 to 10), which do not fit tokens or bytes. + When a correction introduces or changes a histogram, give a literal bucket list in the + metric's unit, and name the edge that matches each alert threshold or say that none is named. + +17. **R2.** Report a new metric with no production update site at the pull request head. Also + confirm that each request path that the name and help text cover updates the metric (for + example, streaming and unary, token and text modes, aggregated and disaggregated), and report + a path that registers the family but never updates it. + +### Stability + +18. **S1-S4.** For a change to an existing metric, read its catalog entry at the pull request + head: an entry with no `**Experimental.**` mark is stable, and one with `deprecated={true}` + is deprecated. Report, on a stable metric, a rename, a type or unit change, or an added or + removed label that does not add a new metric and deprecate the old one. Report a + deprecation that lacks the catalog mark, the help-text note, the replacement or PromQL + recipe, or the removal release. Report a removal of a stable metric that has not been + deprecated for two releases, or any removal on a `release/*` base branch. Search the in-repo + readers (dashboards under `dev/observability/` and `deploy/observability/`, and the planner) + for a renamed or removed name, and report each reader that still uses it. + +### Documentation + +19. **D1.** Report a new or changed Dynamo metric whose catalog entry the pull request does not + add or update, and a new entry that lacks the type, unit, labels and their values, or the + `**Experimental.**` mark. + +20. **D2.** When the pull request adds a metric and does not describe it in the `AGENTS.md` + nearest to the owning code, ask for that description under "Questions for the author" and + name the file. Do not ask for a new `AGENTS.md` file. + +### Logs and traces + +21. **T1, T2.** On added lines only, report a Rust request-path event that formats an ID into + its message instead of passing it as a field, a spawned request-path task that does not + carry the span (`.instrument(...)` or `in_current_span()`), a Python request-path warning or + error that omits `context.id()`, and Python fields passed with `extra=`, which the log + bridge drops. Do not report a log event for not being JSON; the output format is deployment + configuration. + +22. **T3.** Report a metric label added to follow one request across components; a span gives + that view. Report span parentage defects only as the runtime review prompt describes them, + and do not invent span naming rules. + +## Out of scope + +Do not report: + +- A violation on a line that the change does not add or modify, including code that the change + only moves, and the names that the guidelines list under "Existing names that break these + rules" when they already exist before the change (check with `git grep -n `). A + metric that the change registers, exports, or updates for the first time is not pre-existing + (step 4). +- Engine metrics that Dynamo passes through (`vllm:`, `sglang:`, `lmcache:`, and the engine's + `trtllm_` metrics), or changes to the prefix filters that pass them. +- A rule for which the pull request description records an exception with a reason. Mention the + exception once; do not argue it again. + +## Reporting findings + +Report as findings only rule violations that the change introduces and that you verified in the +code at the pull request head. Each finding states: + +- **Location:** the file and line of the definition or update. +- **Rule:** the rule IDs. +- **Problem:** the exported name, label, or line, quoted. +- **Consequence:** the concrete cost, such as the series count with its arithmetic, the + dashboard or alert that would break, a unit that readers would misread, or a series that is + absent until the first request. +- **Correction:** the exact change: the replacement name and constant, the label set, the + seeding call, the log event or PromQL to use instead, or the catalog entry to add. + +Keep one finding per underlying defect. When one fact breaks several rules, report one finding +that cites all of them; when one defect repeats across metrics added together, report it once +and list every location. Put findings about names, types, units, labels, stability, and +unbounded series first, because they are hardest to change after release. For example: + +```text +lib/llm/src/http/service/metrics.rs:1412 (N2, N3) +Problem: new IntCounterVec "prefill_wait_ms" exports dynamo_frontend_prefill_wait_ms{model}. +Consequence: the unit is not base form and the counter has no _total, so rate() panels show + milliseconds per second under a name that reads as a gauge; renaming after release breaks them. +Correction: record seconds and export dynamo_frontend_prefill_wait_seconds_total: rename the + constant to PREFILL_WAIT_SECONDS_TOTAL = "prefill_wait_seconds_total" in prometheus_names.rs. +``` + +Put information that only the author can give (a reader, a series count, a label's source, which +processes update a metric) in a separate "Questions for the author" block, and do not present it +as a defect. End with the metric table from step 5. If no finding qualifies, state that no new +findings were identified; this does not establish that the change is ready to approve. diff --git a/AGENTS.md b/AGENTS.md index d96da4a5c0b6..0e514bf31ec9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,8 @@ to it — edit only the canonical copy. Reach for the right group first: When reviewing frontend or runtime changes, also read the corresponding [frontend review prompt](.github/review-prompts/frontend.md) or [runtime review prompt](.github/review-prompts/runtime.md) for additional CODEOWNERS guidance. +When reviewing a change that adds or changes a metric, a log event, or a trace span, also apply +the [metrics review prompt](.github/review-prompts/metrics.md). Before adding or changing a metric, a log event, or a trace span, read [`.ai/metrics-guidelines.md`](.ai/metrics-guidelines.md). It decides between a metric, a log, diff --git a/CODEOWNERS b/CODEOWNERS index e15672777a55..c093abc3ee2f 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -676,6 +676,7 @@ requirements.txt /lib/kvbm-config/src/messenger.rs @ai-dynamo/dynamo-kv-memory-codeowners @ai-dynamo/dynamo-runtime-codeowners /lib/runtime/src/system_health.rs @ai-dynamo/dynamo-runtime-codeowners @ai-dynamo/dynamo-observability-codeowners /tests/serve/multimodal_profiles/ @ai-dynamo/dynamo-runtime-codeowners @ai-dynamo/dynamo-multimodal-codeowners +/.github/review-prompts/metrics.md @ai-dynamo/dynamo-ops-codeowners @ai-dynamo/dynamo-observability-codeowners /.github/review-prompts/runtime.md @ai-dynamo/dynamo-ops-codeowners @ai-dynamo/dynamo-runtime-codeowners /docs/fern/pages/use-cases/agents/ @ai-dynamo/dynamo-docs-codeowners @ai-dynamo/dynamo-agents-codeowners /examples/deployments/**/sglang/** @ai-dynamo/dynamo-operator-codeowners @ai-dynamo/dynamo-backend-sglang-codeowners From c62370c8c5f5e315e58584c0da8368f7b4977cc1 Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Thu, 29 Oct 2026 15:55:42 -0700 Subject: [PATCH 3/6] docs(agents): define the series count and disallow labels that grow with the fleet A6 now asks for two numbers per process, each as a formula and as a number at a stated scale: label combinations (the product of the declared labels' value counts, counting every value a process sees over its life) and series (combinations times 1 for a counter or gauge, buckets + 3 for a histogram, buckets + 4 in Python). A metric that can reach more than 100 label combinations in one process needs a justification in the pull request. L5 now disallows any label whose values grow with the number of workers, such as worker_id or dp_rank on a frontend, router, or planner metric; a per-worker value belongs in the worker's own metrics. Signed-off-by: Yuewei Na --- .ai/metrics-guidelines.md | 51 +++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/.ai/metrics-guidelines.md b/.ai/metrics-guidelines.md index 50d9b0441a72..3537818f3723 100644 --- a/.ai/metrics-guidelines.md +++ b/.ai/metrics-guidelines.md @@ -67,15 +67,28 @@ name and deprecate the old one. length, block size) go in one `*_info` metric with value 1 and each parameter as a label, for example `dynamo_model_info{model, context_length, kv_cache_block_size}`. -**A6. Series count.** Each distinct combination of label values is one time series, and -each series costs memory in Prometheus. Multiply the number of values of every label: -`model` (10 values) and `status` (3 values) make 30 series, so a counter with five small -labels can still create hundreds. A histogram adds one series per bucket plus `+Inf`, -`_sum`, and `_count`, so 10 buckets make 13 series per label combination. Write the total -in the pull request. The count is per process; the cost in Prometheus is that number times -the number of scraped processes. Python `prometheus_client` also exports a `_created` series -for every counter and histogram label combination: count 2 series per counter combination -and buckets + 4 per histogram combination. +**A6. Series count.** Write two numbers in the pull request, each as a formula and as a +number at a stated scale. Both are per process. + +- **Label combinations:** multiply the largest number of values that each label you declare + can reach. Name what drives each number: a fixed set (`status` has 3 values) or the + deployment's configuration (served models, LoRA adapters). Labels that the runtime adds + have one value per process; do not count them. Count every value a process can see over its + life: a series stays exported until code removes it, so models or adapters that come and go + accumulate. +- **Series:** label combinations times the series per combination: 1 for a counter or gauge, + and buckets + 3 for a histogram (`+Inf`, `_sum`, and `_count`). Python `prometheus_client` + also exports a `_created` series, so count 2 per counter combination and buckets + 4 per + histogram combination. Each series costs memory in Prometheus, and the cost across the fleet + is this number times the number of scraped processes. + +For example, a histogram with 10 buckets and `model` as its only label has `models` +combinations and `models x (10 + 3)` series: 130 per frontend at 10 models. A counter with +`model` and a 3-value `status` has `models x 3` combinations, and as many series in Rust or Go. + +If a metric can reach more than 100 label combinations in one process, justify it in the pull +request or use a log event or a trace instead (**L1**). A label whose values grow with the +number of workers is not allowed (**L5**). **A7. Documentation.** Document the metric in the same pull request (**D1**, **D2**). @@ -84,8 +97,8 @@ Write the answers as one block per metric in the pull request description: ```text Metric: dynamo_frontend_prefill_queue_wait_seconds (histogram) Reader: "Prefill queue wait p95" panel in the router dashboard; alert at 2 s -Labels: model (10 values) -Series: 10 x (10 buckets + 3) = 130 per frontend +Labels: model (served models; 10 in the reference deployment) +Series: models combinations; models x (10 buckets + 3) = 130 series per frontend at 10 models Query?: No. Existing stage_duration_seconds does not split out queue wait. Docs: Added to the Metrics Catalog page ``` @@ -147,11 +160,12 @@ existing sentinel uses `unknown`, as the endpoint picker's `set_served_model` do different quantities, such as `kind` with the values `requests` and `bytes`, should be separate metrics. Static `*_info` metrics are the exception. -**L5.** Do not add per-worker labels (`worker_id`, `dp_rank`) to a new metric unless an -autoscaler or the router needs them. A per-worker series must stop being exported when its -worker is gone, as `lib/llm/src/discovery/worker_monitor.rs` does with -`remove_label_values`. This rule is about labels that you declare. The `worker_id` that the -Rust runtime adds to a process's own metrics identifies that process and is not affected. +**L5.** Do not declare a label whose values grow with the number of workers, such as +`worker_id` or `dp_rank` on a frontend, router, or planner metric: every such process would +then hold series for the whole fleet. Export a per-worker value from the worker's own +metrics instead, where the `worker_id` that the runtime adds identifies the worker; that +label has one value per process and is not affected by this rule. On a worker's own metrics, +add a `dp_rank` label only when an autoscaler or the router needs per-rank values. **L6.** Pre-create the expected values of bounded labels (such as `status` and `endpoint`) at startup, so that `rate()` and `absent()` alerts work from the first scrape. Create the @@ -327,5 +341,6 @@ metric that repeats one of their defects must follow the rules. | `dynamo_component_kv_cache_hit_rate` | R2 | | `dynamo_operator_resources_total` | N4 | -Other existing names in `prometheus_names.rs` and elsewhere also break these rules. They -are not precedents either. +Other existing names in `prometheus_names.rs` and elsewhere also break these rules, such as +the frontend's `dynamo_frontend_worker_*` gauges, which carry `worker_id` and `dp_rank` +(**L5**). They are not precedents either. From ee32d4c5549e659465757c8a631bf110908ecdda Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Thu, 29 Oct 2026 15:56:05 -0700 Subject: [PATCH 4/6] docs(review): check both series numbers and report labels that grow with the fleet Signed-off-by: Yuewei Na --- .github/review-prompts/metrics.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/review-prompts/metrics.md b/.github/review-prompts/metrics.md index 300617a0c15b..4ee71bc0bad6 100644 --- a/.github/review-prompts/metrics.md +++ b/.github/review-prompts/metrics.md @@ -70,9 +70,12 @@ rule violation is a contract defect, not a style preference. 8. **A1-A6.** Check the pull request description for the Metric, Reader, Labels, Series, Query?, and Docs block for each new metric. When the block or an answer is missing, ask for it under "Questions for the author"; do not report it as a defect. When you cannot see the - description, ask the questions anyway. Check that the stated series arithmetic matches the - declared labels and buckets: buckets + 3 per histogram label combination in Rust and Go, and - buckets + 4 in Python, where each counter label combination is also 2 series. + description, ask the questions anyway. Check both numbers that **A6** asks for against the + declared labels and buckets: label combinations, and series (1 per combination for a counter + or gauge, buckets + 3 for a histogram in Rust and Go, and in Python buckets + 4 per histogram + and 2 per counter combination). Report a metric whose declared labels alone can exceed 100 + combinations in one process when the pull request does not justify it; ask when the count + depends on deployment values the diff does not show. 9. **A2, A4, A5.** Search `prometheus_names.rs`, the Metrics Catalog, and `metrics-comparison.md` for an existing metric with the same measurement point, or one that @@ -103,11 +106,11 @@ rule violation is a contract defect, not a style preference. `ModelManager::metric_model_for`; other components use a fixed `unknown` value. Report a label that mixes different quantities. -14. **L5.** For a new declared `worker_id` or `dp_rank` label, check that the pull request names - the router or autoscaler that needs it, and find the code that removes the series when the - worker leaves (`remove_label_values`, `clear()`, or a collector that rebuilds at scrape - time). Ask when the consumer is not named; report when no removal path exists in a - long-lived process. +14. **L5.** Report a new declared label whose values grow with the number of workers, such as + `worker_id` or `dp_rank` on a frontend, router, or planner metric. The correction exports + the value from the worker's own metrics, where the runtime's `worker_id` identifies it. For + a new `dp_rank` label on a worker's own metric, ask which autoscaler or router needs + per-rank values when the pull request does not say. 15. **L6, R1.** Report a new labeled family whose children are created only in the request path, or whose registration happens on first use, for example inside `OnceLock::get_or_init` on From 6b1e42f89cefcc43c53242a31fb185977a1b56a4 Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Fri, 30 Oct 2026 14:34:23 -0700 Subject: [PATCH 5/6] docs(agents): drop the code-location and existing-names sections from the metrics guidelines Signed-off-by: Yuewei Na --- .ai/metrics-guidelines.md | 75 ++------------------------------------- 1 file changed, 3 insertions(+), 72 deletions(-) diff --git a/.ai/metrics-guidelines.md b/.ai/metrics-guidelines.md index 3537818f3723..0b2ecbd84e52 100644 --- a/.ai/metrics-guidelines.md +++ b/.ai/metrics-guidelines.md @@ -106,7 +106,7 @@ Docs: Added to the Metrics Catalog page ## Naming Apply these rules to the exported name, which is what dashboards and alerts see. The APIs -assemble it from parts (see [Where things live](#where-things-live)). +can add a prefix or a suffix to the name in the code. **N1.** The exported name is snake_case and starts with `dynamo__`. @@ -183,8 +183,8 @@ not make it visible: pre-create its children as well (**L6**). **R2.** Do not add a metric that no production code updates. **R3.** When you add, rename, or remove a name in `lib/runtime/src/metrics/prometheus_names.rs`, -regenerate its Python mirror in the same pull request (see [Where things live](#where-things-live)). -Never edit the mirror by hand. +regenerate its Python mirror in the same pull request with +`cargo run -p dynamo-codegen --bin gen-python-prometheus-names`. Never edit the mirror by hand. ## Patterns to follow @@ -275,72 +275,3 @@ are separate fields when the JSONL console format is on **T3.** Use a trace span, which is sampled, to show where one request spent its time across components. Do not add a metric label to get that view. - -## Where things live - -### Rust - -- Add each name as a `pub const` suffix in its subsystem's `pub mod` in - `lib/runtime/src/metrics/prometheus_names.rs`. Then regenerate the Python mirror with - `cargo run -p dynamo-codegen --bin gen-python-prometheus-names`. Do not edit - `lib/bindings/python/src/dynamo/prometheus_names.py` by hand; the - `committed_python_matches_generated` test fails when it is stale. -- Create metrics with `.metrics().create_*` on the runtime, namespace, component, or - endpoint (`lib/runtime/src/metrics.rs`). Pass only the suffix: the API prepends - `dynamo_component_`, or the prefix given to `create_metric_with_options`. A full - `dynamo_...` name gets the prefix twice. -- The API adds the `dynamo_namespace`, `dynamo_component`, `dynamo_endpoint`, and - `worker_id` labels and rejects them as your own labels. In the `*vec` methods, the - argument named `const_labels` holds the variable label names. -- A histogram created with `None` buckets gets the `prometheus` crate defaults: 11 edges from - 0.005 to 10, sized for seconds. Pass buckets that fit the unit and the alert thresholds - (**L7**). -- The frontend's own families are in `lib/llm/src/http/service/metrics.rs`. Use the bounded - enums there (`Status`, `ErrorType`, `Endpoint`) for label values. -- Test that the code updates a metric by rendering the registry with `prometheus_expfmt()`, - as `lib/llm/tests/admission_dp_rank.rs` does. - -### Python - -- Python cannot create runtime-registered metrics. Create `prometheus_client` metrics in a - dedicated `CollectorRegistry`, and expose it with the helpers in - `components/src/dynamo/common/utils/prometheus.py`. Register it on both the exposition - callback and the typed (OTLP) callback; registering only one leaves the metrics out of - OTLP export. -- Name constants shared with Rust are in `dynamo.prometheus_names`, which is generated from - the Rust file. Add a shared name in Rust first. -- `prometheus_client` adds `_total` to a counter's exported name. Apply **N3** to the - exported name. -- Where SGLang's multiprocess mode needs it, import `prometheus_client` inside the function, - with a comment that says why. -- Test end to end with `find_metric_samples` in `tests/utils/prometheus.py`, which tells an - absent series from a zero one. - -### Go (operator) - -- Operator metrics are `client_golang` vectors with `Namespace: "dynamo_operator"` in - `deploy/operator/internal/observability/metrics.go`, registered once in `InitMetrics`. - -## Existing names that break these rules - -These metrics break a naming, type, or registration rule. Their owners have not yet decided -between a deprecating rename and a recorded exception. Do not copy them, and do not rename -them as a side effect of another change. The exemption covers these existing names only: a new -metric that repeats one of their defects must follow the rules. - -| Metric | Rule | -|--------|------| -| `dynamo_tokio_worker_mean_poll_time_ns` | N2 | -| `dynamo_frontend_detokenize_total_us` | N2, N3 | -| `dynamo_frontend_detokenize_token_count` | N3 | -| `dynamo_component_kv_cache_events_applied`, `dynamo_kvrouter_kv_cache_events_applied` | N3 | -| `dynamo_component_kv_cache_event_warnings`, `dynamo_kvrouter_kv_cache_event_warnings` | N3 | -| `dynamo_router_overhead_*_ms` | N2 | -| `kvbm_matched_tokens` | N1, N3 | -| `dynamo_component_gpu_cache_usage_percent` | N2 | -| `dynamo_component_kv_cache_hit_rate` | R2 | -| `dynamo_operator_resources_total` | N4 | - -Other existing names in `prometheus_names.rs` and elsewhere also break these rules, such as -the frontend's `dynamo_frontend_worker_*` gauges, which carry `worker_id` and `dp_rank` -(**L5**). They are not precedents either. From ff548fcd994f61588ba107cc945754ee08493484 Mon Sep 17 00:00:00 2001 From: Yuewei Na Date: Fri, 30 Oct 2026 14:34:32 -0700 Subject: [PATCH 6/6] docs(review): drop the reference to the removed existing-names list Signed-off-by: Yuewei Na --- .github/review-prompts/metrics.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/review-prompts/metrics.md b/.github/review-prompts/metrics.md index 4ee71bc0bad6..770fbb4c995f 100644 --- a/.github/review-prompts/metrics.md +++ b/.github/review-prompts/metrics.md @@ -169,10 +169,8 @@ rule violation is a contract defect, not a style preference. Do not report: - A violation on a line that the change does not add or modify, including code that the change - only moves, and the names that the guidelines list under "Existing names that break these - rules" when they already exist before the change (check with `git grep -n `). A - metric that the change registers, exports, or updates for the first time is not pre-existing - (step 4). + only moves. A metric that the change registers, exports, or updates for the first time is not + pre-existing (step 4). - Engine metrics that Dynamo passes through (`vllm:`, `sglang:`, `lmcache:`, and the engine's `trtllm_` metrics), or changes to the prefix filters that pass them. - A rule for which the pull request description records an exception with a reason. Mention the