[PeaBrane]

Add router-level Prometheus metrics and centralize request tracking

Summary

• Add component-scoped aggregate Prometheus metrics at the router level: request count, TTFT, ITL, ISL, OSL
• Add per-request tracking fields to RequestTracker: ISL tokens, cached tokens, OSL, and average ITL
• Add per-worker last gauge reporting for the Python binding path (TTFT, ISL, ITL)

Changes

Aggregate metrics (RouterRequestMetrics)

• router_requests_total (counter), router_time_to_first_token_seconds, router_inter_token_latency_seconds, router_input_sequence_tokens, router_output_sequence_tokens (histograms)
• Component-scoped via from_component() pattern, router_ prefix distinguishes from ResponseMetricCollector metrics
• Observed in push_router.rs: ISL after routing, TTFT on first token, OSL/avg ITL/num_req at stream end
• Covers both Rust HTTP and Python binding paths

RequestTracker fields

• isl_tokens and cached_tokens: recorded at routing time
• osl_tokens: recorded at stream end
• first_token_time and request_finish_time: now recorded in push_router.rs (Mutex-based to support both prefill and decode phases in disaggregated serving)
• avg_itl_ms(): computed from finish time, first token time, and OSL

Per-worker last gauges for Python path

• observe_first_token_gauges() on RequestTracker: writes TTFT and ISL to WORKER_LAST_TIME_TO_FIRST_TOKEN_GAUGE and WORKER_LAST_INPUT_SEQUENCE_TOKENS_GAUGE
• observe_finish_gauges() on RequestTracker: writes avg ITL to WORKER_LAST_INTER_TOKEN_LATENCY_GAUGE
• Called from kv.rs stream processing (Python path only)

DIS-1412

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced observability with routing performance metrics tracking individual phases (hashing, matching, scheduling).
  • Improved timing accuracy for first-token and inter-token latency measurements.
  • Consolidated worker load metrics for better resource monitoring.

• Refactor

  • Streamlined internal metrics infrastructure and centralized metric collection endpoints.

[coderabbitai]

Walkthrough

This pull request refactors metrics instrumentation across the KV router subsystem by centralizing scattered Prometheus gauge definitions and event publishing into a new dedicated metrics module, while reworking timing tracker internals to support multiple phase updates via mutex-backed storage instead of single-shot locking.

Changes

Cohort / File(s)	Summary
Metrics Module Consolidation lib/llm/src/kv_router/metrics.rs	New module introducing centralized WorkerLoadMetrics, RoutingOverheadMetrics, and RouterRequestMetrics structures with registration helpers and per-event observation methods for worker gauges and routing phase latencies.
Worker Metrics Migration lib/llm/src/discovery.rs, lib/llm/src/discovery/worker_monitor.rs, lib/llm/src/kv_router/sequence.rs	Removed global gauge statics and re-exports; introduced WORKER_LOAD_METRICS centralized holder with updated cleanup logic and observe() method replacing direct gauge interactions.
KV Router Instrumentation lib/llm/src/kv_router.rs, lib/llm/src/kv_router/prefill_router.rs, lib/llm/src/kv_router/push_router.rs	Exposed public metrics module; removed KV_HIT_RATE_SUBJECT; added unconditional timing and routing overhead metric recording; simplified prefill metrics; enhanced push router with request metrics tracking and per-token observations.
Scheduler and Event Plumbing lib/llm/src/kv_router/scheduler.rs	Removed KVHitRateEvent and event publisher; changed SchedulingRequest::respond() to accept &mut self for transmitter ownership transfer.
Timing Infrastructure lib/llm/src/protocols/common/timing.rs	Replaced OneLock<Instant> with Mutex<Option<Instant>> for first-token and finish timings; added token sequence metrics (isl_tokens, cached_tokens, osl_tokens); introduced gauge observation methods observe_first_token_gauges() and observe_finish_gauges(); removed individual worker recording helpers in favor of record_worker_full().
HTTP Service and Delta Response Handlers lib/llm/src/http/service/service_v2.rs, lib/llm/src/protocols/openai/chat_completions/delta.rs, lib/llm/src/protocols/openai/completions/delta.rs	Added routing overhead metrics registration during service init; removed per-token first-token timing records and simplified finish-time handling in delta response paths.
Python KV Streaming lib/bindings/python/rust/llm/kv.rs	Added per-worker gauge observation hooks for first-token and finish events during KV push stream processing.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Metrics hop into one cozy warren,
No more scattered gauges to follow—
Timers now dance with mutex's embrace,
Routing phases tick their overhead trace,
Consolidated, centered, a metrics love affair! 📊✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main changes: adding router-level Prometheus metrics and centralizing request tracking.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check	✅ Passed	The PR description provides a clear summary, detailed changes, and aligns with the code modifications in the raw_summary.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/llm/src/protocols/common/timing.rs (1)

60-72: ⚠️ Potential issue | 🟡 Minor

Stale doc comments reference OnceLock for fields now backed by Mutex.

Lines 65–66 mention "set once via OnceLock" for first_token_time, and lines 70–72 state "The OnceLock fields ensure that values are set exactly once." Both first_token_time and request_finish_time are now Mutex<Option<Instant>> and can be overwritten. The doc should reflect the new semantics.

Proposed doc fix

-/// - `first_token_time`: When the first token was generated (set once via OnceLock)
-/// - `request_finish_time`: When the request finished (set once via OnceLock)
+/// - `first_token_time`: When the first token was generated (Mutex-protected, may be updated across phases)
+/// - `request_finish_time`: When the request finished (Mutex-protected, may be updated across phases)
 /// - KV cache hit rate information
 /// - Worker IDs and types for per-worker Prometheus metrics
 ///
