Verify HTTP metric counts and simplify probe coverage

tests/conftest.py

@@ -4,15 +4,15 @@
 import warnings
 from collections.abc import Generator
 from importlib import reload
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import opentelemetry.metrics._internal
 import opentelemetry.trace
 import prometheus_client
 import pytest
 import sentry_sdk
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
-from opentelemetry.sdk.metrics.export import InMemoryMetricReader
+from opentelemetry.sdk.metrics.export import Histogram, InMemoryMetricReader
 from opentelemetry.util._once import Once
 from sentry_sdk import integrations as sentry_integrations
 from sentry_sdk import scope as sentry_scope
@@ -25,7 +25,7 @@
 
 from lite_bootstrap import import_checker
 from lite_bootstrap.exceptions import InstrumentDependencyMissingWarning
-from lite_bootstrap.instruments import healthchecks_instrument, opentelemetry_instrument
+from lite_bootstrap.instruments import opentelemetry_instrument
 
 
 def pytest_configure() -> None:
@@ -64,22 +64,11 @@ async def raising_probe() -> bool:
 
 
 HEALTH_PROBE_CASES: typing.Final = [
-    pytest.param({"db": passing_probe, "cache": passing_probe}, 200, True, [], id="all-pass"),
-    pytest.param({"db": passing_probe, "cache": failing_probe}, 503, False, ["cache"], id="one-returns-false"),
-    pytest.param({"db": passing_probe, "cache": raising_probe}, 503, False, ["cache"], id="one-raises"),
+    pytest.param(passing_probe, 200, True, id="passes"),
+    pytest.param(failing_probe, 503, False, id="fails"),
 ]
 
 
-@contextlib.contextmanager
-def capture_probe_warnings() -> Generator[MagicMock]:
-    with patch.object(healthchecks_instrument, "logger") as mock_logger:
-        yield mock_logger.warning
-
-
-def failed_probe_names(warning_mock: MagicMock) -> list[str]:
-    return [call.args[1] for call in warning_mock.call_args_list]
-
-
 class SentryTestTransport(sentry_sdk.Transport):
     def __init__(self, *args: typing.Any, **kwargs: typing.Any) -> None:  # noqa: ANN401
         super().__init__(*args, **kwargs)
@@ -247,20 +236,39 @@ def in_memory_opentelemetry_metric_reader() -> Generator[InMemoryMetricReader]:
         yield reader
 
 
