[Bugfix][Router] Fix stale Prometheus endpoint health metrics (#888) (#936)

* fix(router): clear stale Prometheus endpoint health metrics (fixes #888)

Endpoint health gauges retained stale server= labels indefinitely when
a backend was removed from service discovery, causing Prometheus to
keep exporting the last known value for dead endpoints.

Changes:
- Add EndpointInfo.healthy field (default True) to carry health state
  from service discovery into the metrics handler
- Introduce _LABEL_GAUGES list and _clear_label_gauges() helper in
  metrics_router.py; call it at the start of every /metrics scrape so
  only currently-active endpoints appear in the output
- Update healthy_pods_total to use ep.healthy instead of a hardcoded 1
- Add test_stale_metrics.py covering: EndpointInfo.healthy default/
  override, stale label removal, full endpoint removal, unlabeled gauge
  preservation, _LABEL_GAUGES completeness, and post-clear repopulation
- Remove redundant inline comments and section headers throughout

Signed-off-by: prashansapkota <prashan.sapkota3456@gmail.com>

* fix(router): address review feedback on PR #936

- Add num_requests_waiting to _LABEL_GAUGES and import it so stale
  waiting-request series are cleared along with all other server labels
- Restore the metrics() endpoint docstring
- Expand test imports and expected set in
  test_label_gauges_list_contains_all_expected_gauges to cover all
  server-labeled gauges including num_requests_waiting

Signed-off-by: prashansapkota <prashan.sapkota3456@gmail.com>

* fix: remove unused CollectorRegistry import in test_stale_metrics.py

Signed-off-by: prashansapkota <prashan.sapkota3456@gmail.com>

---------

Signed-off-by: prashansapkota <prashan.sapkota3456@gmail.com>
Co-authored-by: Rui Zhang <51696593+ruizhang0101@users.noreply.github.com>

Author: prashansapkota

src/tests/test_stale_metrics.py

@@ -0,0 +1,131 @@
+import pytest
+from prometheus_client import REGISTRY, generate_latest
+
+from vllm_router.routers.metrics_router import _LABEL_GAUGES, _clear_label_gauges
+from vllm_router.service_discovery import EndpointInfo
+from vllm_router.services.metrics_service import (
+    avg_decoding_length,
+    avg_itl,
+    avg_latency,
+    current_qps,
+    gpu_prefix_cache_hit_rate,
+    gpu_prefix_cache_hits_total,
+    gpu_prefix_cache_queries_total,
+    healthy_pods_total,
+    num_decoding_requests,
+    num_prefill_requests,
+    num_requests_running,
+    num_requests_swapped,
+    num_requests_waiting,
+)
+
+
+def test_endpoint_info_healthy_defaults_to_true():
+    ep = EndpointInfo(
+        url="http://ep1:8000",
+        model_names=["llama"],
+        Id="id1",
+        added_timestamp=0,
+        model_label="default",
+        sleep=False,
+    )
+    assert ep.healthy is True
+
+
+def test_endpoint_info_healthy_can_be_set_false():
+    ep = EndpointInfo(
+        url="http://ep1:8000",
+        model_names=["llama"],
+        Id="id1",
+        added_timestamp=0,
+        model_label="default",
+        sleep=False,
+        healthy=False,
+    )
+    assert ep.healthy is False
+
+
+@pytest.fixture(autouse=True)
+def _reset_gauges():
+    """Clear every label-based gauge before and after each test."""
+    _clear_label_gauges()
+    yield
+    _clear_label_gauges()
+
+
+def test_cleared_gauge_removes_stale_labels():
+    """Stale server labels must disappear after _clear_label_gauges()."""
+    # Simulate two active endpoints
+    healthy_pods_total.labels(server="http://ep1:8000").set(1)
+    healthy_pods_total.labels(server="http://ep2:8000").set(1)
+
+    output = generate_latest(REGISTRY).decode()
+    assert "ep1" in output
+    assert "ep2" in output
+
+    # Clear all labels (simulates start of /metrics handler)
+    _clear_label_gauges()
+
+    # Re-populate only ep1 (ep2 was removed from service discovery)
+    healthy_pods_total.labels(server="http://ep1:8000").set(1)
+
+    output = generate_latest(REGISTRY).decode()
+    assert "ep1" in output
+    assert "ep2" not in output
+
+
+def test_clear_removes_all_labels_when_all_endpoints_gone():
+    """When every endpoint is removed, no server labels remain."""
+    current_qps.labels(server="http://ep1:8000").set(42)
+    num_requests_running.labels(server="http://ep1:8000").set(3)
+
+    _clear_label_gauges()
+
+    output = generate_latest(REGISTRY).decode()
+    assert "ep1" not in output
+
+
+def test_clear_does_not_affect_unlabeled_gauges():
+    """System gauges (CPU, memory, disk) have no labels and are unaffected."""
+    from vllm_router.routers.metrics_router import router_cpu_usage_percent
+
+    router_cpu_usage_percent.set(42.0)
+    healthy_pods_total.labels(server="http://ep1:8000").set(1)
+
+    _clear_label_gauges()
+
+    output = generate_latest(REGISTRY).decode()
+    assert "router_cpu_usage_percent" in output
+    assert "42.0" in output
+
+
+def test_label_gauges_list_contains_all_expected_gauges():
+    """Ensure every gauge we export with a server label is in _LABEL_GAUGES."""
+    expected = {
+        current_qps,
+        avg_decoding_length,
+        num_prefill_requests,
+        num_decoding_requests,
+        num_requests_running,
+        num_requests_waiting,
+        avg_latency,
+        avg_itl,
+        num_requests_swapped,
+        gpu_prefix_cache_hit_rate,
+        gpu_prefix_cache_hits_total,
+        gpu_prefix_cache_queries_total,
+        healthy_pods_total,
+    }
+    assert expected.issubset(set(_LABEL_GAUGES))
+
+
+def test_repopulate_after_clear_shows_correct_values():
+    """Values set after clear must reflect in Prometheus output."""
+    _clear_label_gauges()
+
+    healthy_pods_total.labels(server="http://new-ep:8000").set(1)
+    current_qps.labels(server="http://new-ep:8000").set(99.5)
+
+    output = generate_latest(REGISTRY).decode()
+    assert "new-ep" in output
+    assert "99.5" in output

src/vllm_router/routers/metrics_router.py

@@ -18,6 +18,7 @@
 from fastapi import APIRouter, Response
 from prometheus_client import CONTENT_TYPE_LATEST, Gauge, generate_latest
 
+from vllm_router.log import init_logger
 from vllm_router.service_discovery import get_service_discovery
 from vllm_router.services.metrics_service import (
     avg_decoding_length,
@@ -32,13 +33,15 @@
     num_prefill_requests,
     num_requests_running,
     num_requests_swapped,
+    num_requests_waiting,
 )
 from vllm_router.stats.engine_stats import get_engine_stats_scraper
 from vllm_router.stats.request_stats import get_request_stats_monitor
 
+logger = init_logger(__name__)
+
 metrics_router = APIRouter()
 
-# Define Gauges for system resource usage
 router_cpu_usage_percent = Gauge(
     "router_cpu_usage_percent",
     "CPU usage percent",
@@ -53,10 +56,30 @@
 )
 
 
-# --- Prometheus Metrics Endpoint ---
+_LABEL_GAUGES = [
+    current_qps,
+    avg_decoding_length,
+    num_prefill_requests,
+    num_decoding_requests,
+    num_requests_running,
+    num_requests_waiting,
+    avg_latency,
+    avg_itl,
+    num_requests_swapped,
+    gpu_prefix_cache_hit_rate,
+    gpu_prefix_cache_hits_total,
+    gpu_prefix_cache_queries_total,
+    healthy_pods_total,
+]
+
+
+def _clear_label_gauges() -> None:
+    for gauge in _LABEL_GAUGES:
+        gauge.clear()
+
+
 @metrics_router.get("/metrics")
 async def metrics():
-    # Retrieve request stats from the monitor.
     """
     Endpoint to expose Prometheus metrics for the vLLM router.
 
@@ -72,20 +95,17 @@ async def metrics():
         Response: A HTTP response containing the latest Prometheus metrics in
         the appropriate content type.
     """
+    _clear_label_gauges()
 
-    # Collect CPU utilization (short interval)
     cpu_percent = psutil.cpu_percent(interval=0.1)
     router_cpu_usage_percent.set(cpu_percent)
 
-    # Collect memory utilization
     memory_percent = psutil.virtual_memory().percent
     router_memory_usage_percent.set(memory_percent)
 
-    # Collect disk utilization on root filesystem
     disk_percent = psutil.disk_usage("/").percent
     router_disk_usage_percent.set(disk_percent)
 
-    # Existing vLLM router request statistics
     stats = get_request_stats_monitor().get_request_stats(time.time())
     for server, stat in stats.items():
         current_qps.labels(server=server).set(stat.qps)
@@ -99,7 +119,6 @@ async def metrics():
         avg_itl.labels(server=server).set(stat.avg_itl)
         num_requests_swapped.labels(server=server).set(stat.num_swapped_requests)
 
-    # Engine statistics (GPU prefix cache metrics)
     engine_stats = get_engine_stats_scraper().get_engine_stats()
     for server, engine_stat in engine_stats.items():
         gpu_prefix_cache_hit_rate.labels(server=server).set(
@@ -112,12 +131,8 @@ async def metrics():
             engine_stat.gpu_prefix_cache_queries_total
         )
 
-    # Service discovery health status
     endpoints = get_service_discovery().get_endpoint_info()
     for ep in endpoints:
-        healthy_pods_total.labels(server=ep.url).set(
-            1 if getattr(ep, "healthy", True) else 0
-        )
+        healthy_pods_total.labels(server=ep.url).set(1 if ep.healthy else 0)
 
-    # Return all metrics in Prometheus format
     return Response(generate_latest(), media_type=CONTENT_TYPE_LATEST)

src/vllm_router/service_discovery.py

@@ -96,34 +96,16 @@ def to_dict(self) -> Dict:
 
 @dataclass
 class EndpointInfo:
-    # Endpoint's url
     url: str
-
-    # Model names
     model_names: List[str]
-
-    # Endpoint Id
     Id: str
-
-    # Added timestamp
     added_timestamp: float
-
-    # Model label
     model_label: str
-
-    # Endpoint's sleep status
     sleep: bool
-
-    # Pod name
+    healthy: bool = True
     pod_name: Optional[str] = None
-
-    # Service name
     service_name: Optional[str] = None
-
-    # Namespace
     namespace: Optional[str] = None
-
-    # Model information including relationships
     model_info: Dict[str, ModelInfo] = None
 
     def __str__(self):