-/// The `OnceLock` fields ensure that values are set exactly once,
-/// which is important for disaggregated serving where the "first token"
-/// might appear multiple times.
+/// The `Mutex`-protected time fields allow updates from multiple router phases
+/// (prefill/decode) in disaggregated serving, where timing events may be
+/// recorded by different pipeline stages.

🤖 Fix all issues with AI agents

In `@lib/llm/src/kv_router/metrics.rs`:
- Around line 258-287: The fallback constructor new_unregistered uses plain
Histogram::with_opts(...) and thus ends up with default Prometheus buckets;
update new_unregistered to mirror the same custom bucket configuration used in
from_component for the router histograms (e.g.,
router_time_to_first_token_seconds, router_inter_token_latency_seconds,
router_input_sequence_tokens, router_output_sequence_tokens) by applying the
identical HistogramOpts::buckets(...) call(s) so tests/fallbacks use the same
bucket boundaries as from_component.
- Around line 200-256: The router histograms currently pass buckets: None to
metrics.create_histogram which causes Prometheus to use inappropriate default
time buckets; update from_component (and the histograms created via
metrics.create_histogram for "router_time_to_first_token_seconds",
"router_inter_token_latency_seconds", "router_input_sequence_tokens", and
"router_output_sequence_tokens" inside the ROUTER_REQUEST_METRICS
initialization) to supply explicit bucket arrays matching the frontend Metrics
(log-spaced token buckets for input/output and appropriate time buckets for TTFT
and ITL). Replace the None bucket arguments with arrays mirroring the frontend
ranges (e.g., input sequence tokens ~50–128000 log-spaced, output sequence
tokens ~50–32000 log-spaced, TTFT ~0.001–480 seconds log-spaced, ITL ~0.001–2
seconds log-spaced) so the histograms preserve distribution visibility and then
re-run tests to ensure no metric creation errors.

🧹 Nitpick comments (3)

lib/llm/src/protocols/common/timing.rs (1)

256-265: avg_itl_ms looks correct; note the implicit panic if times are inverted.

finish.duration_since(first_token) will panic if finish < first_token. Under normal operation this is impossible, but with the new overwrite semantics, a mis-ordered call to record_first_token/record_finish from different phases could trigger it. If you'd prefer a defensive approach, checked_duration_since would return None instead of panicking. Given the fail-fast philosophy this is fine as-is, just flagging for awareness.

lib/llm/src/kv_router/metrics.rs (2)

132-151: Duration subtraction panics if cumulative timestamps arrive out of order.

The delta calculations (e.g., find_matches_elapsed - hash_elapsed) use Duration subtraction, which panics on underflow. This relies on callers passing strictly monotonic cumulative durations. If that invariant is guaranteed by the routing pipeline, this is fine — just calling it out since the observe() method accepts arbitrary Duration arguments from the outside.

Defensive alternative using saturating subtraction

     self.indexer_find_matches
-        .observe((find_matches_elapsed - hash_elapsed).as_secs_f64() * 1000.0);
+        .observe(find_matches_elapsed.saturating_sub(hash_elapsed).as_secs_f64() * 1000.0);
     self.seq_hashing
-        .observe((seq_hash_elapsed - find_matches_elapsed).as_secs_f64() * 1000.0);
+        .observe(seq_hash_elapsed.saturating_sub(find_matches_elapsed).as_secs_f64() * 1000.0);
     self.scheduling
-        .observe((total_elapsed - seq_hash_elapsed).as_secs_f64() * 1000.0);
+        .observe(total_elapsed.saturating_sub(seq_hash_elapsed).as_secs_f64() * 1000.0);

---

50-73: Hardcoded dynamo_frontend_ prefix bypasses the configurable DYN_METRICS_PREFIX.

The frontend Metrics (in http/service/metrics.rs) reads DYN_METRICS_PREFIX to allow users to customize the metric namespace. The worker load gauges here hardcode dynamo_frontend_, so they won't follow any user-configured prefix. This appears to be pre-existing behavior carried over from the old globals, but worth noting if prefix consistency is a goal.

[keivenchang]

Hey Rudy, good direction -- always-on routing overhead histograms and aggregate TTFT/ITL/ISL/OSL at the router level. Consolidating the record_worker methods is a nice cleanup too.

1. I see mixed metric patterns: RoutingOverheadMetrics uses raw LazyLock
   (dynamo_routing_overhead_), RouterRequestMetrics uses from_component()
   (dynamo_component_router_), can you address this in the comments.

2. Duration subtraction in RoutingOverheadMetrics::observe() can panic.

3. Can you add tests for the added Prometheus metrics? You can re-use prometheus_names.rs in the test names.

Add #[cfg(test)] module in your lib/llm/src/kv_router/metrics.rs and lib/llm/src/protocols/common/timing.rs. At minimum, test that the Prometheus Exposition Format (PEF text output) is generated -- compare against a golden output. An example of PEF is at lib/runtime/src/metrics.rs (search for expected_namespace_output), which has a PEF like this:

        let expected_namespace_output = r#"# HELP dynamo_component_testcounter A test counter
# TYPE dynamo_component_testcounter counter
dynamo_component_testcounter{dynamo_component="comp345",dynamo_endpoint="ep345",dynamo_namespace="ns345"} 123.456789
# HELP dynamo_component_testgauge A test gauge
# TYPE dynamo_component_testgauge gauge
dynamo_component_testgauge{dynamo_component="comp345",dynamo_namespace="ns345"} 50000
# HELP dynamo_component_testintcounter A test int counter
# TYPE dynamo_component_testintcounter counter
dynamo_component_testintcounter{dynamo_namespace="ns345"} 12345"#.to_string();