-def opentelemetry_recorded_http_server_metrics(reader: InMemoryMetricReader) -> bool:
-    metrics_data: typing.Final = reader.get_metrics_data()
-    if metrics_data is None:
-        return False
-    return any(
-        metric.name.startswith("http.server.")
-        for resource_metrics in metrics_data.resource_metrics
-        for scope_metrics in resource_metrics.scope_metrics
-        for metric in scope_metrics.metrics
-    )
+HTTP_METRIC_CASES: typing.Final = [
+    pytest.param({"opentelemetry_metrics_endpoint": "localhost:4317"}, (1, 0), id="both-keeps-otel"),
+    pytest.param(
+        {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_http_metrics_enabled": True},
+        (1, 1),
+        id="both-restored",
+    ),
+    pytest.param({}, (0, 1), id="prometheus-alone"),
+    pytest.param(
+        {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_metrics_path": ""},
+        (1, 0),
+        id="otel-alone",
+    ),
+    pytest.param({"prometheus_http_metrics_enabled": False}, (0, 0), id="prometheus-http-off"),
+]
 
 
-def prometheus_recorded_path(path: str) -> bool:
-    return f'"{path}"' in prometheus_client.generate_latest().decode()
+def opentelemetry_http_request_count(reader: InMemoryMetricReader) -> int:
+    """Count requests without adding the legacy and stable series when both are emitted."""
+    metrics_data = reader.get_metrics_data()
+    if metrics_data is None:
+        return 0
+    counts: dict[str, int] = {}
+    for resource_metrics in metrics_data.resource_metrics:
+        for scope_metrics in resource_metrics.scope_metrics:
+            for metric in scope_metrics.metrics:
+                if metric.name in {"http.server.duration", "http.server.request.duration"} and isinstance(
+                    metric.data, Histogram
+                ):
+                    counts[metric.name] = counts.get(metric.name, 0) + sum(
+                        point.count for point in metric.data.data_points
+                    )
+    return max(counts.values(), default=0)
 
 
 def prometheus_request_count(metric_name: str, path_label: str, path: str) -> float:

tests/instruments/test_healthchecks_instrument.py

@@ -1,11 +1,11 @@
 import asyncio
-import time
 from unittest.mock import MagicMock
 
 import pytest
 
 from lite_bootstrap.instruments import healthchecks_instrument
-from lite_bootstrap.instruments.healthchecks_instrument import HealthChecksConfig, HealthChecksInstrument
+from lite_bootstrap.instruments.healthchecks_instrument import HealthChecksConfig, HealthChecksInstrument, HealthProbe
+from tests.conftest import ProbeError, failing_probe, passing_probe, raising_probe
 
 
 def test_healthchecks_instrument_configured_by_default() -> None:
@@ -52,18 +52,51 @@ def test_healthchecks_config_defaults() -> None:
 
 
 async def test_healthchecks_probes_run_concurrently() -> None:
-    interval = 0.2
+    first_started = asyncio.Event()
+    second_started = asyncio.Event()
 
-    async def slow_probe() -> bool:
-        await asyncio.sleep(interval)
+    async def first() -> bool:
+        first_started.set()
+        await second_started.wait()
+        return True
+
+    async def second() -> bool:
+        await first_started.wait()
+        second_started.set()
         return True
 
     instrument = HealthChecksInstrument(
-        bootstrap_config=HealthChecksConfig(health_checks_probes={"first": slow_probe, "second": slow_probe}),
+        bootstrap_config=HealthChecksConfig(health_checks_probes={"first": first, "second": second}),
     )
-    started = time.perf_counter()
-    assert await instrument.define_health_status()
-    assert time.perf_counter() - started < interval * 1.5
+    assert await asyncio.wait_for(instrument.define_health_status(), timeout=5)
+
+
+@pytest.mark.parametrize(
+    ("probe", "expected_message", "expected_exception"),
+    [
+        pytest.param(failing_probe, "Health probe cache failed", None, id="returns-false"),
+        pytest.param(raising_probe, "Health probe cache raised", ProbeError, id="raises"),
+    ],
+)
+async def test_failed_probe_makes_health_unhealthy_and_warns(
+    probe: HealthProbe,
+    expected_message: str,
+    expected_exception: type[Exception] | None,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    instrument = HealthChecksInstrument(
+        bootstrap_config=HealthChecksConfig(health_checks_probes={"db": passing_probe, "cache": probe}),
+    )
+
+    assert not await instrument.define_health_status()
+
+    [record] = [record for record in caplog.records if record.name == healthchecks_instrument.__name__]
+    assert record.getMessage() == expected_message
+    if expected_exception is None:
+        assert record.exc_info is None
+    else:
+        assert record.exc_info
+        assert isinstance(record.exc_info[1], expected_exception)
 
 
 async def test_healthchecks_status_is_healthy_without_probes() -> None:

tests/test_fastapi_bootstrap.py

@@ -29,15 +29,13 @@
 from lite_bootstrap.types import UNSET
 from tests.conftest import (
     HEALTH_PROBE_CASES,
+    HTTP_METRIC_CASES,
     CustomInstrumentor,
     SentryTestTransport,
-    capture_probe_warnings,
     emulate_package_missing,
     emulate_package_missing_with_module_reload,
-    failed_probe_names,
     in_memory_opentelemetry_metric_reader,
-    opentelemetry_recorded_http_server_metrics,
-    prometheus_recorded_path,
+    opentelemetry_http_request_count,
     prometheus_request_count,
     unregister_prometheus_request_metrics,
     warning_source_files,
@@ -614,8 +612,8 @@ async def items() -> list[int]:
     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."""
+def _http_request_counts(config: FastAPIConfig) -> tuple[int, float]:
+    """Return the OpenTelemetry and Prometheus request counts after one request."""
     path: typing.Final = f"/items-{uuid.uuid4().hex}"
     application: typing.Final = config.app
     unregister_prometheus_request_metrics()
@@ -630,41 +628,31 @@ async def items() -> str:
     try:
         with TestClient(application) as client:
             assert client.get(path).status_code == status.HTTP_200_OK
-        return opentelemetry_recorded_http_server_metrics(reader), prometheus_recorded_path(path)
+        return opentelemetry_http_request_count(reader), prometheus_request_count(
+            "http_requests_total", "handler", path
+        )
     finally:
         bootstrapper.teardown()
 
 
 @pytest.mark.parametrize(
     ("overrides", "expected"),
     [
-        pytest.param({"opentelemetry_metrics_endpoint": "localhost:4317"}, (True, False), id="both-keeps-otel"),
+        *HTTP_METRIC_CASES,
         pytest.param(
             {
                 "opentelemetry_metrics_endpoint": "http://localhost:4318/v1/metrics",
                 "opentelemetry_exporter_protocol": "http",
             },
-            (True, False),
+            (1, 0),
             id="both-keeps-otel-over-http",
         ),
-        pytest.param(
-            {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_http_metrics_enabled": True},
-            (True, True),
-            id="both-restored",
-        ),
-        pytest.param({}, (False, True), id="prometheus-alone"),
-        pytest.param(
-            {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_metrics_path": ""},
-            (True, False),
-            id="otel-alone",
-        ),
-        pytest.param({"prometheus_http_metrics_enabled": False}, (False, False), id="prometheus-http-off"),
     ],
 )
 def test_fastapi_http_metrics_are_recorded_once(
-    fastapi_config: FastAPIConfig, overrides: dict[str, typing.Any], expected: tuple[bool, bool]
+    fastapi_config: FastAPIConfig, overrides: dict[str, typing.Any], expected: tuple[int, int]
 ) -> None:
-    assert _http_metric_sets_one_request_moves(dataclasses.replace(fastapi_config, **overrides)) == expected
+    assert _http_request_counts(dataclasses.replace(fastapi_config, **overrides)) == expected
 
 
 def test_fastapi_prometheus_keeps_http_metrics_when_the_otlp_metric_exporter_is_missing(
@@ -675,7 +663,7 @@ def test_fastapi_prometheus_keeps_http_metrics_when_the_otlp_metric_exporter_is_
         patch.object(import_checker, "is_otlp_grpc_exporter_installed", new=False),
         pytest.warns(InstrumentDependencyMissingWarning, match="gRPC OTLP exporter is not installed"),
     ):
-        assert _http_metric_sets_one_request_moves(config) == (False, True)
+        assert _http_request_counts(config) == (0, 1)
 
 
 def test_fastapi_prometheus_still_serves_metrics_while_opentelemetry_records_http_metrics(
@@ -771,18 +759,17 @@ def test_fastapi_otel_honors_excluded_urls_from_environment(
     assert span_names == ["GET /custom-healthy"]
 
 
-@pytest.mark.parametrize(("probes", "expected_status", "expected_health_status", "expected_failed"), HEALTH_PROBE_CASES)
+@pytest.mark.parametrize(("probe", "expected_status", "expected_health_status"), HEALTH_PROBE_CASES)
 def test_fastapi_health_check_runs_probes(
     fastapi_config: FastAPIConfig,
-    probes: dict[str, HealthProbe],
+    probe: HealthProbe,
     expected_status: int,
     expected_health_status: bool,
-    expected_failed: list[str],
 ) -> None:
-    config = dataclasses.replace(fastapi_config, health_checks_probes=probes)
+    config = dataclasses.replace(fastapi_config, health_checks_probes={"cache": probe})
     bootstrapper = FastAPIBootstrapper(bootstrap_config=config)
     application = bootstrapper.bootstrap()
-    with capture_probe_warnings() as probe_warnings, TestClient(application) as test_client:
+    with TestClient(application) as test_client:
         response = test_client.get(config.health_checks_path)
 
     assert response.status_code == expected_status
@@ -791,7 +778,6 @@ def test_fastapi_health_check_runs_probes(
         "service_name": "microservice",
         "service_version": "2.0.0",
     }
-    assert failed_probe_names(probe_warnings) == expected_failed
 
 
 def test_built_app_title_and_version_fall_back_when_unset() -> None:

tests/test_fastmcp_bootstrap.py

@@ -40,13 +40,12 @@
 from lite_bootstrap.instruments.healthchecks_instrument import HealthProbe
 from tests.conftest import (
     HEALTH_PROBE_CASES,
-    capture_probe_warnings,
+    HTTP_METRIC_CASES,
     emulate_package_missing,
     emulate_package_missing_with_module_reload,
-    failed_probe_names,
     in_memory_opentelemetry_metric_reader,
-    opentelemetry_recorded_http_server_metrics,
-    prometheus_recorded_path,
+    opentelemetry_http_request_count,
+    prometheus_request_count,
     unregister_prometheus_request_metrics,
 )
 
@@ -573,8 +572,8 @@ def _unregister_request_metrics() -> None:
     unregister_prometheus_request_metrics()
 
 
-def _http_metric_sets_one_request_moves(**overrides: typing.Any) -> tuple[bool, bool]:  # noqa: ANN401
-    """Return whether one application request reached OpenTelemetry and Prometheus HTTP metrics, in that order."""
+def _http_request_counts(**overrides: typing.Any) -> tuple[int, float]:  # noqa: ANN401
+    """Return the OpenTelemetry and Prometheus request counts after one request."""
     config = _make_test_config(**overrides)
     path = f"/items-{uuid.uuid4().hex}"
 
@@ -588,31 +587,16 @@ async def items(_request: Request) -> Response:
     try:
         with TestClient(application.http_app()) as client:
             assert client.get(path).status_code == status.HTTP_200_OK
-        return opentelemetry_recorded_http_server_metrics(reader), prometheus_recorded_path(path)
+        return opentelemetry_http_request_count(reader), prometheus_request_count(
+            "http_requests_total", "handler", path
+        )
     finally:
         bootstrapper.teardown()
 
 
-@pytest.mark.parametrize(
-    ("overrides", "expected"),
-    [
-        pytest.param({"opentelemetry_metrics_endpoint": "localhost:4317"}, (True, False), id="both-keeps-otel"),
-        pytest.param(
-            {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_http_metrics_enabled": True},
-            (True, True),
-            id="both-restored",
-        ),
-        pytest.param({}, (False, True), id="prometheus-alone"),
-        pytest.param(
-            {"opentelemetry_metrics_endpoint": "localhost:4317", "prometheus_metrics_path": ""},
-            (True, False),
-            id="otel-alone",
-        ),
-        pytest.param({"prometheus_http_metrics_enabled": False}, (False, False), id="prometheus-http-off"),
-    ],
-)
-def test_fastmcp_http_metrics_are_recorded_once(overrides: dict[str, typing.Any], expected: tuple[bool, bool]) -> None:
-    assert _http_metric_sets_one_request_moves(**overrides) == expected
+@pytest.mark.parametrize(("overrides", "expected"), HTTP_METRIC_CASES)
+def test_fastmcp_http_metrics_are_recorded_once(overrides: dict[str, typing.Any], expected: tuple[int, int]) -> None:
+    assert _http_request_counts(**overrides) == expected
 
 
 def test_fastmcp_prometheus_keeps_tool_metrics_and_metrics_route_while_opentelemetry_records_http_metrics() -> None:
@@ -876,18 +860,17 @@ def test_fastmcp_otel_honors_excluded_urls_from_environment(monkeypatch: pytest.
     ]
 
 
-@pytest.mark.parametrize(("probes", "expected_status", "expected_health_status", "expected_failed"), HEALTH_PROBE_CASES)
+@pytest.mark.parametrize(("probe", "expected_status", "expected_health_status"), HEALTH_PROBE_CASES)
 def test_fastmcp_health_check_runs_probes(
-    probes: dict[str, HealthProbe],
+    probe: HealthProbe,
     expected_status: int,
     expected_health_status: bool,
-    expected_failed: list[str],
 ) -> None:
-    config = _make_test_config(health_checks_probes=probes)
+    config = _make_test_config(health_checks_probes={"cache": probe})
     bootstrapper = FastMcpBootstrapper(bootstrap_config=config)
     application = bootstrapper.bootstrap()
     try:
-        with capture_probe_warnings() as probe_warnings, TestClient(application.http_app()) as test_client:
+        with TestClient(application.http_app()) as test_client:
             response = test_client.get(config.health_checks_path)
     finally:
         bootstrapper.teardown()
@@ -898,4 +881,3 @@ def test_fastmcp_health_check_runs_probes(
         "service_name": "test-mcp",
         "service_version": "1.2.3",
     }
-    assert failed_probe_names(probe_warnings) == expected_failed