diff --git a/docs/migration/to-2.x.md b/docs/migration/to-2.x.md index ac2538b..b350339 100644 --- a/docs/migration/to-2.x.md +++ b/docs/migration/to-2.x.md @@ -12,3 +12,11 @@ methods your application supports, such as `["GET", "POST"]`. `cors_allowed_headers` defaults to `[]`. Both frameworks allow their built-in CORS headers, including `Content-Type`. Add headers such as `Authorization` explicitly if your application needs them. + +## Prometheus HTTP metrics on FastAPI, FastMCP, and Litestar + +Health checks and metrics scrapes are excluded from Prometheus HTTP request +metrics. Existing user-defined exclusions still apply. Dashboards and alerts +that previously included these requests may show lower request totals and +different latency percentiles. Review thresholds that were based on the old +traffic mix. diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index 87ebe70..e7626c7 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -250,7 +250,12 @@ def dependencies_installed() -> bool: def bootstrap(self) -> None: config = self.bootstrap_config application = config.app - instrumentator = Instrumentator(**config.prometheus_instrumentator_additional_params) + instrumentator_params = dict(config.prometheus_instrumentator_additional_params) + instrumentator_params["excluded_handlers"] = [ + *instrumentator_params.get("excluded_handlers", []), + *self.infrastructure_excluded_patterns(), + ] + instrumentator = Instrumentator(**instrumentator_params) if config.is_prometheus_http_metrics_on( opentelemetry_records_http_metrics=FastAPIOpenTelemetryInstrument.is_recording_metrics(config) ): diff --git a/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py b/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py index 44f0b50..494ffc3 100644 --- a/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py @@ -2,7 +2,6 @@ import dataclasses import functools import inspect -import re import time import typing from collections.abc import AsyncGenerator, Mapping @@ -280,10 +279,11 @@ def dependencies_installed() -> bool: def _instrument_http_app(self, http_application: "StarletteWithLifespan") -> "StarletteWithLifespan": config: typing.Final = self.bootstrap_config - instrumentator_params: typing.Final[dict[str, typing.Any]] = { - "excluded_handlers": [f"^{re.escape(config.prometheus_metrics_path)}$"], - **config.prometheus_instrumentator_additional_params, - } + instrumentator_params = dict(config.prometheus_instrumentator_additional_params) + instrumentator_params["excluded_handlers"] = [ + *instrumentator_params.get("excluded_handlers", []), + *self.infrastructure_excluded_patterns(), + ] # A second http_app() would register the default metrics again, which the instrumentator # treats as a duplicate and silently skips, so every application shares the first set. if self._default_metrics is None: diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index 84f019c..4a2acea 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -415,6 +415,10 @@ class LitestarPrometheusController(PrometheusController): "group_path": config.prometheus_group_path, **config.prometheus_additional_params, } + user_exclusions = prometheus_params.get("exclude") + if isinstance(user_exclusions, str): + user_exclusions = [user_exclusions] + prometheus_params["exclude"] = [*(user_exclusions or []), *self.infrastructure_excluded_patterns()] litestar_prometheus_config = PrometheusConfig(app_name=config.service_name_or_default, **prometheus_params) config.application_config.route_handlers.append(LitestarPrometheusController) diff --git a/lite_bootstrap/instruments/prometheus_instrument.py b/lite_bootstrap/instruments/prometheus_instrument.py index ef7b08c..491814b 100644 --- a/lite_bootstrap/instruments/prometheus_instrument.py +++ b/lite_bootstrap/instruments/prometheus_instrument.py @@ -1,6 +1,7 @@ """Prometheus config and readiness check; framework-specific bootstrap lives in the bootstrapper subclasses.""" import dataclasses +import re import typing from lite_bootstrap.exceptions import ConfigurationError @@ -42,3 +43,8 @@ class PrometheusInstrument(BaseInstrument[PrometheusConfigT]): @classmethod def is_configured(cls, bootstrap_config: "PrometheusConfig") -> bool: return bool(bootstrap_config.prometheus_metrics_path) + + def infrastructure_excluded_patterns(self) -> list[str]: + config = self.bootstrap_config + paths = (getattr(config, "health_checks_path", ""), config.prometheus_metrics_path) + return [rf"^{re.escape(path.rstrip('/'))}/?$" for path in paths if path] diff --git a/tests/conftest.py b/tests/conftest.py index 19e9ce9..605ad42 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -263,6 +263,15 @@ def prometheus_recorded_path(path: str) -> bool: return f'"{path}"' in prometheus_client.generate_latest().decode() +def prometheus_request_count(metric_name: str, path_label: str, path: str) -> float: + return sum( + sample.value + for metric in prometheus_client.REGISTRY.collect() + for sample in metric.samples + if sample.name == metric_name and sample.labels.get(path_label) == path + ) + + def unregister_prometheus_request_metrics() -> None: """Unregister the instrumentator's request metrics: a second registration is skipped, not reused.""" registered_collectors: typing.Final = dict(prometheus_client.REGISTRY._collector_to_names) # noqa: SLF001 diff --git a/tests/test_fastapi_bootstrap.py b/tests/test_fastapi_bootstrap.py index 3cb11af..1ad185a 100644 --- a/tests/test_fastapi_bootstrap.py +++ b/tests/test_fastapi_bootstrap.py @@ -37,6 +37,7 @@ in_memory_opentelemetry_metric_reader, opentelemetry_recorded_http_server_metrics, prometheus_recorded_path, + prometheus_request_count, unregister_prometheus_request_metrics, warning_source_files, ) @@ -577,6 +578,32 @@ def test_fastapi_otel_hands_the_instrumentor_the_meter_provider(fastapi_config: assert isinstance(get_meter_provider(), SDKMeterProvider) +def test_fastapi_prometheus_excludes_infrastructure_and_user_paths() -> None: + unregister_prometheus_request_metrics() + suffix = uuid.uuid4().hex + normal_path = f"/health-{suffix}/report" + excluded_path = f"/excluded-{suffix}" + config = FastAPIConfig( + health_checks_path=f"/health-{suffix}", + prometheus_metrics_path=f"/metrics-{suffix}", + prometheus_instrumentator_additional_params={"excluded_handlers": [f"^{excluded_path}$"]}, + ) + + async def items() -> list[int]: + return [1] + + for path in (normal_path, excluded_path): + config.app.add_api_route(path, items, methods=["GET"]) + application = FastAPIBootstrapper(bootstrap_config=config).bootstrap() + with TestClient(application) as client: + for path in (config.health_checks_path, config.prometheus_metrics_path, excluded_path, normal_path): + assert client.get(path).is_success + + for path in (config.health_checks_path, config.prometheus_metrics_path, excluded_path): + assert prometheus_request_count("http_requests_total", "handler", path) == 0 + assert prometheus_request_count("http_requests_total", "handler", normal_path) == 1 + + def _http_metric_sets_one_request_moves(config: FastAPIConfig) -> tuple[bool, bool]: """Return whether one request reached OpenTelemetry's HTTP metrics and Prometheus's, in that order.""" path: typing.Final = f"/items-{uuid.uuid4().hex}" diff --git a/tests/test_fastmcp_bootstrap.py b/tests/test_fastmcp_bootstrap.py index 6e60e5b..f925b75 100644 --- a/tests/test_fastmcp_bootstrap.py +++ b/tests/test_fastmcp_bootstrap.py @@ -19,6 +19,8 @@ from opentelemetry.trace import SpanKind, get_tracer_provider from starlette import status from starlette.applications import Starlette +from starlette.requests import Request +from starlette.responses import Response from starlette.testclient import TestClient from lite_bootstrap import BootstrapperNotReadyError, FastMcpBootstrapper, FastMcpConfig @@ -574,15 +576,21 @@ def _unregister_request_metrics() -> None: def _http_metric_sets_one_request_moves(**overrides: typing.Any) -> tuple[bool, bool]: # noqa: ANN401 - """Return whether one health-check request reached OpenTelemetry's HTTP metrics and Prometheus's, in that order.""" - config: typing.Final = _make_test_config(**{"opentelemetry_generate_health_check_spans": True, **overrides}) + """Return whether one application request reached OpenTelemetry and Prometheus HTTP metrics, in that order.""" + config = _make_test_config(**overrides) + path = f"/items-{uuid.uuid4().hex}" + + @config.application.custom_route(path, methods=["GET"]) + async def items(_request: Request) -> Response: + return Response("ok") + with in_memory_opentelemetry_metric_reader() as reader: bootstrapper = FastMcpBootstrapper(bootstrap_config=config) application = bootstrapper.bootstrap() try: with TestClient(application.http_app()) as client: - assert client.get(config.health_checks_path).status_code == status.HTTP_200_OK - return opentelemetry_recorded_http_server_metrics(reader), prometheus_recorded_path(config.health_checks_path) + assert client.get(path).status_code == status.HTTP_200_OK + return opentelemetry_recorded_http_server_metrics(reader), prometheus_recorded_path(path) finally: bootstrapper.teardown() @@ -630,56 +638,56 @@ def _http_requests_total(handler: str, method: str = "GET", status_group: str = return _sample_value("http_requests_total", {"handler": handler, "method": method, "status": status_group}) -def test_fastmcp_prometheus_counts_http_requests_by_route_template() -> None: - health_checks_path = f"/health-{uuid.uuid4().hex}/" - config = _make_test_config(health_checks_path=health_checks_path) +def test_fastmcp_prometheus_excludes_infrastructure_and_user_paths() -> None: + health_path = f"/health-{uuid.uuid4().hex}/" + normal_path = f"{health_path}report" + excluded_path = "/excluded" + config = _make_test_config( + health_checks_path=health_path, + prometheus_instrumentator_additional_params={"excluded_handlers": [f"^{excluded_path}$"]}, + ) + + @config.application.custom_route(normal_path, methods=["GET"]) + @config.application.custom_route(excluded_path, methods=["GET"]) + async def items(_request: Request) -> Response: + return Response("ok") + bootstrapper = FastMcpBootstrapper(bootstrap_config=config) application = bootstrapper.bootstrap() try: with TestClient(application.http_app()) as client: - client.get(health_checks_path) - client.get(health_checks_path) + for path in (health_path, config.prometheus_metrics_path, excluded_path, normal_path): + assert client.get(path).is_success client.get("/missing") - client.get(config.prometheus_metrics_path) finally: bootstrapper.teardown() - expected_health_requests = 2 - assert _http_requests_total(health_checks_path) == expected_health_requests - assert _http_requests_total("none", status_group="4xx") == 1 + assert _http_requests_total(health_path) == 0 assert _http_requests_total(config.prometheus_metrics_path) == 0 + assert _http_requests_total(excluded_path) == 0 + assert _http_requests_total(normal_path) == 1 + assert _http_requests_total("none", status_group="4xx") == 1 def test_fastmcp_prometheus_counts_requests_of_every_http_app() -> None: - health_checks_path = f"/health-{uuid.uuid4().hex}/" - bootstrapper = FastMcpBootstrapper(bootstrap_config=_make_test_config(health_checks_path=health_checks_path)) - application = bootstrapper.bootstrap() - try: - for http_application in (application.http_app(), application.http_app(path="/other")): - with TestClient(http_application) as client: - client.get(health_checks_path) - finally: - bootstrapper.teardown() - - expected_requests = 2 - assert _http_requests_total(health_checks_path) == expected_requests + path = f"/items-{uuid.uuid4().hex}" + config = _make_test_config() + @config.application.custom_route(path, methods=["GET"]) + async def items(_request: Request) -> Response: + return Response("ok") -def test_fastmcp_prometheus_instrumentator_params_are_passed() -> None: - health_checks_path = f"/health-{uuid.uuid4().hex}/" - config = _make_test_config( - health_checks_path=health_checks_path, - prometheus_instrumentator_additional_params={"excluded_handlers": [health_checks_path]}, - ) bootstrapper = FastMcpBootstrapper(bootstrap_config=config) application = bootstrapper.bootstrap() try: - with TestClient(application.http_app()) as client: - client.get(health_checks_path) + for http_application in (application.http_app(), application.http_app(path="/other")): + with TestClient(http_application) as client: + client.get(path) finally: bootstrapper.teardown() - assert _http_requests_total(health_checks_path) == 0 + expected_requests = 2 + assert _http_requests_total(path) == expected_requests def test_fastmcp_prometheus_teardown_restores_http_app() -> None: diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index 4058755..6343b6e 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -49,6 +49,7 @@ in_memory_opentelemetry_metric_reader, opentelemetry_recorded_http_server_metrics, prometheus_recorded_path, + prometheus_request_count, ) @@ -660,6 +661,32 @@ def _server_spans(exporter: InMemorySpanExporter) -> list[ReadableSpan]: return [span for span in exporter.get_finished_spans() if span.kind == SpanKind.SERVER] +def test_litestar_prometheus_excludes_infrastructure_and_user_paths() -> None: + suffix = uuid.uuid4().hex + normal_path = f"/health-{suffix}/report" + excluded_path = f"/excluded-{suffix}" + config = LitestarConfig( + health_checks_path=f"/health-{suffix}", + prometheus_metrics_path=f"/metrics-{suffix}", + prometheus_additional_params={"exclude": f"^{excluded_path}$"}, + ) + + async def items() -> list[int]: + return [1] + + config.application_config.route_handlers.extend( + [litestar.get(normal_path)(items), litestar.get(excluded_path)(items)] + ) + application = LitestarBootstrapper(bootstrap_config=config).bootstrap() + with TestClient(application) as client: + for path in (config.health_checks_path, config.prometheus_metrics_path, excluded_path, normal_path): + assert client.get(path).is_success + + for path in (config.health_checks_path, config.prometheus_metrics_path, excluded_path): + assert prometheus_request_count("litestar_requests_total", "path", path) == 0 + assert prometheus_request_count("litestar_requests_total", "path", normal_path) == 1 + + def _http_metric_sets_one_request_moves(config: LitestarConfig) -> tuple[bool, bool]: """Return whether one request reached OpenTelemetry's HTTP metrics and Prometheus's, in that order.""" path: typing.Final = f"/items-{uuid.uuid4().hex}"