diff --git a/karapace/client.py b/karapace/client.py index d699e6a73..dae79b244 100644 --- a/karapace/client.py +++ b/karapace/client.py @@ -91,6 +91,7 @@ async def get( headers: Optional[Headers] = None, auth: Optional[BasicAuth] = None, params: Optional[Mapping[str, str]] = None, + json_response: bool = True, ) -> Result: path = self.path_for(path) if not headers: @@ -105,8 +106,8 @@ async def get( params=params, ) as res: # required for forcing the response body conversion to json despite missing valid Accept headers - json_result = await res.json(content_type=None) - return Result(res.status, json_result, headers=res.headers) + result = await res.json(content_type=None) if json_response else await res.text() + return Result(res.status, result, headers=res.headers) async def delete( self, diff --git a/karapace/instrumentation/prometheus.py b/karapace/instrumentation/prometheus.py index d4b168d35..d50933d83 100644 --- a/karapace/instrumentation/prometheus.py +++ b/karapace/instrumentation/prometheus.py @@ -4,13 +4,14 @@ Copyright (c) 2024 Aiven Ltd See LICENSE for details """ +# mypy: disable-error-code="call-overload" from __future__ import annotations -from aiohttp.web import middleware, Request, RequestHandler, Response +from aiohttp.web import middleware, Request, Response from karapace.rapu import RestApp from prometheus_client import CollectorRegistry, Counter, Gauge, generate_latest, Histogram -from typing import ClassVar +from typing import Awaitable, Callable, Final import logging import time @@ -19,34 +20,34 @@ class PrometheusInstrumentation: - METRICS_ENDPOINT_PATH: ClassVar[str] = "/metrics" - START_TIME_REQUEST_KEY: ClassVar[str] = "start_time" + METRICS_ENDPOINT_PATH: Final[str] = "/metrics" + START_TIME_REQUEST_KEY: Final[str] = "start_time" - registry: ClassVar[CollectorRegistry] = CollectorRegistry() + registry: Final[CollectorRegistry] = CollectorRegistry() - karapace_http_requests_total: ClassVar[Counter] = Counter( + karapace_http_requests_total: Final[Counter] = Counter( registry=registry, name="karapace_http_requests_total", documentation="Total Request Count for HTTP/TCP Protocol", labelnames=("method", "path", "status"), ) - karapace_http_requests_latency_seconds: ClassVar[Histogram] = Histogram( + karapace_http_requests_duration_seconds: Final[Histogram] = Histogram( registry=registry, - name="karapace_http_requests_latency_seconds", + name="karapace_http_requests_duration_seconds", documentation="Request Duration for HTTP/TCP Protocol", labelnames=("method", "path"), ) - karapace_http_requests_in_progress: ClassVar[Gauge] = Gauge( + karapace_http_requests_in_progress: Final[Gauge] = Gauge( registry=registry, name="karapace_http_requests_in_progress", - documentation="Request Duration for HTTP/TCP Protocol", + documentation="In-progress requests for HTTP/TCP Protocol", labelnames=("method", "path"), ) @classmethod - def setup_metrics(cls: PrometheusInstrumentation, *, app: RestApp) -> None: + def setup_metrics(cls, *, app: RestApp) -> None: LOG.info("Setting up prometheus metrics") app.route( cls.METRICS_ENDPOINT_PATH, @@ -57,22 +58,26 @@ def setup_metrics(cls: PrometheusInstrumentation, *, app: RestApp) -> None: json_body=False, auth=None, ) - app.app.middlewares.insert(0, cls.http_request_metrics_middleware) + app.app.middlewares.insert(0, cls.http_request_metrics_middleware) # type: ignore[arg-type] + + # disable-error-code="call-overload" is used at the top of this file to allow mypy checks. + # the issue is in the type difference (Counter, Gauge, etc) of the arguments which we are + # passing to `__setitem__()`, but we need to keep these objects in the `app.app` dict. app.app[cls.karapace_http_requests_total] = cls.karapace_http_requests_total - app.app[cls.karapace_http_requests_latency_seconds] = cls.karapace_http_requests_latency_seconds + app.app[cls.karapace_http_requests_duration_seconds] = cls.karapace_http_requests_duration_seconds app.app[cls.karapace_http_requests_in_progress] = cls.karapace_http_requests_in_progress @classmethod - async def serve_metrics(cls: PrometheusInstrumentation) -> bytes: + async def serve_metrics(cls) -> bytes: return generate_latest(cls.registry) @classmethod @middleware async def http_request_metrics_middleware( - cls: PrometheusInstrumentation, + cls, request: Request, - handler: RequestHandler, - ) -> None: + handler: Callable[[Request], Awaitable[Response]], + ) -> Response: request[cls.START_TIME_REQUEST_KEY] = time.time() # Extract request labels @@ -86,7 +91,7 @@ async def http_request_metrics_middleware( response: Response = await handler(request) # Instrument request latency - request.app[cls.karapace_http_requests_latency_seconds].labels(method, path).observe( + request.app[cls.karapace_http_requests_duration_seconds].labels(method, path).observe( time.time() - request[cls.START_TIME_REQUEST_KEY] ) diff --git a/karapace/karapace_all.py b/karapace/karapace_all.py index 620928428..240da1008 100644 --- a/karapace/karapace_all.py +++ b/karapace/karapace_all.py @@ -63,9 +63,8 @@ def main() -> int: logging.log(logging.DEBUG, "Config %r", config_without_secrets) try: - # `close` will be called by the callback `close_by_app` set by `KarapaceBase` PrometheusInstrumentation.setup_metrics(app=app) - app.run() + app.run() # `close` will be called by the callback `close_by_app` set by `KarapaceBase` except Exception as ex: # pylint: disable-broad-except app.stats.unexpected_exception(ex=ex, where="karapace") raise diff --git a/tests/integration/instrumentation/__init__.py b/tests/integration/instrumentation/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/integration/instrumentation/test_prometheus.py b/tests/integration/instrumentation/test_prometheus.py new file mode 100644 index 000000000..cdac52ca8 --- /dev/null +++ b/tests/integration/instrumentation/test_prometheus.py @@ -0,0 +1,30 @@ +""" +karapace - prometheus instrumentation tests + +Copyright (c) 2024 Aiven Ltd +See LICENSE for details +""" + +from http import HTTPStatus +from karapace.client import Client, Result +from karapace.instrumentation.prometheus import PrometheusInstrumentation +from prometheus_client.parser import text_string_to_metric_families + + +async def test_metrics_endpoint(registry_async_client: Client) -> None: + result: Result = await registry_async_client.get( + PrometheusInstrumentation.METRICS_ENDPOINT_PATH, + json_response=False, + ) + assert result.status_code == HTTPStatus.OK.value + + +async def test_metrics_endpoint_parsed_response(registry_async_client: Client) -> None: + result: Result = await registry_async_client.get( + PrometheusInstrumentation.METRICS_ENDPOINT_PATH, + json_response=False, + ) + metrics = [family.name for family in text_string_to_metric_families(result.json_result)] + assert "karapace_http_requests" in metrics + assert "karapace_http_requests_duration_seconds" in metrics + assert "karapace_http_requests_in_progress" in metrics diff --git a/tests/unit/instrumentation/test_prometheus.py b/tests/unit/instrumentation/test_prometheus.py index 7fcbf4f19..9753d3853 100644 --- a/tests/unit/instrumentation/test_prometheus.py +++ b/tests/unit/instrumentation/test_prometheus.py @@ -27,15 +27,15 @@ def test_constants(self, prometheus: PrometheusInstrumentation) -> None: def test_metric_types(self, prometheus: PrometheusInstrumentation) -> None: assert isinstance(prometheus.karapace_http_requests_total, Counter) - assert isinstance(prometheus.karapace_http_requests_latency_seconds, Histogram) + assert isinstance(prometheus.karapace_http_requests_duration_seconds, Histogram) assert isinstance(prometheus.karapace_http_requests_in_progress, Gauge) def test_metric_values(self, prometheus: PrometheusInstrumentation) -> None: # `_total` suffix is stripped off the metric name for `Counters`, but needed for clarity. assert repr(prometheus.karapace_http_requests_total) == "prometheus_client.metrics.Counter(karapace_http_requests)" assert ( - repr(prometheus.karapace_http_requests_latency_seconds) - == "prometheus_client.metrics.Histogram(karapace_http_requests_latency_seconds)" + repr(prometheus.karapace_http_requests_duration_seconds) + == "prometheus_client.metrics.Histogram(karapace_http_requests_duration_seconds)" ) assert ( repr(prometheus.karapace_http_requests_in_progress) @@ -62,8 +62,8 @@ def test_setup_metrics(self, caplog: LogCaptureFixture, prometheus: PrometheusIn [ call(prometheus.karapace_http_requests_total, prometheus.karapace_http_requests_total), call( - prometheus.karapace_http_requests_latency_seconds, - prometheus.karapace_http_requests_latency_seconds, + prometheus.karapace_http_requests_duration_seconds, + prometheus.karapace_http_requests_duration_seconds, ), call(prometheus.karapace_http_requests_in_progress, prometheus.karapace_http_requests_in_progress), ] @@ -99,7 +99,7 @@ async def test_http_request_metrics_middleware( call().inc(), ] ) - request.app[prometheus.karapace_http_requests_latency_seconds].labels.assert_has_calls( + request.app[prometheus.karapace_http_requests_duration_seconds].labels.assert_has_calls( [ call("GET", "/path"), call().observe(request.__getitem__.return_value.__rsub__.return_value),