From 184fdd15b0ccf32ad5cedc4211e48ee314445a77 Mon Sep 17 00:00:00 2001 From: Alexey Shalaev <75322386+AlexeyShalaev@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:26:38 +0300 Subject: [PATCH 1/2] feat: add get_redis_metrics, one cached RedisMetrics per prefix Prometheus registers a metric name once per registry, so the second RedisMetrics(prefix=None) in a process raises "Duplicated timeseries in CollectorRegistry" -- what a test suite runs into when it rebuilds its container per test. get_redis_metrics(prefix=None) caches one instance per prefix on the default registry and hands the same one back afterwards, the shape grpc-client-kit's get_grpc_client_metrics has; "" and None are the same unprefixed instance, since RedisMetrics treats them alike. RedisMetrics itself is unchanged: building two by hand still raises, and the docstring and the docs now say so and point at the getter. --- README.md | 4 +++ docs/agents.md | 11 ++++-- docs/guide/advanced.md | 17 ++++++++++ redis_client_kit/metrics/__init__.py | 4 +-- redis_client_kit/metrics/redis.py | 32 +++++++++++++++++- tests/unit/metrics/test_redis_metrics.py | 43 ++++++++++++++++++++++++ 6 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 tests/unit/metrics/test_redis_metrics.py diff --git a/README.md b/README.md index 2da3c92..19c894a 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,10 @@ client = create_async_redis_client(settings, metrics=metrics) # - myapp_redis_connection_errors_total{error_type} ``` +Prometheus registers a metric name once per process, so a second `RedisMetrics(prefix="myapp")` +raises `ValueError`. `get_redis_metrics(prefix="myapp")` returns the one instance per prefix +instead, which is what a test suite that rebuilds its container per test wants. + ### With Dishka DI ```python diff --git a/docs/agents.md b/docs/agents.md index d700373..f5e7856 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -225,7 +225,7 @@ The rest lives one import deeper. | `redis_client_kit.protocols` | — | `RedisMetricsProtocol` | | `redis_client_kit.utils` | — | the three exported helpers, plus `mask_redis_kwargs(kwargs)` for logging and `WRITE_PROBE_TTL_S`, the write probe's expiry in seconds | | `redis_client_kit.settings` | `settings` | `BaseRedisSettings`, `RedisConnectionSettings`, `RedisClusterSettings`, `RedisPoolSettings`, `RedisRetrySettings`, `RedisSSLSettings`, `RedisResponseSettings` | -| `redis_client_kit.metrics` | `metrics` | `RedisMetrics`, `REDIS_COMMAND_DURATION_BUCKETS` | +| `redis_client_kit.metrics` | `metrics` | `RedisMetrics`, `get_redis_metrics`, `REDIS_COMMAND_DURATION_BUCKETS` | | `redis_client_kit.providers` | `providers` | `AsyncRedisProvider(check_health_on_startup=True, provide_default_metrics=True)` | Each optional module raises `ImportError` at import time when its extra is missing, naming @@ -250,6 +250,12 @@ Buckets are `REDIS_COMMAND_DURATION_BUCKETS` — `(0.0001, 0.0005, 0.001, 0.005, argument of `execute_command`, upper-cased — it is unbounded cardinality only if you send unbounded command names. +`get_redis_metrics(prefix=None)` returns the one `RedisMetrics` per prefix on the default +registry, creating it on the first call and handing the same instance back afterwards; +`""` and `None` are the same unprefixed instance. Use it wherever the collector may be +asked for twice in one process — a container rebuilt per test, above all — since a second +`RedisMetrics()` with the same prefix raises (rule 18). + ### Dishka ```python @@ -375,7 +381,8 @@ See rules 15 to 17. block startup. 18. **A `RedisMetrics` instance owns global Prometheus names.** Building a second one with the same prefix raises a duplicate-timeseries `ValueError` from the default registry. - Build one per process and inject it. + That is Prometheus, not this package: build one per process and inject it, or ask + `get_redis_metrics(prefix)` and get the same instance back on every call. 19. **Cluster clients record no pool statistics.** Single-node clients, async and sync, report `redis_pool_size` and `redis_pool_checked_out` from the pool's own containers before every command; `InstrumentedRedisCluster` reports neither. Command counts, diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index e0bd963..929168d 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -92,6 +92,23 @@ client = create_async_redis_client(settings, metrics=metrics) # - myapp_redis_connection_errors_total{error_type} ``` +### One Instance Per Prefix + +Prometheus registers a metric name once per registry, so a second +`RedisMetrics(prefix="myapp")` on the default registry raises +`ValueError: Duplicated timeseries in CollectorRegistry` — what a test suite runs into +when it builds a container per test. `get_redis_metrics(prefix=None)` caches one instance +per prefix and hands it back on every later call: + +```python +from redis_client_kit.metrics import get_redis_metrics + +metrics = get_redis_metrics(prefix="myapp") +assert get_redis_metrics(prefix="myapp") is metrics +``` + +The [Dishka provider](#with-metrics) calls the getter for you. + ### Metrics Configuration ```python diff --git a/redis_client_kit/metrics/__init__.py b/redis_client_kit/metrics/__init__.py index ad44899..c7159f9 100644 --- a/redis_client_kit/metrics/__init__.py +++ b/redis_client_kit/metrics/__init__.py @@ -5,6 +5,6 @@ if not HAS_PROMETHEUS: raise ImportError("prometheus-client not installed. Install redis-client-kit[metrics] to use metrics.") -from .redis import REDIS_COMMAND_DURATION_BUCKETS, RedisMetrics +from .redis import REDIS_COMMAND_DURATION_BUCKETS, RedisMetrics, get_redis_metrics -__all__ = ["REDIS_COMMAND_DURATION_BUCKETS", "RedisMetrics"] +__all__ = ["REDIS_COMMAND_DURATION_BUCKETS", "RedisMetrics", "get_redis_metrics"] diff --git a/redis_client_kit/metrics/redis.py b/redis_client_kit/metrics/redis.py index 844e38c..8604458 100644 --- a/redis_client_kit/metrics/redis.py +++ b/redis_client_kit/metrics/redis.py @@ -30,6 +30,9 @@ class RedisMetrics: >>> metrics = RedisMetrics(prefix="myapp") >>> metrics.record_command("GET", "success", 0.001) >>> metrics.record_pool_stats(pool_size=10, pool_checked_out=3) + + Prometheus registers a metric name once per registry, so a second instance with the + same prefix raises ``ValueError``; ``get_redis_metrics`` hands back the first one instead. """ def __init__(self, prefix: str | None = None) -> None: @@ -37,6 +40,10 @@ def __init__(self, prefix: str | None = None) -> None: Args: prefix: Optional metric name prefix (e.g., "myapp" -> "myapp_redis_pool_size") + + Raises: + ValueError: If a metric of the same name is already registered on the default + registry -- see ``get_redis_metrics`` for the second instance. """ metric_prefix = f"{prefix}_" if prefix else "" @@ -95,4 +102,27 @@ def record_pool_stats(self, pool_size: int, pool_checked_out: int) -> None: self.pool_checked_out.set(float(pool_checked_out)) -__all__ = ["REDIS_COMMAND_DURATION_BUCKETS", "RedisMetrics"] +_REDIS_METRICS_CACHE: dict[str | None, RedisMetrics] = {} + + +def get_redis_metrics(prefix: str | None = None) -> RedisMetrics: + """Get (or lazily create) the cached ``RedisMetrics`` for a prefix, on the default registry. + + Caching by prefix is what lets a container be rebuilt -- a test suite does it per test -- + without Prometheus refusing the second registration of the same series. + + Args: + prefix: The metric name prefix the instance was, or is, created with; ``""`` and + ``None`` are the same unprefixed instance. + + Returns: + The one instance for that prefix. + """ + key = prefix or None + metrics = _REDIS_METRICS_CACHE.get(key) + if metrics is None: + metrics = _REDIS_METRICS_CACHE[key] = RedisMetrics(prefix=key) + return metrics + + +__all__ = ["REDIS_COMMAND_DURATION_BUCKETS", "RedisMetrics", "get_redis_metrics"] diff --git a/tests/unit/metrics/test_redis_metrics.py b/tests/unit/metrics/test_redis_metrics.py new file mode 100644 index 0000000..ad1d0fb --- /dev/null +++ b/tests/unit/metrics/test_redis_metrics.py @@ -0,0 +1,43 @@ +"""Tests for the cached RedisMetrics getter.""" + +import pytest + +from redis_client_kit.metrics import RedisMetrics, get_redis_metrics + + +def test__redis_metrics__same_prefix_twice__raises_duplicated_timeseries() -> None: + """Prometheus registers a name once per registry; the second instance is refused.""" + # Arrange + RedisMetrics(prefix="twice_test") + + # Act & Assert + with pytest.raises(ValueError, match="Duplicated timeseries"): + RedisMetrics(prefix="twice_test") + + +def test__get_redis_metrics__no_prefix_twice__returns_the_cached_instance() -> None: + """A second container asking for the collector gets the one the first container registered.""" + # Act + first = get_redis_metrics() + second = get_redis_metrics(prefix=None) + + # Assert + assert first is second + assert isinstance(first, RedisMetrics) + + +def test__get_redis_metrics__different_prefixes__returns_different_instances() -> None: + # Act + first = get_redis_metrics(prefix="cache_a") + second = get_redis_metrics(prefix="cache_b") + + # Assert + assert first is not second + assert first.pool_size._name == "cache_a_redis_pool_size" + assert second.pool_size._name == "cache_b_redis_pool_size" + + +def test__get_redis_metrics__empty_prefix__is_the_unprefixed_instance() -> None: + """RedisMetrics treats "" as no prefix, so the cache must too or the second call would re-register.""" + # Act & Assert + assert get_redis_metrics(prefix="") is get_redis_metrics() From 3a8f97bc4990c0d0a31f570f69029863556c1bbc Mon Sep 17 00:00:00 2001 From: Alexey Shalaev <75322386+AlexeyShalaev@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:29:25 +0300 Subject: [PATCH 2/2] feat: ship PrometheusRedisMetricsProvider next to AsyncRedisProvider AsyncRedisProvider reads RedisMetricsProtocol | None and RedisMetrics implements it, but nothing in the kit provided the one from the other, so every consumer carried the same dishka provider. PrometheusRedisMetricsProvider (*, prefix=None) is that join: Scope.APP, requests RedisSettingsProtocol, provides RedisMetricsProtocol | None as get_redis_metrics(prefix) when settings.metrics_enabled and None otherwise -- so it drops into AsyncRedisProvider(provide_default_metrics=False) and the client is instrumented exactly when the settings say so, and a container rebuilt per test hands out the same collector instead of registering the series twice. RedisSettingsProtocol gains metrics_enabled: bool for that. BaseRedisSettings has carried the field from the start; a hand-written settings class needs the one attribute, and the configuration guide's example now has it. With metrics_enabled on and the metrics extra missing, resolving the collector raises the ImportError naming the extra, the way every other extra here fails. test_metrics_init.py now plants its mocked modules through monkeypatch, so they leave sys.modules after each test instead of staying there for the rest of the session and feeding the provider's lazy import a MagicMock. --- README.md | 5 +- docs/agents.md | 49 ++++--- docs/guide/advanced.md | 23 +++ docs/guide/configuration.md | 6 +- redis_client_kit/config.py | 1 + redis_client_kit/providers/__init__.py | 3 +- redis_client_kit/providers/metrics.py | 44 ++++++ tests/conftest.py | 1 + tests/unit/metrics/test_metrics_init.py | 26 ++-- tests/unit/providers/test_metrics_provider.py | 134 ++++++++++++++++++ tests/unit/providers/test_providers_init.py | 1 + 11 files changed, 256 insertions(+), 37 deletions(-) create mode 100644 redis_client_kit/providers/metrics.py create mode 100644 tests/unit/providers/test_metrics_provider.py diff --git a/README.md b/README.md index 19c894a..b01efc5 100644 --- a/README.md +++ b/README.md @@ -193,10 +193,11 @@ instead, which is what a test suite that rebuilds its container per test wants. ```python from dishka import make_async_container -from redis_client_kit.providers import AsyncRedisProvider +from redis_client_kit.providers import AsyncRedisProvider, PrometheusRedisMetricsProvider container = make_async_container( - AsyncRedisProvider(), + AsyncRedisProvider(provide_default_metrics=False), + PrometheusRedisMetricsProvider(), # RedisMetrics when settings.metrics_enabled, else None SettingsProvider(), # Your settings provider ) diff --git a/docs/agents.md b/docs/agents.md index f5e7856..7dcb5ef 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -137,7 +137,7 @@ raises `AttributeError` on the first one missing. | `response` | `RedisResponseSettings` | all defaults | | `key_prefix` | `str` | **required** — never read by this library | | `health_check_interval` | `int | None`, `ge=0` | `30` | -| `metrics_enabled` | `bool` | `False` — never read by this library | +| `metrics_enabled` | `bool` | `False` — read only by `PrometheusRedisMetricsProvider` | | Group | Field | Default | Notes | |---|---|---|---| @@ -226,7 +226,7 @@ The rest lives one import deeper. | `redis_client_kit.utils` | — | the three exported helpers, plus `mask_redis_kwargs(kwargs)` for logging and `WRITE_PROBE_TTL_S`, the write probe's expiry in seconds | | `redis_client_kit.settings` | `settings` | `BaseRedisSettings`, `RedisConnectionSettings`, `RedisClusterSettings`, `RedisPoolSettings`, `RedisRetrySettings`, `RedisSSLSettings`, `RedisResponseSettings` | | `redis_client_kit.metrics` | `metrics` | `RedisMetrics`, `get_redis_metrics`, `REDIS_COMMAND_DURATION_BUCKETS` | -| `redis_client_kit.providers` | `providers` | `AsyncRedisProvider(check_health_on_startup=True, provide_default_metrics=True)` | +| `redis_client_kit.providers` | `providers` | `AsyncRedisProvider(check_health_on_startup=True, provide_default_metrics=True)`, `PrometheusRedisMetricsProvider(prefix=None)` | Each optional module raises `ImportError` at import time when its extra is missing, naming the extra. The root package imports none of them. @@ -263,9 +263,7 @@ from dishka import Provider, Scope, make_async_container, provide from redis_client_kit import AsyncRedisClient from redis_client_kit.config import RedisSettingsProtocol -from redis_client_kit.metrics import RedisMetrics -from redis_client_kit.protocols import RedisMetricsProtocol -from redis_client_kit.providers import AsyncRedisProvider +from redis_client_kit.providers import AsyncRedisProvider, PrometheusRedisMetricsProvider from redis_client_kit.settings import BaseRedisSettings class AppProvider(Provider): @@ -273,13 +271,13 @@ class AppProvider(Provider): @provide def settings(self) -> RedisSettingsProtocol: - return BaseRedisSettings(key_prefix="myapp") + return BaseRedisSettings(key_prefix="myapp", metrics_enabled=True) - @provide - def metrics(self) -> RedisMetricsProtocol | None: # this exact annotation - return RedisMetrics(prefix="myapp") - -container = make_async_container(AsyncRedisProvider(), AppProvider()) # this order +container = make_async_container( + AsyncRedisProvider(provide_default_metrics=False), + PrometheusRedisMetricsProvider(prefix="myapp"), + AppProvider(), +) client = await container.get(AsyncRedisClient) ``` @@ -290,7 +288,14 @@ registers: | Argument | Default | Effect | |---|---|---| | `check_health_on_startup` | `True` | pings Redis before yielding the client, and raises when it does not answer; `False` registers the factory that yields immediately | -| `provide_default_metrics` | `True` | provides `RedisMetricsProtocol | None` as `None` so a container without metrics resolves; `False` leaves that type to your own provider | +| `provide_default_metrics` | `True` | provides `RedisMetricsProtocol | None` as `None` so a container without metrics resolves; `False` leaves that type to another provider | + +`PrometheusRedisMetricsProvider(*, prefix=None)` is that other provider: `Scope.APP`, +requests `RedisSettingsProtocol` and nothing else, provides `RedisMetricsProtocol | None` +as `get_redis_metrics(prefix)` when `settings.metrics_enabled` and as `None` otherwise. +With `metrics_enabled` on it needs the `metrics` extra, and raises `ImportError` naming it +when the collector is resolved. A collector of your own is a provider of the same key, +registered in its place. See rules 15 to 17. @@ -326,10 +331,11 @@ See rules 15 to 17. from a bad command is raised on the first try. The delay is `min(backoff_cap, backoff_base * 2**failures)` with no jitter, so the first retry waits exactly `backoff_base`. -7. **`key_prefix` and `metrics_enabled` are declared and never read.** `key_prefix` is - required by `BaseRedisSettings` and used by nothing in this package; - `metrics_enabled=True` does not turn on instrumentation. Passing `metrics=` to the - factory does, and it is the only thing that does. +7. **`key_prefix` is declared and never read; `metrics_enabled` is read by one thing.** + `key_prefix` is required by `BaseRedisSettings` and used by nothing in this package. + `metrics_enabled` is read only by `PrometheusRedisMetricsProvider`; the factory ignores + it, so `metrics_enabled=True` without that provider turns nothing on. Passing + `metrics=` to the factory does, and outside Dishka it is the only thing that does. 8. **`BaseRedisSettings` is grouped and forbids extras.** `BaseRedisSettings(host="…")` raises `ValidationError: Extra inputs are not permitted`. Pass `connection=RedisConnectionSettings(host="…")`. @@ -366,8 +372,9 @@ See rules 15 to 17. Dishka the last provider to claim a type wins — put it second and your metrics are silently dropped, leaving an uninstrumented client. `AsyncRedisProvider(provide_default_metrics=False)` registers no default, so order - stops mattering. Either way the annotation on your factory must be exactly - `RedisMetricsProtocol | None`; `RedisMetricsProtocol` is a different key. + stops mattering. This holds for `PrometheusRedisMetricsProvider` as much as for a + provider of your own; for your own, the annotation on the factory must be exactly + `RedisMetricsProtocol | None` — `RedisMetricsProtocol` is a different key. 16. **The provider registers one client factory, chosen at construction.** `AsyncRedisProvider()` registers `get_redis_with_health_check()`; `AsyncRedisProvider(check_health_on_startup=False)` registers `get_redis()` instead. @@ -434,12 +441,12 @@ client = redis.asyncio.Redis(**build_base_redis_kwargs(settings, asyncio=True), ``` ```python -# WRONG — metrics_enabled does nothing, and the client is never instrumented +# WRONG — the factory never reads metrics_enabled, and the client is never instrumented settings = BaseRedisSettings(key_prefix="myapp", metrics_enabled=True) client = create_async_redis_client(settings) -# RIGHT -client = create_async_redis_client(settings, metrics=RedisMetrics(prefix="myapp")) +# RIGHT — pass the collector; only PrometheusRedisMetricsProvider reads the flag, and only in Dishka +client = create_async_redis_client(settings, metrics=get_redis_metrics(prefix="myapp")) ``` ```python diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index 929168d..91b80d1 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -166,6 +166,29 @@ contact Redis at all. ### With Metrics +`PrometheusRedisMetricsProvider` provides `RedisMetricsProtocol | None` — the key +`AsyncRedisProvider` reads — as `get_redis_metrics(prefix)` when `settings.metrics_enabled` +is on and `None` when it is off, so the client is instrumented exactly when the settings +say so: + +```python +from redis_client_kit.providers import AsyncRedisProvider, PrometheusRedisMetricsProvider + +container = make_async_container( + AsyncRedisProvider(provide_default_metrics=False), + PrometheusRedisMetricsProvider(), # or PrometheusRedisMetricsProvider(prefix="myapp") + SettingsProvider(), +) +``` + +With `metrics_enabled` on it needs the `metrics` extra; without it, resolving the +collector raises `ImportError` naming the extra. The collector is the one +`get_redis_metrics(prefix)` returns, so a container built per test never registers the +same series twice. + +To wire a collector of your own — the `PrometheusRedisMetrics` above, say — provide the +same key yourself: + ```python from redis_client_kit.protocols import RedisMetricsProtocol diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index d3b82aa..51daf54 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -360,6 +360,7 @@ class MySettings: ssl: MySSL response: MyResponse health_check_interval: int = 30 + metrics_enabled: bool = False # Use it settings = MySettings( @@ -374,8 +375,9 @@ settings = MySettings( client = create_async_redis_client(settings) ``` -Every attribute the protocols name has to be there: the factory reads all of them and -raises `AttributeError` on the first one missing. The protocols are not +Every attribute the protocols name has to be there: the factory reads all of them but +`metrics_enabled`, which only `PrometheusRedisMetricsProvider` reads, and raises +`AttributeError` on the first one missing. The protocols are not `@runtime_checkable`, so `isinstance(settings, RedisSettingsProtocol)` raises `TypeError`. ## Configuration Best Practices diff --git a/redis_client_kit/config.py b/redis_client_kit/config.py index f404ef6..866a5fa 100644 --- a/redis_client_kit/config.py +++ b/redis_client_kit/config.py @@ -89,3 +89,4 @@ class RedisSettingsProtocol(Protocol): ssl: RedisSSLProtocol response: RedisResponseProtocol health_check_interval: int | None + metrics_enabled: bool diff --git a/redis_client_kit/providers/__init__.py b/redis_client_kit/providers/__init__.py index ddd9ae0..203c2b9 100644 --- a/redis_client_kit/providers/__init__.py +++ b/redis_client_kit/providers/__init__.py @@ -5,6 +5,7 @@ if not HAS_DISHKA: raise ImportError("dishka not installed. Install redis-client-kit[providers] to use AsyncRedisProvider.") +from .metrics import PrometheusRedisMetricsProvider from .redis import AsyncRedisProvider -__all__ = ["AsyncRedisProvider"] +__all__ = ["AsyncRedisProvider", "PrometheusRedisMetricsProvider"] diff --git a/redis_client_kit/providers/metrics.py b/redis_client_kit/providers/metrics.py new file mode 100644 index 0000000..e5b96b5 --- /dev/null +++ b/redis_client_kit/providers/metrics.py @@ -0,0 +1,44 @@ +"""Dishka provider for the Prometheus Redis metrics collector.""" + +from ..config import RedisSettingsProtocol +from ..protocols import RedisMetricsProtocol +from ._deps import Provider, Scope, provide + + +class PrometheusRedisMetricsProvider(Provider): # type: ignore[misc] + """Dishka provider for the collector ``AsyncRedisProvider`` records into. + + Provides ``RedisMetricsProtocol | None`` -- the key ``AsyncRedisProvider`` reads -- as + ``get_redis_metrics(prefix)`` when ``settings.metrics_enabled`` and ``None`` otherwise, + so the client is instrumented exactly when the settings say so. Register it with + ``AsyncRedisProvider(provide_default_metrics=False)``, or after ``AsyncRedisProvider()``, + since the last provider of a type wins. + + The collector comes from ``get_redis_metrics``: one instance per prefix on the default + registry, so a container rebuilt per test never asks Prometheus to register the same + series twice. Resolving it with metrics on needs the ``metrics`` extra; without it the + import raises ``ImportError`` naming the extra. + """ + + scope = Scope.APP # type: ignore[misc] + + def __init__(self, *, prefix: str | None = None) -> None: + """Remember the prefix the collector is created with. + + Args: + prefix: Metric name prefix (``"myapp"`` gives ``myapp_redis_pool_size``) + """ + super().__init__() + self._prefix = prefix + + @provide + def get_metrics(self, redis_settings: RedisSettingsProtocol) -> RedisMetricsProtocol | None: + """Provide the collector when ``metrics_enabled`` is on, ``None`` otherwise.""" + if not redis_settings.metrics_enabled: + return None + from ..metrics import get_redis_metrics # noqa: PLC0415 - lazy: needs the [metrics] extra + + return get_redis_metrics(self._prefix) + + +__all__ = ["PrometheusRedisMetricsProvider"] diff --git a/tests/conftest.py b/tests/conftest.py index 65ae447..7ec067c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -71,5 +71,6 @@ def mock_redis_settings() -> MagicMock: settings.ssl = ssl settings.response = response settings.health_check_interval = 30 + settings.metrics_enabled = False return settings diff --git a/tests/unit/metrics/test_metrics_init.py b/tests/unit/metrics/test_metrics_init.py index 573df87..4e5eaeb 100644 --- a/tests/unit/metrics/test_metrics_init.py +++ b/tests/unit/metrics/test_metrics_init.py @@ -6,17 +6,24 @@ import pytest +def _unload_metrics(monkeypatch: pytest.MonkeyPatch) -> None: + """Drop the metrics package from sys.modules for the duration of one test. + + monkeypatch restores every entry afterwards, so the modules the rest of the + suite already imported keep pointing at the real ones. + """ + for module in [key for key in sys.modules if key.startswith("redis_client_kit.metrics")]: + monkeypatch.delitem(sys.modules, module) + + def test__metrics_init__prometheus_not_installed__raises_import_error(monkeypatch: pytest.MonkeyPatch) -> None: # Arrange - # Remove redis_client_kit.metrics from modules if already imported - modules_to_remove = [key for key in sys.modules if key.startswith("redis_client_kit.metrics")] - for module in modules_to_remove: - del sys.modules[module] + _unload_metrics(monkeypatch) # Mock _deps to simulate prometheus not installed mock_deps = MagicMock() mock_deps.HAS_PROMETHEUS = False - sys.modules["redis_client_kit.metrics._deps"] = mock_deps + monkeypatch.setitem(sys.modules, "redis_client_kit.metrics._deps", mock_deps) # Act & Assert with pytest.raises(ImportError, match="prometheus-client not installed"): @@ -25,18 +32,15 @@ def test__metrics_init__prometheus_not_installed__raises_import_error(monkeypatc def test__metrics_init__prometheus_installed__imports_successfully(monkeypatch: pytest.MonkeyPatch) -> None: # Arrange - # Remove redis_client_kit.metrics from modules if already imported - modules_to_remove = [key for key in sys.modules if key.startswith("redis_client_kit.metrics")] - for module in modules_to_remove: - del sys.modules[module] + _unload_metrics(monkeypatch) # Mock _deps to simulate prometheus installed mock_deps = MagicMock() mock_deps.HAS_PROMETHEUS = True mock_deps.REDIS_COMMAND_DURATION_BUCKETS = (0.001, 0.01, 0.1, 1.0) mock_deps.RedisMetrics = MagicMock - sys.modules["redis_client_kit.metrics._deps"] = mock_deps - sys.modules["redis_client_kit.metrics.redis"] = mock_deps + monkeypatch.setitem(sys.modules, "redis_client_kit.metrics._deps", mock_deps) + monkeypatch.setitem(sys.modules, "redis_client_kit.metrics.redis", mock_deps) # Act import redis_client_kit.metrics # noqa: F401, PLC0415 diff --git a/tests/unit/providers/test_metrics_provider.py b/tests/unit/providers/test_metrics_provider.py new file mode 100644 index 0000000..724b907 --- /dev/null +++ b/tests/unit/providers/test_metrics_provider.py @@ -0,0 +1,134 @@ +"""Tests for PrometheusRedisMetricsProvider next to AsyncRedisProvider.""" + +import sys +from unittest.mock import MagicMock + +import pytest +from dishka import AsyncContainer, Provider, Scope, make_async_container +from redis.asyncio import Redis + +from redis_client_kit import AsyncRedisClient +from redis_client_kit.aio import InstrumentedRedis +from redis_client_kit.config import RedisSettingsProtocol +from redis_client_kit.metrics import RedisMetrics, get_redis_metrics +from redis_client_kit.protocols import RedisMetricsProtocol +from redis_client_kit.providers import AsyncRedisProvider, PrometheusRedisMetricsProvider + + +def _settings_provider(settings: MagicMock) -> Provider: + provider = Provider(scope=Scope.APP) + provider.provide(lambda: settings, provides=RedisSettingsProtocol) + return provider + + +def _container(settings: MagicMock, *, prefix: str | None = None) -> AsyncContainer: + """The reporter's container: the kit's client provider with its default metrics off, and ours.""" + return make_async_container( + AsyncRedisProvider(check_health_on_startup=False, provide_default_metrics=False), + PrometheusRedisMetricsProvider(prefix=prefix), + _settings_provider(settings), + ) + + +@pytest.mark.asyncio +async def test__metrics_provider__metrics_disabled__client_is_plain(mock_redis_settings: MagicMock) -> None: + # Arrange + mock_redis_settings.metrics_enabled = False + container = _container(mock_redis_settings) + + # Act + metrics = await container.get(RedisMetricsProtocol | None) + client = await container.get(AsyncRedisClient) + + # Assert + assert metrics is None + assert type(client) is Redis + await container.close() + + +@pytest.mark.asyncio +async def test__metrics_provider__metrics_enabled__client_gets_the_cached_collector( + mock_redis_settings: MagicMock, +) -> None: + # Arrange + mock_redis_settings.metrics_enabled = True + container = _container(mock_redis_settings) + + # Act + client = await container.get(AsyncRedisClient) + + # Assert + assert isinstance(client, InstrumentedRedis) + assert client._metrics is get_redis_metrics() + await container.close() + + +@pytest.mark.asyncio +async def test__metrics_provider__prefix__names_the_collector(mock_redis_settings: MagicMock) -> None: + # Arrange + mock_redis_settings.metrics_enabled = True + container = _container(mock_redis_settings, prefix="provider_prefix_test") + + # Act + metrics = await container.get(RedisMetricsProtocol | None) + + # Assert + assert isinstance(metrics, RedisMetrics) + assert metrics.pool_size._name == "provider_prefix_test_redis_pool_size" + await container.close() + + +@pytest.mark.asyncio +async def test__metrics_provider__container_rebuilt__does_not_register_twice(mock_redis_settings: MagicMock) -> None: + """The per-test container: the second build hands out the collector the first one registered.""" + # Arrange + mock_redis_settings.metrics_enabled = True + first = _container(mock_redis_settings) + second = _container(mock_redis_settings) + + # Act + first_metrics = await first.get(RedisMetricsProtocol | None) + second_metrics = await second.get(RedisMetricsProtocol | None) + + # Assert + assert first_metrics is second_metrics + await first.close() + await second.close() + + +@pytest.mark.asyncio +async def test__metrics_provider__registered_after_default__wins(mock_redis_settings: MagicMock) -> None: + """AsyncRedisProvider() keeps its None default; the last provider of the type wins, so ours must follow it.""" + # Arrange + mock_redis_settings.metrics_enabled = True + container = make_async_container( + AsyncRedisProvider(check_health_on_startup=False), + PrometheusRedisMetricsProvider(), + _settings_provider(mock_redis_settings), + ) + + # Act + client = await container.get(AsyncRedisClient) + + # Assert + assert isinstance(client, InstrumentedRedis) + await container.close() + + +@pytest.mark.asyncio +async def test__metrics_provider__metrics_extra_missing__raises_import_error_naming_the_extra( + mock_redis_settings: MagicMock, monkeypatch: pytest.MonkeyPatch +) -> None: + # Arrange + mock_redis_settings.metrics_enabled = True + container = _container(mock_redis_settings) + for module in [key for key in sys.modules if key.startswith("redis_client_kit.metrics")]: + monkeypatch.delitem(sys.modules, module) + mock_deps = MagicMock() + mock_deps.HAS_PROMETHEUS = False + monkeypatch.setitem(sys.modules, "redis_client_kit.metrics._deps", mock_deps) + + # Act & Assert + with pytest.raises(ImportError, match=r"redis-client-kit\[metrics\]"): + await container.get(RedisMetricsProtocol | None) + await container.close() diff --git a/tests/unit/providers/test_providers_init.py b/tests/unit/providers/test_providers_init.py index 25e074d..3c89f4e 100644 --- a/tests/unit/providers/test_providers_init.py +++ b/tests/unit/providers/test_providers_init.py @@ -39,6 +39,7 @@ def test__providers_init__dishka_installed__imports_successfully(monkeypatch: py mock_deps.HAS_DISHKA = True mock_deps.AsyncRedisProvider = MagicMock monkeypatch.setitem(sys.modules, "redis_client_kit.providers._deps", mock_deps) + monkeypatch.setitem(sys.modules, "redis_client_kit.providers.metrics", mock_deps) monkeypatch.setitem(sys.modules, "redis_client_kit.providers.redis", mock_deps) monkeypatch.setitem(sys.modules, "redis_client_kit.providers.utils", mock_deps)