fix(metrics): address review findings before merge

- Replace all hardcoded test ports (9093-19200) with port=0 and read
  actual port from server_address[1] to prevent CI port collisions
- Clarify CacheMetrics.window_sizes docstring: windowing is not
  automatic; callers must pass window= explicitly to get_stats()
- Add README note that entry_count/total_size_bytes are populated for
  the memory backend only; all other backends report 0
- Standardize MetricsContext guards to 'if self._m is not None:'
- Remove dead _init_prometheus_metrics no-op method and its call site
- Replace deprecated typing.Deque/Dict with deque[...]/dict[...] builtins

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: Borda

README.rst

@@ -364,6 +364,8 @@ The metrics system tracks:
 * **Size limit rejections**: Entries rejected due to ``entry_size_limit``
 * **Cache size (memory backend only)**: Number of entries and total size in bytes for the in-memory cache core
 
+  Note: ``entry_count`` and ``total_size_bytes`` are populated only for the memory backend. Other backends (pickle, redis, sql, mongo) currently always report ``0`` for these fields.
+
 Sampling Rate
 -------------
 

src/cachier/exporters/prometheus.py

@@ -7,7 +7,7 @@
 # http://www.opensource.org/licenses/MIT-license
 
 import threading
-from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, Protocol, cast
+from typing import TYPE_CHECKING, Any, Callable, Optional, Protocol, cast
 
 from .base import MetricsExporter
 
@@ -60,7 +60,7 @@ def collect(self) -> Any:
         """Collect metrics from all registered functions."""
         # Snapshot all metrics in one lock acquisition for consistency
         with self.exporter._lock:
-            snapshots: Dict[str, "MetricSnapshot"] = {}
+            snapshots: dict[str, "MetricSnapshot"] = {}
             for func_name, func in self.exporter._registered_functions.items():
                 m = _get_func_metrics(func)
                 if m is not None:
