diff --git a/tests/conftest.py b/tests/conftest.py index f5b7541..861d168 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,7 +4,7 @@ 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 @@ -12,7 +12,7 @@ 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: diff --git a/tests/instruments/test_healthchecks_instrument.py b/tests/instruments/test_healthchecks_instrument.py index 157651d..e22b3ea 100644 --- a/tests/instruments/test_healthchecks_instrument.py +++ b/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: diff --git a/tests/test_fastapi_bootstrap.py b/tests/test_fastapi_bootstrap.py index a607fa0..d4dc1a9 100644 --- a/tests/test_fastapi_bootstrap.py +++ b/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,7 +628,9 @@ 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() @@ -638,33 +638,21 @@ async def items() -> str: @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: diff --git a/tests/test_fastmcp_bootstrap.py b/tests/test_fastmcp_bootstrap.py index ba1dc15..b7726be 100644 --- a/tests/test_fastmcp_bootstrap.py +++ b/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 diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index 46721b5..e62bc2c 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -46,10 +46,8 @@ HEALTH_PROBE_CASES, CustomInstrumentor, SentryTestTransport, - capture_probe_warnings, emulate_package_missing, emulate_package_missing_with_module_reload, - failed_probe_names, passing_probe, ) @@ -855,21 +853,19 @@ def __init__( bootstrapper.teardown() -@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) async def test_faststream_health_check_runs_probes( broker: RedisBroker, - probes: dict[str, HealthProbe], + probe: HealthProbe, expected_status: int, expected_health_status: bool, - expected_failed: list[str], ) -> None: - config = dataclasses.replace(build_faststream_config(broker=broker), health_checks_probes=probes) + config = dataclasses.replace(build_faststream_config(broker=broker), health_checks_probes={"cache": probe}) bootstrapper = FastStreamBootstrapper(bootstrap_config=config) application = bootstrapper.bootstrap() try: with ( patch.object(broker, "ping", new=AsyncMock(return_value=True)), - capture_probe_warnings() as probe_warnings, TestClient(app=application) as test_client, ): response = test_client.get(config.health_checks_path) @@ -882,7 +878,6 @@ async def test_faststream_health_check_runs_probes( "service_name": "microservice", "service_version": "2.0.0", } - assert failed_probe_names(probe_warnings) == expected_failed async def test_faststream_health_check_fails_on_broker_ping_despite_passing_probe(broker: RedisBroker) -> None: diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index f213282..1a29912 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -40,15 +40,13 @@ from lite_bootstrap.instruments.healthchecks_instrument import HealthProbe 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, ) @@ -679,8 +677,8 @@ async def items() -> list[int]: 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.""" +def _http_request_counts(config: LitestarConfig) -> tuple[int, float]: + """Return the OpenTelemetry and Prometheus request counts after one request.""" path: typing.Final = f"/items-{uuid.uuid4().hex}" async def get_items() -> list[int]: @@ -693,33 +691,18 @@ async def get_items() -> list[int]: try: with TestClient(app=application) as client: assert client.get(path).status_code == status_codes.HTTP_200_OK - return opentelemetry_recorded_http_server_metrics(reader), prometheus_recorded_path(path) + return opentelemetry_http_request_count(reader), prometheus_request_count( + "litestar_requests_total", "path", 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"), - ], -) +@pytest.mark.parametrize(("overrides", "expected"), HTTP_METRIC_CASES) def test_litestar_http_metrics_are_recorded_once( - litestar_config: LitestarConfig, overrides: dict[str, typing.Any], expected: tuple[bool, bool] + litestar_config: LitestarConfig, overrides: dict[str, typing.Any], expected: tuple[int, int] ) -> None: - assert _http_metric_sets_one_request_moves(dataclasses.replace(litestar_config, **overrides)) == expected + assert _http_request_counts(dataclasses.replace(litestar_config, **overrides)) == expected def test_litestar_prometheus_still_serves_metrics_while_opentelemetry_records_http_metrics( @@ -859,18 +842,17 @@ async def traced() -> None: ... assert [span.name for span in _server_spans(exporter)] == ["GET /traced"] -@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_litestar_health_check_runs_probes( litestar_config: LitestarConfig, - probes: dict[str, HealthProbe], + probe: HealthProbe, expected_status: int, expected_health_status: bool, - expected_failed: list[str], ) -> None: - config = dataclasses.replace(litestar_config, health_checks_probes=probes) + config = dataclasses.replace(litestar_config, health_checks_probes={"cache": probe}) bootstrapper = LitestarBootstrapper(bootstrap_config=config) application = bootstrapper.bootstrap() - with capture_probe_warnings() as probe_warnings, TestClient(app=application) as test_client: + with TestClient(app=application) as test_client: response = test_client.get(config.health_checks_path) assert response.status_code == expected_status @@ -879,7 +861,6 @@ def test_litestar_health_check_runs_probes( "service_name": "microservice", "service_version": "2.0.0", } - assert failed_probe_names(probe_warnings) == expected_failed def test_litestar_openapi_and_prometheus_fall_back_to_default_service_name() -> None: diff --git a/tests/test_opentelemetry_tracing_policy.py b/tests/test_opentelemetry_tracing_policy.py index 35f042a..cd0a2d1 100644 --- a/tests/test_opentelemetry_tracing_policy.py +++ b/tests/test_opentelemetry_tracing_policy.py @@ -13,7 +13,7 @@ from starlette import status from lite_bootstrap import FastAPIBootstrapper, FastAPIConfig, LitestarBootstrapper, LitestarConfig -from tests.conftest import in_memory_opentelemetry_metric_reader, opentelemetry_recorded_http_server_metrics +from tests.conftest import in_memory_opentelemetry_metric_reader, opentelemetry_http_request_count @pytest.mark.parametrize("framework", ["fastapi", "litestar"]) @@ -59,7 +59,7 @@ async def recording() -> dict[str, bool]: response = client.get("/recording") assert response.status_code == status.HTTP_200_OK assert response.json() == {"recording": external_setup == "before"} - assert opentelemetry_recorded_http_server_metrics(reader) + assert opentelemetry_http_request_count(reader) == 1 assert bool(exporter.get_finished_spans()) is (external_setup == "before") finally: bootstrapper.teardown()