feat: ship PrometheusRedisMetricsProvider and a cached get_redis_metrics - #37
Merged
Merged
Conversation
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #36.
Problem
AsyncRedisProviderreadsRedisMetricsProtocol | NoneandRedisMetricsimplements it, but the kit ships nothing that provides the one from the other, so every consumer carries the same dishka provider. AndRedisMetrics(prefix=None)twice raises Prometheus's duplicated-timeseries error, so a container rebuilt per test cannot ask for the collector twice.What ships
Two
featcommits.redis_client_kit.metrics.get_redis_metrics(prefix=None)— oneRedisMetricsper prefix on the default registry, cached, the shape of grpc-client-kit'sget_grpc_client_metrics.""andNoneare the same unprefixed instance, becauseRedisMetricstreats them alike.RedisMetricsitself is unchanged: building two by hand still raises, and the docstring and the docs say that is Prometheus and point at the getter.redis_client_kit.providers.PrometheusRedisMetricsProvider(*, prefix=None)—Scope.APP, requestsRedisSettingsProtocol, providesRedisMetricsProtocol | Noneasget_redis_metrics(prefix)whensettings.metrics_enabledandNoneotherwise, so it drops intoAsyncRedisProvider(provide_default_metrics=False)(or afterAsyncRedisProvider()) and the client is instrumented exactly when the settings say so.RedisSettingsProtocolgainsmetrics_enabled: boolfor that;BaseRedisSettingshas carried the field from the start, a hand-written settings class needs the one attribute, and the configuration guide's example now has it. Withmetrics_enabledon and themetricsextra missing, resolving the collector raises theImportErrornaming the extra, the way every other extra here fails.Rejected
PrometheusMetricsSettingsProtocol+default_prefixkeys (sqlalchemy-foundation-kit's shape). Those are the application's keys, and a kit provider that requests them makes every consumer provide them. The prefix is a constructor argument, as in grpc-client-kit.Nonewhen the extra is missing (grpc-client-kit's choice). Silent in production; this kit fails loud on a missing extra everywhere else.registry=parameter onRedisMetrics. Nothing here needs it.Tests
""isNone;RedisMetricstwice raises.AsyncRedisProvider(provide_default_metrics=False), PrometheusRedisMetricsProvider()): metrics off givesNoneand a plainRedis; metrics on givesInstrumentedRediswhose_metricsisget_redis_metrics();prefix=names the series; two containers in a row hand out the same collector; registered afterAsyncRedisProvider()it wins; a missing extra raisesImportErrornaming it.test_metrics_init.pyplants its mocked modules throughmonkeypatchnow, so they leavesys.modulesafter each test instead of staying there for the rest of the session (they fed the provider's lazy import aMagicMock).Gate:
uv run ruff check .,uv run ruff format --check .,uv run mypy redis_client_kit,uv run pytest -m unit— 160 passed, unit coverage 99.66%.Docs: advanced guide (metrics and Dishka sections), configuration guide, agents page, README.