@@ -144,7 +144,7 @@ def __init__(self, port: int = 9090, use_prometheus_client: bool = True, host: s
         self.port = port
         self.host = host
         self.use_prometheus_client = use_prometheus_client
-        self._registered_functions: Dict[str, _MetricsEnabledCallable] = {}
+        self._registered_functions: dict[str, _MetricsEnabledCallable] = {}
         self._lock = threading.Lock()
         self._server: Optional[Any] = None
         self._server_thread: Optional[threading.Thread] = None
@@ -156,7 +156,6 @@ def __init__(self, port: int = 9090, use_prometheus_client: bool = True, host: s
         self._registry: Optional[Any] = None
         if use_prometheus_client and PROMETHEUS_CLIENT_AVAILABLE:
             self._prom_client = prometheus_client
-            self._init_prometheus_metrics()
             self._setup_collector()
 
     def _setup_collector(self) -> None:
@@ -168,16 +167,6 @@ def _setup_collector(self) -> None:
         self._registry = CollectorRegistry()
         self._registry.register(CachierCollector(self))
 
-    def _init_prometheus_metrics(self) -> None:
-        """Initialize Prometheus metrics using prometheus_client.
-
-        Note: With custom collector, we don't need to pre-define metrics.
-        The collector will generate them dynamically at scrape time.
-
-        """
-        # Metrics are now handled by the custom collector in _setup_collector()
-        pass
-
     def register_function(self, func: Callable[..., Any]) -> None:
         """Register a cached function for metrics export.
 
@@ -232,7 +221,7 @@ def _generate_text_metrics(self) -> str:
         """
         # Snapshot all metrics in one lock acquisition for consistency
         with self._lock:
-            snapshots: Dict[str, "MetricSnapshot"] = {}
+            snapshots: dict[str, "MetricSnapshot"] = {}
             for func_name, func in self._registered_functions.items():
                 m = _get_func_metrics(func)
                 if m is not None:

src/cachier/metrics.py

@@ -12,7 +12,7 @@
 from collections import deque
 from dataclasses import dataclass
 from datetime import timedelta
-from typing import Deque, Optional
+from typing import Optional
 
 
 @dataclass
@@ -96,7 +96,10 @@ class CacheMetrics:
         Sampling rate for metrics collection (0.0-1.0), by default 1.0
         Lower values reduce overhead at the cost of accuracy
     window_sizes : list of timedelta, optional
-        Time windows to track for aggregated metrics, by default [1 minute, 1 hour, 1 day]
+        Informational window presets, by default [1 minute, 1 hour, 1 day].
+        These values are not applied automatically. To filter latency metrics
+        by time window, pass ``window=timedelta(...)`` explicitly to
+        ``get_stats()``.
 
     Examples
     --------
@@ -116,7 +119,9 @@ def __init__(self, sampling_rate: float = 1.0, window_sizes: Optional[list[timed
         sampling_rate : float
             Sampling rate between 0.0 and 1.0
         window_sizes : list of timedelta, optional
-            Time windows for aggregated metrics
+            Informational window presets. These values are stored but not
+            applied automatically; pass ``window=timedelta(...)`` to
+            ``get_stats()`` for time-windowed latency aggregation.
 
         """
         if not 0.0 <= sampling_rate <= 1.0:
@@ -147,7 +152,7 @@ def __init__(self, sampling_rate: float = 1.0, window_sizes: Optional[list[timed
         # Assuming ~1000 ops/sec max, keep 1 day of data = 86.4M points
         # Limit to 100K points for memory efficiency
         max_latency_points = 100000
-        self._latencies: Deque[_TimestampedMetric] = deque(maxlen=max_latency_points)
+        self._latencies: deque[_TimestampedMetric] = deque(maxlen=max_latency_points)
 
         # Size tracking
         self._entry_count = 0
@@ -355,30 +360,30 @@ def __exit__(self, *_: object) -> None:
 
     def record_hit(self) -> None:
         """Record a cache hit."""
-        if self._m:
+        if self._m is not None:
             self._m.record_hit()
 
     def record_miss(self) -> None:
         """Record a cache miss."""
-        if self._m:
+        if self._m is not None:
             self._m.record_miss()
 
     def record_stale_hit(self) -> None:
         """Record a stale cache hit."""
-        if self._m:
+        if self._m is not None:
             self._m.record_stale_hit()
 
     def record_recalculation(self) -> None:
         """Record a cache recalculation."""
-        if self._m:
+        if self._m is not None:
             self._m.record_recalculation()
 
     def record_wait_timeout(self) -> None:
         """Record a wait timeout."""
-        if self._m:
+        if self._m is not None:
             self._m.record_wait_timeout()
 
     def record_size_limit_rejection(self) -> None:
         """Record an entry rejection due to size limit."""
-        if self._m:
+        if self._m is not None:
             self._m.record_size_limit_rejection()

tests/test_exporters.py

@@ -54,7 +54,7 @@ def test_func(x):
 
     test_func.clear_cache()
 
-    exporter = PrometheusExporter(port=9093, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.register_function(test_func)
 
     # Generate some metrics
@@ -89,7 +89,7 @@ def func2(x):
     func1.clear_cache()
     func2.clear_cache()
 
-    exporter = PrometheusExporter(port=9094, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.register_function(func1)
     exporter.register_function(func2)
 
@@ -132,10 +132,10 @@ def test_func(x):
     test_func.clear_cache()
     test_func(5)
 
-    exporter1 = PrometheusExporter(port=9097, use_prometheus_client=False)
+    exporter1 = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter1.register_function(test_func)
 
-    exporter2 = PrometheusExporter(port=9098, use_prometheus_client=False)
+    exporter2 = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter2.register_function(test_func)
 
     # Both should generate valid metrics
@@ -158,7 +158,7 @@ def test_func(x):
 
     test_func.clear_cache()
 
-    exporter = PrometheusExporter(port=9099, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.register_function(test_func)
 
     test_func(5)  # miss
@@ -290,7 +290,7 @@ def test_func(x):
     test_func.clear_cache()
     test_func(5)
 
-    exporter = PrometheusExporter(port=19093, use_prometheus_client=True)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=True)
     assert exporter._prom_client is None
     exporter.register_function(test_func)
     text = exporter._generate_text_metrics()
@@ -303,8 +303,7 @@ def test_func(x):
 def test_prometheus_prom_client_available_paths():
     """Cover prometheus_client-available code paths via module-level patching.
 
-    Exercises: __init__ branch (L157-160), _setup_collector (L168-169),
-    _init_prometheus_metrics (L179), CachierCollector.describe (L57), and
+    Exercises: __init__ branch, _setup_collector, CachierCollector.describe, and
     CachierCollector.collect() None-metrics skip (L66 False branch).
 
     """
@@ -382,7 +381,7 @@ def test_prometheus_module_import_with_prom_client():
 @pytest.mark.memory
 def test_prometheus_stop_when_not_started():
     """Test that stop() is a no-op when the server was never started."""
-    exporter = PrometheusExporter(port=19094, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.stop()  # Should not raise
 
 
@@ -391,10 +390,11 @@ def test_prometheus_simple_server_404():
     """Test that simple HTTP server returns 404 for non-metrics paths."""
     import http.client
 
-    exporter = PrometheusExporter(port=19095, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.start()
+    actual_port = exporter._server.server_address[1]
     try:
-        conn = http.client.HTTPConnection("127.0.0.1", 19095)
+        conn = http.client.HTTPConnection("127.0.0.1", actual_port)
         conn.request("GET", "/notfound")
         response = conn.getresponse()
         assert response.status == 404
@@ -410,10 +410,11 @@ def test_prometheus_prometheus_server_404():
 
     pytest.importorskip("prometheus_client")
 
-    exporter = PrometheusExporter(port=19096, use_prometheus_client=True)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=True)
     exporter.start()
+    actual_port = exporter._server.server_address[1]
     try:
-        conn = http.client.HTTPConnection("127.0.0.1", 19096)
+        conn = http.client.HTTPConnection("127.0.0.1", actual_port)
         conn.request("GET", "/notfound")
         response = conn.getresponse()
         assert response.status == 404
@@ -428,7 +429,7 @@ def test_prometheus_collector_collect_empty():
     pytest.importorskip("prometheus_client")
     from prometheus_client import generate_latest
 
-    exporter = PrometheusExporter(port=19097, use_prometheus_client=True)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=True)
     assert exporter._registry is not None
     # No functions registered — collect() should run without error and yield metric families
     output = generate_latest(exporter._registry).decode()
@@ -448,11 +449,12 @@ def test_func(x):
     test_func.clear_cache()
     test_func(5)
 
-    exporter = PrometheusExporter(port=19098, use_prometheus_client=False)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=False)
     exporter.register_function(test_func)
     exporter.start()
+    actual_port = exporter._server.server_address[1]
     try:
-        response = urllib.request.urlopen("http://127.0.0.1:19098/metrics", timeout=5)
+        response = urllib.request.urlopen(f"http://127.0.0.1:{actual_port}/metrics", timeout=5)
         body = response.read().decode()
         assert "cachier_cache_hits_total" in body
     finally:
@@ -474,11 +476,12 @@ def test_func(x):
     test_func.clear_cache()
     test_func(5)
 
-    exporter = PrometheusExporter(port=19099, use_prometheus_client=True)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=True)
     exporter.register_function(test_func)
     exporter.start()
+    actual_port = exporter._server.server_address[1]
     try:
-        response = urllib.request.urlopen("http://127.0.0.1:19099/metrics")
+        response = urllib.request.urlopen(f"http://127.0.0.1:{actual_port}/metrics")
         body = response.read().decode()
         assert "cachier_cache_hits_total" in body
     finally:
@@ -568,7 +571,7 @@ def test_prometheus_collector_collect_skips_none_metrics():
     pytest.importorskip("prometheus_client")
     from prometheus_client import generate_latest
 
-    exporter = PrometheusExporter(port=19200, use_prometheus_client=True)
+    exporter = PrometheusExporter(port=0, use_prometheus_client=True)
 
     class _NoMetrics:
         __module__ = "test"