From 73ef80592cfeaa873a7f9fd081d51ecd600e4fda Mon Sep 17 00:00:00 2001 From: Alberto Donato Date: Tue, 24 Oct 2023 08:24:56 +0200 Subject: [PATCH] Rework MetricConfig, make labels an attribute (#7) --- README.rst | 13 +++-- prometheus_aioexporter/__init__.py | 10 ++-- .../{metric.py => _metric.py} | 56 +++++++++---------- .../{script.py => _script.py} | 16 +++--- prometheus_aioexporter/{web.py => _web.py} | 13 ++--- prometheus_aioexporter/sample.py | 21 +++++-- tests/metric_test.py | 54 ++++++++++++------ tests/script_test.py | 14 ++--- tests/web_test.py | 14 ++--- tox.ini | 3 +- 10 files changed, 123 insertions(+), 91 deletions(-) rename prometheus_aioexporter/{metric.py => _metric.py} (68%) rename prometheus_aioexporter/{script.py => _script.py} (94%) rename prometheus_aioexporter/{web.py => _web.py} (92%) diff --git a/README.rst b/README.rst index 007f9c1..8d21016 100644 --- a/README.rst +++ b/README.rst @@ -120,11 +120,14 @@ with a list of ``MetricConfig``\s. This is typically done in ``configure()``: .. code:: python - def configure(self, args: argparse.Namespace): + def configure(self, args: argparse.Namespace) -> None: # ... self.create_metrics( - [MetricConfig("metric1", "a metric", "gauge", {}), - MetricConfig("metric2", "another metric", "counter", {})]) + [ + MetricConfig("metric1", "a metric", "gauge"), + MetricConfig("metric2", "another metric", "counter", labels=("l1", "l2")), + ] + ) Web application setup @@ -145,11 +148,11 @@ coroutine and is called with a dict mapping metric names to metric objects: .. code:: python - async def on_application_startup(self, application): + async def on_application_startup(self, application: aiohttp.web.Application) -> None: # ... application["exporter"].set_metric_update_handler(self._update_handler) - async def _update_handler(self, metrics): + async def _update_handler(self, metrics: dict[str, prometheus_client.metrics.MetricWrapperBase]): for name, metric in metrics.items(): metric.set(...) diff --git a/prometheus_aioexporter/__init__.py b/prometheus_aioexporter/__init__.py index 546aeeb..fb4d870 100644 --- a/prometheus_aioexporter/__init__.py +++ b/prometheus_aioexporter/__init__.py @@ -1,16 +1,18 @@ """Asyncio library for creating Prometheus exporters.""" -from .metric import ( +from ._metric import ( + InvalidMetricType, MetricConfig, MetricsRegistry, ) -from .script import PrometheusExporterScript +from ._script import PrometheusExporterScript __all__ = [ - "__version__", + "InvalidMetricType", "MetricConfig", "MetricsRegistry", "PrometheusExporterScript", + "__version__", ] -__version__ = "1.7.0" +__version__ = "2.0.dev1" diff --git a/prometheus_aioexporter/metric.py b/prometheus_aioexporter/_metric.py similarity index 68% rename from prometheus_aioexporter/metric.py rename to prometheus_aioexporter/_metric.py index a698937..6994f58 100644 --- a/prometheus_aioexporter/metric.py +++ b/prometheus_aioexporter/_metric.py @@ -1,14 +1,11 @@ -"""Helpers around prometheus-client to create and register metrics.""" +"""Helpers around prometheus_client to create and register metrics.""" from collections.abc import Iterable from dataclasses import ( dataclass, field, ) -from typing import ( - Any, - NamedTuple, -) +from typing import Any from prometheus_client import ( CollectorRegistry, @@ -18,31 +15,28 @@ generate_latest, Histogram, Info, - Metric, Summary, ) +from prometheus_client.metrics import MetricWrapperBase from prometheus_client.registry import Collector -class MetricType(NamedTuple): +@dataclass(frozen=True) +class MetricType: """Details about a metric type.""" - cls: Metric - options: dict[str, str] = field(default_factory=dict) + cls: type[MetricWrapperBase] + options: list[str] = field(default_factory=list) # Map metric types to their MetricTypes METRIC_TYPES: dict[str, MetricType] = { - "counter": MetricType(cls=Counter, options={"labels": "labelnames"}), - "enum": MetricType( - cls=Enum, options={"labels": "labelnames", "states": "states"} - ), - "gauge": MetricType(cls=Gauge, options={"labels": "labelnames"}), - "histogram": MetricType( - cls=Histogram, options={"labels": "labelnames", "buckets": "buckets"} - ), - "info": MetricType(cls=Info, options={"labels": "labelnames"}), - "summary": MetricType(cls=Summary, options={"labels": "labelnames"}), + "counter": MetricType(cls=Counter), + "enum": MetricType(cls=Enum, options=["states"]), + "gauge": MetricType(cls=Gauge), + "histogram": MetricType(cls=Histogram, options=["buckets"]), + "info": MetricType(cls=Info), + "summary": MetricType(cls=Summary), } @@ -53,9 +47,11 @@ class MetricConfig: name: str description: str type: str - config: dict[str, Any] + labels: Iterable[str] = field(default_factory=tuple) + config: dict[str, Any] = field(default_factory=dict) def __post_init__(self) -> None: + self.labels = tuple(sorted(self.labels)) if self.type not in METRIC_TYPES: raise InvalidMetricType(self.name, self.type) @@ -79,13 +75,13 @@ class MetricsRegistry: def __init__(self) -> None: self.registry = CollectorRegistry(auto_describe=True) - self._metrics: dict[str, Metric] = {} + self._metrics: dict[str, MetricWrapperBase] = {} def create_metrics( self, configs: Iterable[MetricConfig] - ) -> dict[str, Metric]: + ) -> dict[str, MetricWrapperBase]: """Create Prometheus metrics from a list of MetricConfigs.""" - metrics: dict[str, Metric] = { + metrics: dict[str, MetricWrapperBase] = { config.name: self._register_metric(config) for config in configs } self._metrics.update(metrics) @@ -93,7 +89,7 @@ def create_metrics( def get_metric( self, name: str, labels: dict[str, str] | None = None - ) -> Metric: + ) -> MetricWrapperBase: """Return a metric, optionally configured with labels.""" metric = self._metrics[name] if labels: @@ -101,7 +97,7 @@ def get_metric( return metric - def get_metrics(self) -> dict[str, Metric]: + def get_metrics(self) -> dict[str, MetricWrapperBase]: """Return a dict mapping names to metrics.""" return self._metrics.copy() @@ -118,13 +114,17 @@ def generate_metrics(self) -> bytes: """Generate text with metrics values from the registry.""" return bytes(generate_latest(self.registry)) - def _register_metric(self, config: MetricConfig) -> Metric: + def _register_metric(self, config: MetricConfig) -> MetricWrapperBase: metric_type = METRIC_TYPES[config.type] options = { - metric_type.options[key]: value + key: value for key, value in config.config.items() if key in metric_type.options } return metric_type.cls( - config.name, config.description, registry=self.registry, **options + config.name, + config.description, + labelnames=config.labels, + registry=self.registry, + **options, ) diff --git a/prometheus_aioexporter/script.py b/prometheus_aioexporter/_script.py similarity index 94% rename from prometheus_aioexporter/script.py rename to prometheus_aioexporter/_script.py index f5a6e9a..7b71dac 100644 --- a/prometheus_aioexporter/script.py +++ b/prometheus_aioexporter/_script.py @@ -8,18 +8,16 @@ from typing import IO from aiohttp.web import Application -from prometheus_client import ( - Metric, - ProcessCollector, -) +from prometheus_client import ProcessCollector +from prometheus_client.metrics import MetricWrapperBase from toolrack.log import setup_logger from toolrack.script import Script -from .metric import ( +from ._metric import ( MetricConfig, MetricsRegistry, ) -from .web import PrometheusExporter +from ._web import PrometheusExporter class PrometheusExporterScript(Script): # type: ignore @@ -95,7 +93,7 @@ def configure(self, args: argparse.Namespace) -> None: def create_metrics( self, metric_configs: Iterable[MetricConfig] - ) -> dict[str, Metric]: + ) -> dict[str, MetricWrapperBase]: """Create and register metrics from a list of MetricConfigs.""" return self.registry.create_metrics(metric_configs) @@ -176,8 +174,10 @@ def _setup_logging(self, log_level: str) -> None: def _configure_registry(self, include_process_stats: bool = False) -> None: """Configure the MetricRegistry.""" if include_process_stats: + # XXX ignore type until + # https://github.com/prometheus/client_python/pull/970 is fixed self.registry.register_additional_collector( - ProcessCollector(registry=None) + ProcessCollector(registry=None) # type: ignore[arg-type] ) def _get_ssl_context( diff --git a/prometheus_aioexporter/web.py b/prometheus_aioexporter/_web.py similarity index 92% rename from prometheus_aioexporter/web.py rename to prometheus_aioexporter/_web.py index b67beac..90313fb 100644 --- a/prometheus_aioexporter/web.py +++ b/prometheus_aioexporter/_web.py @@ -3,7 +3,6 @@ from collections.abc import ( Awaitable, Callable, - Iterable, ) from ssl import SSLContext from textwrap import dedent @@ -14,15 +13,13 @@ Response, run_app, ) -from prometheus_client import ( - CONTENT_TYPE_LATEST, - Metric, -) +from prometheus_client import CONTENT_TYPE_LATEST +from prometheus_client.metrics import MetricWrapperBase -from .metric import MetricsRegistry +from ._metric import MetricsRegistry # Signature for update handler -UpdateHandler = Callable[[Iterable[Metric]], Awaitable[None]] +UpdateHandler = Callable[[dict[str, MetricWrapperBase]], Awaitable[None]] class PrometheusExporter: @@ -65,7 +62,7 @@ def set_metric_update_handler(self, handler: UpdateHandler) -> None: as argument, mapping metric names to metrics. The signature is the following: - async def update_handler(metrics): + async def update_handler(metrics: dict[str, MetricWrapperBase]) -> None: """ self._update_handler = handler diff --git a/prometheus_aioexporter/sample.py b/prometheus_aioexporter/sample.py index 64e6565..621db18 100644 --- a/prometheus_aioexporter/sample.py +++ b/prometheus_aioexporter/sample.py @@ -1,8 +1,13 @@ from argparse import Namespace import random +from typing import cast from aiohttp.web import Application -from prometheus_client import Metric +from prometheus_client import ( + Counter, + Gauge, +) +from prometheus_client.metrics import MetricWrapperBase from . import ( MetricConfig, @@ -20,10 +25,10 @@ def configure(self, args: Namespace) -> None: self.create_metrics( [ MetricConfig( - "a_gauge", "a gauge", "gauge", {"labels": ["foo", "bar"]} + "a_gauge", "a gauge", "gauge", labels=("foo", "bar") ), MetricConfig( - "a_counter", "a counter", "counter", {"labels": ["baz"]} + "a_counter", "a counter", "counter", labels=("baz",) ), ] ) @@ -31,12 +36,16 @@ def configure(self, args: Namespace) -> None: async def on_application_startup(self, application: Application) -> None: application["exporter"].set_metric_update_handler(self._update_handler) - async def _update_handler(self, metrics: dict[str, Metric]) -> None: - metrics["a_gauge"].labels( + async def _update_handler( + self, metrics: dict[str, MetricWrapperBase] + ) -> None: + gauge = cast(Gauge, metrics["a_gauge"]) + gauge.labels( foo=random.choice(["this-foo", "other-foo"]), bar=random.choice(["this-bar", "other-bar"]), ).set(random.uniform(0, 100)) - metrics["a_counter"].labels( + counter = cast(Counter, metrics["a_counter"]) + counter.labels( baz=random.choice(["this-baz", "other-baz"]), ).inc(random.choice(range(10))) diff --git a/tests/metric_test.py b/tests/metric_test.py index 7e40b8e..ec64ab9 100644 --- a/tests/metric_test.py +++ b/tests/metric_test.py @@ -1,6 +1,12 @@ +from typing import ( + Any, + Callable, +) + +from prometheus_client.metrics import MetricWrapperBase import pytest -from prometheus_aioexporter.metric import ( +from prometheus_aioexporter._metric import ( InvalidMetricType, MetricConfig, MetricsRegistry, @@ -11,19 +17,23 @@ class TestMetricConfig: def test_invalid_metric_type(self): """An invalid metric type raises an error.""" with pytest.raises(InvalidMetricType) as error: - MetricConfig("m1", "desc1", "unknown", {}) + MetricConfig("m1", "desc1", "unknown") assert str(error.value) == ( "Invalid type for m1: must be one of counter, enum, " "gauge, histogram, info, summary" ) + def test_labels_sorted(self) -> None: + config = MetricConfig("m", "desc", "counter", labels=["foo", "bar"]) + assert config.labels == ("bar", "foo") + class TestMetricsRegistry: def test_create_metrics(self): """Prometheus metrics are created from the specified config.""" configs = [ - MetricConfig("m1", "desc1", "counter", {}), - MetricConfig("m2", "desc2", "histogram", {}), + MetricConfig("m1", "desc1", "counter"), + MetricConfig("m2", "desc2", "histogram"), ] metrics = MetricsRegistry().create_metrics(configs) assert len(metrics) == 2 @@ -33,7 +43,9 @@ def test_create_metrics(self): def test_create_metrics_with_config(self): """Metric configs are applied.""" configs = [ - MetricConfig("m1", "desc1", "histogram", {"buckets": [10, 20]}) + MetricConfig( + "m1", "desc1", "histogram", config={"buckets": [10, 20]} + ) ] metrics = MetricsRegistry().create_metrics(configs) # The two specified bucket plus +Inf @@ -41,7 +53,9 @@ def test_create_metrics_with_config(self): def test_create_metrics_config_ignores_unknown(self): """Unknown metric configs are ignored and don't cause an error.""" - configs = [MetricConfig("m1", "desc1", "gauge", {"unknown": "value"})] + configs = [ + MetricConfig("m1", "desc1", "gauge", config={"unknown": "value"}) + ] metrics = MetricsRegistry().create_metrics(configs) assert len(metrics) == 1 @@ -50,8 +64,8 @@ def test_get_metrics(self): registry = MetricsRegistry() metrics = registry.create_metrics( [ - MetricConfig("metric1", "A test gauge", "gauge", {}), - MetricConfig("metric2", "A test histogram", "histogram", {}), + MetricConfig("metric1", "A test gauge", "gauge"), + MetricConfig("metric2", "A test histogram", "histogram"), ] ) assert registry.get_metrics() == metrics @@ -60,7 +74,10 @@ def test_get_metric(self): """get_metric returns a metric.""" configs = [ MetricConfig( - "m", "A test gauge", "gauge", {"labels": ["l1", "l2"]} + "m", + "A test gauge", + "gauge", + labels=["l1", "l2"], ) ] registry = MetricsRegistry() @@ -72,9 +89,7 @@ def test_get_metric(self): def test_get_metric_with_labels(self): """get_metric returns a metric configured with labels.""" configs = [ - MetricConfig( - "m", "A test gauge", "gauge", {"labels": ["l1", "l2"]} - ) + MetricConfig("m", "A test gauge", "gauge", labels=("l1", "l2")) ] registry = MetricsRegistry() registry.create_metrics(configs) @@ -82,7 +97,7 @@ def test_get_metric_with_labels(self): assert metric._labelvalues == ("v1", "v2") @pytest.mark.parametrize( - "typ,params,action,text", + "metric_type,config,action,text", [ ( "counter", @@ -117,12 +132,17 @@ def test_get_metric_with_labels(self): ), ], ) - def test_generate_metrics(self, typ, params, action, text): - """generate_metrics returns text with metrics values.""" + def test_generate_metrics( + self, + metric_type: str, + config: dict[str, Any], + action: Callable[[MetricWrapperBase], None], + text: str, + ) -> None: registry = MetricsRegistry() - name = "test_" + typ + name = "test_" + metric_type metrics = registry.create_metrics( - [MetricConfig(name, "A test metric", typ, params)] + [MetricConfig(name, "A test metric", metric_type, config=config)] ) action(metrics[name]) assert text in registry.generate_metrics().decode("utf-8") diff --git a/tests/script_test.py b/tests/script_test.py index 32bbe2f..3459378 100644 --- a/tests/script_test.py +++ b/tests/script_test.py @@ -5,8 +5,8 @@ import pytest -from prometheus_aioexporter.metric import MetricConfig -from prometheus_aioexporter.script import PrometheusExporterScript +from prometheus_aioexporter._metric import MetricConfig +from prometheus_aioexporter._script import PrometheusExporterScript class SampleScript(PrometheusExporterScript): @@ -62,9 +62,9 @@ def test_create_metrics(self, script): def test_setup_logging(self, mocker, script): """Logging is set up.""" mock_setup_logger = mocker.patch( - "prometheus_aioexporter.script.setup_logger" + "prometheus_aioexporter._script.setup_logger" ) - mocker.patch("prometheus_aioexporter.web.PrometheusExporter.run") + mocker.patch("prometheus_aioexporter._web.PrometheusExporter.run") script([]) logger_names = ( "aiohttp.access", @@ -139,7 +139,7 @@ def test_ssl_components( def test_include_process_stats(self, mocker, script): """The script can include process stats in metrics.""" - mocker.patch("prometheus_aioexporter.web.PrometheusExporter.run") + mocker.patch("prometheus_aioexporter._web.PrometheusExporter.run") script(["--process-stats"]) # process stats are present in the registry assert ( @@ -163,7 +163,7 @@ def test_script_run_exporter_ssl( tls_public_key_path, ): """The script runs the exporter application.""" - mock_run_app = mocker.patch("prometheus_aioexporter.web.run_app") + mock_run_app = mocker.patch("prometheus_aioexporter._web.run_app") script( [ "--ssl-public-key", @@ -179,7 +179,7 @@ def test_script_run_exporter_ssl( def test_script_run_exporter(self, mocker, script): """The script runs the exporter application.""" - mock_run_app = mocker.patch("prometheus_aioexporter.web.run_app") + mock_run_app = mocker.patch("prometheus_aioexporter._web.run_app") script([]) mock_run_app.assert_called_with( mock.ANY, diff --git a/tests/web_test.py b/tests/web_test.py index 554de1f..47e75ee 100644 --- a/tests/web_test.py +++ b/tests/web_test.py @@ -3,11 +3,11 @@ import pytest -from prometheus_aioexporter.metric import ( +from prometheus_aioexporter._metric import ( MetricConfig, MetricsRegistry, ) -from prometheus_aioexporter.web import PrometheusExporter +from prometheus_aioexporter._web import PrometheusExporter from tests.conftest import ssl_context @@ -60,7 +60,7 @@ def test_app_exporter_reference(self, exporter): @pytest.mark.parametrize("exporter", [ssl_context, None], indirect=True) def test_run(self, mocker, exporter): """The script starts the web application.""" - mock_run_app = mocker.patch("prometheus_aioexporter.web.run_app") + mock_run_app = mocker.patch("prometheus_aioexporter._web.run_app") exporter.run() mock_run_app.assert_called_with( mock.ANY, @@ -106,7 +106,7 @@ async def test_homepage_no_description(self, aiohttp_client, exporter): async def test_metrics(self, aiohttp_client, exporter, registry): """The /metrics page display Prometheus metrics.""" metrics = registry.create_metrics( - [MetricConfig("test_gauge", "A test gauge", "gauge", {})] + [MetricConfig("test_gauge", "A test gauge", "gauge")] ) metrics["test_gauge"].set(12.3) client = await aiohttp_client(exporter.app) @@ -132,7 +132,7 @@ async def test_metrics_different_path( ssl_context=ssl_context, ) metrics = registry.create_metrics( - [MetricConfig("test_gauge", "A test gauge", "gauge", {})] + [MetricConfig("test_gauge", "A test gauge", "gauge")] ) metrics["test_gauge"].set(12.3) client = await aiohttp_client(exporter.app) @@ -159,8 +159,8 @@ async def update_handler(metrics): exporter.set_metric_update_handler(update_handler) metrics = registry.create_metrics( [ - MetricConfig("metric1", "A test gauge", "gauge", {}), - MetricConfig("metric2", "A test histogram", "histogram", {}), + MetricConfig("metric1", "A test gauge", "gauge"), + MetricConfig("metric2", "A test histogram", "histogram"), ] ) client = await aiohttp_client(exporter.app) diff --git a/tox.ini b/tox.ini index a3cc009..3366565 100644 --- a/tox.ini +++ b/tox.ini @@ -11,9 +11,10 @@ commands = [testenv:check] deps = + .[testing] mypy commands = - mypy -p prometheus_aioexporter {posargs} + mypy prometheus_aioexporter {posargs} [testenv:coverage] deps =