diff --git a/README.md b/README.md index 1ddeae5..e248639 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,16 @@ Those instruments can be bootstrapped for: - [FastAPI](https://lite-bootstrap.readthedocs.io/integrations/fastapi) - [FastMCP](https://lite-bootstrap.readthedocs.io/integrations/fastmcp) - [services and scripts without frameworks](https://lite-bootstrap.readthedocs.io/integrations/free) + +## Lifecycle constraints + +A few constraints that aren't obvious from the API: + +- **One bootstrapper per application instance.** Constructing two `FastAPIBootstrapper`s around the same `fastapi.FastAPI` (or two `FastMcpBootstrapper`s around the same `FastMCP`) stacks teardown hooks and re-wraps the lifespan. The library warns and skips the second attachment, but the second bootstrapper's `teardown()` won't fire on ASGI shutdown. +- **One `OpenTelemetryInstrument` per process.** `bootstrap()` calls `opentelemetry.trace.set_tracer_provider(...)`, which the OTel SDK enforces as set-once — subsequent calls log a warning and have no effect. `teardown()` flushes spans and closes exporters but can't reset the process-global pointer. +- **`teardown()` is idempotent.** `BaseBootstrapper.teardown()` short-circuits if not bootstrapped; per-instrument teardown methods are safe to call multiple times. +- **Partial teardown failures are aggregated.** If an instrument's teardown raises, the bootstrapper continues with the rest of the instruments and raises `TeardownError` at the end with all collected failures. + --- Usage examples: diff --git a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py index 6ac2b89..59287f0 100644 --- a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py @@ -126,13 +126,15 @@ def bootstrap(self) -> None: self._broker_logger_replaced = True def teardown(self) -> None: - if self._broker_logger_replaced: - broker = self.bootstrap_config.application.broker - if broker is not None: - broker.config.logger.params_storage = self._prior_broker_params_storage - self._broker_logger_replaced = False - self._prior_broker_params_storage = None - super().teardown() + try: + if self._broker_logger_replaced: + broker = self.bootstrap_config.application.broker + if broker is not None: + broker.config.logger.params_storage = self._prior_broker_params_storage + self._broker_logger_replaced = False + self._prior_broker_params_storage = None + finally: + super().teardown() @dataclasses.dataclass(kw_only=True) diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index e2a6781..a82e025 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -6,6 +6,7 @@ import warnings from lite_bootstrap import import_checker +from lite_bootstrap.exceptions import TeardownError from lite_bootstrap.instruments.base import BaseConfig, BaseInstrument @@ -193,19 +194,27 @@ def bootstrap(self) -> None: one_instrumentor.instrument(tracer_provider=tracer_provider) def teardown(self) -> None: + errors: list[tuple[str, BaseException]] = [] for one_instrumentor in self.bootstrap_config.opentelemetry_instrumentors: - if isinstance(one_instrumentor, InstrumentorWithParams): - one_instrumentor.instrumentor.uninstrument(**one_instrumentor.additional_params) - else: - one_instrumentor.uninstrument() + try: + if isinstance(one_instrumentor, InstrumentorWithParams): + one_instrumentor.instrumentor.uninstrument(**one_instrumentor.additional_params) + else: + one_instrumentor.uninstrument() + except Exception as e: # noqa: BLE001, PERF203 + errors.append((type(one_instrumentor).__name__, e)) for logger_name, prior in self._prior_logger_disabled.items(): logging.getLogger(logger_name).disabled = prior self._prior_logger_disabled.clear() if self._tracer_provider is not None: try: self._tracer_provider.shutdown() + except Exception as e: # noqa: BLE001 + errors.append(("TracerProvider", e)) finally: self._tracer_provider = None + if errors: + raise TeardownError(errors) from errors[0][1] # Backward-compatible alias preserved for users importing the old (lowercase t) spelling. diff --git a/lite_bootstrap/instruments/sentry_instrument.py b/lite_bootstrap/instruments/sentry_instrument.py index 35e77b8..2c82555 100644 --- a/lite_bootstrap/instruments/sentry_instrument.py +++ b/lite_bootstrap/instruments/sentry_instrument.py @@ -130,5 +130,7 @@ def teardown(self) -> None: cleans up after a bootstrap so the same process can be torn down and re-tested without leaking the previous DSN/transport into subsequent code. """ - sentry_sdk.flush(timeout=2) - sentry_sdk.init() + try: + sentry_sdk.flush(timeout=2) + finally: + sentry_sdk.init() diff --git a/tests/instruments/test_opentelemetry_instrument.py b/tests/instruments/test_opentelemetry_instrument.py index 1be3754..66433e2 100644 --- a/tests/instruments/test_opentelemetry_instrument.py +++ b/tests/instruments/test_opentelemetry_instrument.py @@ -1,9 +1,12 @@ import logging +import typing import warnings from unittest.mock import patch import pytest +from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from lite_bootstrap.exceptions import TeardownError from lite_bootstrap.instruments.opentelemetry_instrument import ( InstrumentorWithParams, OpenTelemetryConfig, @@ -146,3 +149,37 @@ def test_opentelemetry_config_no_warning_for_unix_socket_endpoint() -> None: opentelemetry_endpoint="unix:///var/run/otel.sock", opentelemetry_insecure=True, ) + + +def test_opentelemetry_teardown_aggregates_instrumentor_and_shutdown_errors() -> None: + class BoomInstrumentor(BaseInstrumentor): + def instrumentation_dependencies(self) -> typing.Collection[str]: + return [] + + def _instrument(self, **_kwargs: object) -> None: ... + + def _uninstrument(self, **_kwargs: object) -> None: + msg = "instrumentor boom" + raise RuntimeError(msg) + + instrument = OpenTelemetryInstrument( + bootstrap_config=OpenTelemetryConfig( + opentelemetry_instrumentors=[BoomInstrumentor()], + opentelemetry_log_traces=True, + ), + ) + instrument.bootstrap() + tracer_provider = instrument._tracer_provider # noqa: SLF001 + assert tracer_provider is not None + + with ( + patch.object(tracer_provider, "shutdown", side_effect=RuntimeError("shutdown boom")), + pytest.raises(TeardownError) as excinfo, + ): + instrument.teardown() + + error_msgs = [str(err) for _, err in excinfo.value.errors] + assert any("instrumentor boom" in m for m in error_msgs), error_msgs + assert any("shutdown boom" in m for m in error_msgs), error_msgs + assert instrument._tracer_provider is None # noqa: SLF001 + assert instrument._prior_logger_disabled == {} # noqa: SLF001 diff --git a/tests/instruments/test_sentry_instrument.py b/tests/instruments/test_sentry_instrument.py index 036796e..8c25646 100644 --- a/tests/instruments/test_sentry_instrument.py +++ b/tests/instruments/test_sentry_instrument.py @@ -1,6 +1,7 @@ import copy import logging import typing +from unittest.mock import patch import pytest import sentry_sdk @@ -134,3 +135,17 @@ def test_skip(self, event: "sentry_types.Event") -> None: ) def test_modify(self, event_before: "sentry_types.Event", event_after: "sentry_types.Event") -> None: assert enrich_sentry_event_from_structlog_log(event_before, {}) == event_after + + +def test_sentry_teardown_runs_init_when_flush_raises(minimal_sentry_config: SentryConfig) -> None: + instrument = SentryInstrument(bootstrap_config=minimal_sentry_config) + instrument.bootstrap() + + with ( + patch("sentry_sdk.flush", side_effect=RuntimeError("flush boom")), + pytest.raises(RuntimeError, match="flush boom"), + ): + instrument.teardown() + + # init() still ran (in the finally), so SDK is now disabled. + assert sentry_sdk.get_client().dsn is None diff --git a/tests/test_config.py b/tests/test_config.py index 27f5d56..f680f19 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,9 @@ import dataclasses +import warnings -from lite_bootstrap import FastAPIConfig +import pytest + +from lite_bootstrap import FastAPIConfig, FastStreamConfig, FreeConfig, LitestarConfig from lite_bootstrap.instruments.base import BaseConfig from tests.conftest import CustomInstrumentor @@ -92,3 +95,15 @@ def test_from_dict_drops_unknown_keys_silently() -> None: def test_from_dict_explicit_none_overrides_default() -> None: config = BaseConfig.from_dict({"service_name": None}) assert config.service_name is None + + +@pytest.mark.parametrize("config_cls", [FreeConfig, LitestarConfig, FastStreamConfig, FastAPIConfig]) +def test_otel_insecure_warning_fires_through_config_cascade(config_cls: type) -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + config_cls( + opentelemetry_endpoint="http://collector.example.com:4317", + opentelemetry_insecure=True, + ) + matching = [w for w in caught if "unencrypted" in str(w.message)] + assert matching, f"{config_cls.__name__}: {[str(w.message) for w in caught]}" diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index 7af2eba..ba49464 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -17,7 +17,10 @@ from starlette.testclient import TestClient from lite_bootstrap import FastStreamBootstrapper, FastStreamConfig -from lite_bootstrap.bootstrappers.faststream_bootstrapper import FastStreamOpenTelemetryInstrument +from lite_bootstrap.bootstrappers.faststream_bootstrapper import ( + FastStreamLoggingInstrument, + FastStreamOpenTelemetryInstrument, +) from tests.conftest import ( CustomInstrumentor, SentryTestTransport, @@ -236,3 +239,31 @@ async def test_faststream_prometheus_uses_injected_registry(broker: RedisBroker) assert counter_name.encode() in response.content finally: bootstrapper.teardown() + + +def test_faststream_logging_teardown_runs_super_when_broker_write_raises(broker: RedisBroker) -> None: + bootstrap_config = build_faststream_config(broker=broker) + instrument = FastStreamLoggingInstrument(bootstrap_config=bootstrap_config) + instrument.bootstrap() + + # Make broker.config.logger.params_storage raise on write by promoting the instance to a + # subclass that shadows params_storage with a property whose setter always raises. + logger_state = broker.config.logger + + class _BrokenLoggerState(type(logger_state)): # ty: ignore[unsupported-base] + @property # type: ignore[override] + def params_storage(self) -> object: # pragma: no cover + return None + + @params_storage.setter + def params_storage(self, _value: object) -> None: + msg = "broker write boom" + raise RuntimeError(msg) + + logger_state.__class__ = _BrokenLoggerState + + # super().teardown() still runs (LoggingInstrument's structlog reset); broker write raises last. + with pytest.raises(RuntimeError, match="broker write boom"): + instrument.teardown() + # If super().teardown() ran, structlog defaults were reset — no exception below. + structlog.get_logger("verify-reset")