From 474d2e2ca786f858b3bff8dc38bc3ad19b71a4cc Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 09:52:08 +0300 Subject: [PATCH 1/8] refactor: replace InstrumentNotReadyWarning with is_configured classmethod + summary log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #86 introduced InstrumentNotReadyWarning, escalating the not-ready skip path from a (silently dropped) logger.info call to a real warnings.warn. The escalation surfaced expected-path events as warnings: every service using a subset of available instruments emits N warnings on bootstrap, and lite-bootstrap's own tests can't suppress without breaking the warning assertions. Replace is_ready (instance method, ran after instantiation) with is_configured (classmethod taking config, runs before instantiation). The bootstrapper now: 1. is_configured(config) False → silent skip; append to new bootstrapper.skipped_instruments: list[tuple[type, str]]. 2. check_dependencies() False → InstrumentDependencyMissingWarning (kept; this is the genuine "configured but dep missing" deployment surprise). 3. Otherwise instantiate and register. After the loop, emit one INFO-level summary log listing configured + skipped instruments. Default Python logging suppresses INFO, so silent by default with an opt-in path via logging.basicConfig. PR #88's constraint (check_dependencies before instantiation, because some instruments have default_factory that NameErrors when optional extras aren't installed) is preserved: is_configured is a classmethod, so it runs without instantiation. Production-code subtlety: bootstrappers/base.py uses a fresh logger per call via _get_logger() instead of a module-level cached one. The module-level structlog.getLogger(__name__) interacts badly with LoggingInstrument's cache_logger_on_first_use=True — the BoundLogger gets cached with whatever processors were active at first use, and structlog.testing.capture_logs() can't override the cache. The fresh-per-call approach keeps the structlog pipeline reactive. Removed: - InstrumentNotReadyWarning class (hard removal; 4 weeks old; exports go too) - BaseInstrument.is_ready instance method (replaced by is_configured) - The _register_or_skip helper (flow inlined) - The dead `import_checker.is_prometheus_client_installed` conjunct in FastStreamPrometheusInstrument.is_configured (covered by check_dependencies()). Kept: - InstrumentSkippedWarning (base for forward-compat) - InstrumentDependencyMissingWarning (still useful for the genuine deployment surprise) - not_ready_message class attribute (used in skipped_instruments tuples and the summary log) - Bootstrapper-level is_ready methods (framework availability checks; unrelated) Tests migrated to call XInstrument.is_configured(config) instead of instrument.is_ready(). test_free_bootstrap_logging_disabled rewritten to assert on bootstrapper.skipped_instruments. New test_free_bootstrap_emits_summary_log pins the summary log behavior via capture_logs. Framework instrument overrides (FastStreamOpenTelemetryInstrument, FastStreamPrometheusInstrument, LitestarSwaggerInstrument) need `# ty: ignore[invalid-method-override]` because they narrow bootstrap_config to framework-specific types — LSP violation in static type but correct at runtime (each framework bootstrapper only calls them with its own config). 22 files modified (15 production + 6 tests + 2 docs). Closes design doc 2026-06-01-instrument-skip-rework-design.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 5 +- docs/introduction/configuration.md | 29 ++++---- lite_bootstrap/__init__.py | 2 - lite_bootstrap/bootstrappers/base.py | 68 +++++++++++-------- .../bootstrappers/faststream_bootstrapper.py | 14 ++-- .../bootstrappers/litestar_bootstrapper.py | 5 +- lite_bootstrap/exceptions.py | 4 -- lite_bootstrap/instruments/base.py | 4 +- lite_bootstrap/instruments/cors_instrument.py | 7 +- .../instruments/healthchecks_instrument.py | 5 +- .../instruments/logging_instrument.py | 5 +- .../instruments/opentelemetry_instrument.py | 5 +- .../instruments/prometheus_instrument.py | 7 +- .../instruments/pyroscope_instrument.py | 7 +- .../instruments/sentry_instrument.py | 5 +- tests/instruments/test_cors_instrument.py | 32 ++++----- .../test_healthchecks_instrument.py | 14 ++-- .../instruments/test_prometheus_instrument.py | 28 ++++---- .../instruments/test_pyroscope_instrument.py | 12 ++-- tests/instruments/test_swagger_instrument.py | 6 +- tests/test_fastmcp_bootstrap.py | 14 ++-- tests/test_free_bootstrap.py | 45 ++++++++---- 22 files changed, 176 insertions(+), 147 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 56ded7e..54a1b43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,7 +26,7 @@ BaseConfig (frozen dataclass, kw_only) └── Framework configs compose multiple instrument configs via multiple inheritance BaseInstrument[ConfigT] (generic, non-frozen dataclass with slots) - └── Instrument subclasses: lifecycle via bootstrap() / teardown() / is_ready() + └── Instrument subclasses: lifecycle via bootstrap() / teardown(); skip check via is_configured() classmethod BaseBootstrapper (abc.ABC) ├── FastAPIBootstrapper @@ -39,6 +39,7 @@ BaseBootstrapper (abc.ABC) ### Key design decisions - **Optional dependencies**: Each instrument checks for its optional package via `import_checker.py` (`importlib.util.find_spec`). Instruments are skipped silently if the package is absent. Optional packages are imported inside `if import_checker.is_X_installed:` blocks; static analyzers that don't model this guard will report spurious "possibly unbound" diagnostics — the project uses `ty` which handles the pattern correctly. +- **Instrument skip ordering**: `BaseBootstrapper.__init__` runs `instrument_type.is_configured(config)` first (silent skip if the user's config indicates the instrument shouldn't run — populates `bootstrapper.skipped_instruments: list[tuple[type, str]]`); then `check_dependencies()` (emits `InstrumentDependencyMissingWarning` only for configured-but-dep-missing — the genuine deployment surprise); then instantiates. One `logger.info` summary line at the end lists configured + skipped instruments. The bootstrappers/base.py logger is obtained fresh per call (`_get_logger()`) instead of cached at module level, so `structlog.testing.capture_logs()` can override processors at test time even after `LoggingInstrument.bootstrap()` sets `cache_logger_on_first_use=True`. - **Frozen configs, non-frozen instruments**: All `*Config` classes are `@dataclasses.dataclass(kw_only=True, frozen=True)`. All `*Instrument` classes lose `frozen=True` because two instruments (`LoggingInstrument`, `OpenTelemetryInstrument`) cache mutable runtime state (`_logger_factory`, `_tracer_provider`); Python's dataclass rules require the whole hierarchy to be non-frozen. `from_dict()` and `from_object()` filter unknown keys before constructing. - **`FastAPIConfig.application` uses an `UnsetType` sentinel**: shared in `lite_bootstrap/types.py` as `UnsetType` + `UNSET` (singleton). `FastAPIConfig.__post_init__` checks `isinstance(self.application, UnsetType)` and replaces with a constructed `FastAPI()` via `object.__setattr__` (config stays frozen for user-facing immutability). A one-line comment in `__post_init__` documents the freeze bypass. - **Instrument registry**: `BaseBootstrapper` holds a list of instrument instances; it calls `bootstrap()` on each in order and `teardown()` in reverse during shutdown. @@ -78,7 +79,7 @@ Install via `pip install lite-bootstrap[]` or `uv add lite-bootstrap[ typing.Any: # noqa: ANN401 + """Get a fresh logger instance each call. + + We deliberately avoid a module-level cached logger because structlog's + `cache_logger_on_first_use=True` (set by LoggingInstrument.bootstrap) memoizes the + BoundLogger and its processor chain on first use — making it impossible for + `structlog.testing.capture_logs()` to override the binding after the cache is set. + Returning a fresh proxy per call keeps the structlog pipeline reactive to config changes. + """ + if _structlog_available: + return structlog.get_logger(__name__) + return logging.getLogger(__name__) InstrumentT = typing.TypeVar("InstrumentT", bound=BaseInstrument) @@ -27,6 +40,7 @@ class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): instruments_types: typing.ClassVar[list[type[BaseInstrument]]] instruments: list[BaseInstrument] + skipped_instruments: list[tuple[type[BaseInstrument], str]] bootstrap_config: BaseConfig def __init__(self, bootstrap_config: BaseConfig) -> None: @@ -37,31 +51,29 @@ def __init__(self, bootstrap_config: BaseConfig) -> None: self.bootstrap_config = bootstrap_config self.instruments = [] + self.skipped_instruments = [] for instrument_type in self.instruments_types: - if (instrument := self._register_or_skip(instrument_type)) is not None: - self.instruments.append(instrument) - - def _register_or_skip(self, instrument_type: type[BaseInstrument]) -> BaseInstrument | None: - # Check dependencies before instantiation: an instrument's __init__ - # may reference symbols gated behind an optional import (e.g. a - # default_factory that calls into the missing package), which would - # raise NameError before the check_dependencies skip could run. - if not instrument_type.check_dependencies(): - warnings.warn( - instrument_type.missing_dependency_message, - category=InstrumentDependencyMissingWarning, - stacklevel=4, - ) - return None - instrument = instrument_type(bootstrap_config=self.bootstrap_config) - if not instrument.is_ready(): - warnings.warn( - f"{instrument_type.__name__} is not ready: {instrument.not_ready_message}", - category=InstrumentNotReadyWarning, - stacklevel=4, - ) - return None - return instrument + # Config-level skip first: silent (no warning). Runs before instantiation so a + # missing-optional-dep doesn't fail in a dataclass default_factory before we + # can decide the user opted out. + if not instrument_type.is_configured(self.bootstrap_config): + self.skipped_instruments.append((instrument_type, instrument_type.not_ready_message)) + continue + # Dep-missing for a CONFIGURED instrument is a genuine deployment surprise. + if not instrument_type.check_dependencies(): + warnings.warn( + instrument_type.missing_dependency_message, + category=InstrumentDependencyMissingWarning, + stacklevel=3, + ) + continue + self.instruments.append(instrument_type(bootstrap_config=self.bootstrap_config)) + + _get_logger().info( + f"{type(self).__name__}: " + f"configured={[type(i).__name__ for i in self.instruments]}, " + f"skipped={[(cls.__name__, reason) for cls, reason in self.skipped_instruments]}" + ) @property @abc.abstractmethod @@ -91,7 +103,7 @@ def teardown(self) -> None: one_instrument.teardown() except Exception as e: # noqa: BLE001, PERF203 name = type(one_instrument).__name__ - logger.warning(f"Error tearing down {name}: {e}") + _get_logger().warning(f"Error tearing down {name}: {e}") errors.append((name, e)) if errors: raise TeardownError(errors) from errors[0][1] diff --git a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py index 72ecb25..40974b9 100644 --- a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py @@ -125,8 +125,9 @@ class FastStreamOpenTelemetryInstrument(OpenTelemetryInstrument): bootstrap_config: FastStreamConfig not_ready_message = OpenTelemetryInstrument.not_ready_message + " or opentelemetry_middleware_cls is empty" - def is_ready(self) -> bool: - return super().is_ready() and bool(self.bootstrap_config.opentelemetry_middleware_cls) + @classmethod + def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: # ty: ignore[invalid-method-override] + return super().is_configured(bootstrap_config) and bool(bootstrap_config.opentelemetry_middleware_cls) def bootstrap(self) -> None: if self.bootstrap_config.opentelemetry_middleware_cls and self.bootstrap_config.application.broker: @@ -148,12 +149,9 @@ class FastStreamPrometheusInstrument(PrometheusInstrument): not_ready_message = PrometheusInstrument.not_ready_message + " or prometheus_middleware_cls is missing" missing_dependency_message = "prometheus_client is not installed" - def is_ready(self) -> bool: - return ( - super().is_ready() - and import_checker.is_prometheus_client_installed - and bool(self.bootstrap_config.prometheus_middleware_cls) - ) + @classmethod + def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: # ty: ignore[invalid-method-override] + return super().is_configured(bootstrap_config) and bool(bootstrap_config.prometheus_middleware_cls) @staticmethod def check_dependencies() -> bool: diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index 0addc25..901accc 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -218,8 +218,9 @@ class LitestarSwaggerInstrument(SwaggerInstrument): bootstrap_config: LitestarConfig not_ready_message = "swagger_path is empty or not valid" - def is_ready(self) -> bool: - return bool(self.bootstrap_config.swagger_path) and is_valid_path(self.bootstrap_config.swagger_path) + @classmethod + def is_configured(cls, bootstrap_config: "LitestarConfig") -> bool: # ty: ignore[invalid-method-override] + return bool(bootstrap_config.swagger_path) and is_valid_path(bootstrap_config.swagger_path) def bootstrap(self) -> None: render_plugins: typing.Final = ( diff --git a/lite_bootstrap/exceptions.py b/lite_bootstrap/exceptions.py index 69db96e..6b8a0db 100644 --- a/lite_bootstrap/exceptions.py +++ b/lite_bootstrap/exceptions.py @@ -25,7 +25,3 @@ class InstrumentSkippedWarning(UserWarning): class InstrumentDependencyMissingWarning(InstrumentSkippedWarning): """Emitted when an instrument is skipped because its optional dependency is not installed.""" - - -class InstrumentNotReadyWarning(InstrumentSkippedWarning): - """Emitted when an instrument is skipped because its config indicates it should not run.""" diff --git a/lite_bootstrap/instruments/base.py b/lite_bootstrap/instruments/base.py index 5c2304c..7d7f159 100644 --- a/lite_bootstrap/instruments/base.py +++ b/lite_bootstrap/instruments/base.py @@ -39,7 +39,9 @@ def bootstrap(self) -> None: ... def teardown(self) -> None: ... - def is_ready(self) -> bool: + @classmethod + def is_configured(cls, bootstrap_config: ConfigT) -> bool: # noqa: ARG003 + """Return True if config indicates this instrument should be active. Default: always active.""" return True @staticmethod diff --git a/lite_bootstrap/instruments/cors_instrument.py b/lite_bootstrap/instruments/cors_instrument.py index 1b16de4..e00ccc0 100644 --- a/lite_bootstrap/instruments/cors_instrument.py +++ b/lite_bootstrap/instruments/cors_instrument.py @@ -18,7 +18,6 @@ class CorsConfig(BaseConfig): class CorsInstrument(BaseInstrument[CorsConfig]): not_ready_message = "cors_allowed_origins or cors_allowed_origin_regex must be provided" - def is_ready(self) -> bool: - return bool(self.bootstrap_config.cors_allowed_origins) or bool( - self.bootstrap_config.cors_allowed_origin_regex, - ) + @classmethod + def is_configured(cls, bootstrap_config: "CorsConfig") -> bool: + return bool(bootstrap_config.cors_allowed_origins) or bool(bootstrap_config.cors_allowed_origin_regex) diff --git a/lite_bootstrap/instruments/healthchecks_instrument.py b/lite_bootstrap/instruments/healthchecks_instrument.py index 9d815d4..b6518fa 100644 --- a/lite_bootstrap/instruments/healthchecks_instrument.py +++ b/lite_bootstrap/instruments/healthchecks_instrument.py @@ -30,8 +30,9 @@ def health_check_data(self) -> HealthCheckTypedDict: class HealthChecksInstrument(BaseInstrument[HealthChecksConfig]): not_ready_message = "health_checks_enabled is False" - def is_ready(self) -> bool: - return self.bootstrap_config.health_checks_enabled + @classmethod + def is_configured(cls, bootstrap_config: "HealthChecksConfig") -> bool: + return bootstrap_config.health_checks_enabled def render_health_check_data(self) -> HealthCheckTypedDict: return self.bootstrap_config.health_check_data diff --git a/lite_bootstrap/instruments/logging_instrument.py b/lite_bootstrap/instruments/logging_instrument.py index 0443384..37ef81e 100644 --- a/lite_bootstrap/instruments/logging_instrument.py +++ b/lite_bootstrap/instruments/logging_instrument.py @@ -93,8 +93,9 @@ def structlog_pre_chain_processors(self) -> list[typing.Any]: structlog.processors.UnicodeDecoder(), ] - def is_ready(self) -> bool: - return self.bootstrap_config.logging_enabled + @classmethod + def is_configured(cls, bootstrap_config: "LoggingConfig") -> bool: + return bootstrap_config.logging_enabled @staticmethod def check_dependencies() -> bool: diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index e753ce7..9c1c640 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -85,8 +85,9 @@ class OpenTelemetryInstrument(BaseInstrument[OpenTelemetryConfig]): default_factory=lambda: None, init=False, repr=False, compare=False ) - def is_ready(self) -> bool: - return bool(self.bootstrap_config.opentelemetry_endpoint or self.bootstrap_config.opentelemetry_log_traces) + @classmethod + def is_configured(cls, bootstrap_config: "OpenTelemetryConfig") -> bool: + return bool(bootstrap_config.opentelemetry_endpoint or bootstrap_config.opentelemetry_log_traces) @staticmethod def check_dependencies() -> bool: diff --git a/lite_bootstrap/instruments/prometheus_instrument.py b/lite_bootstrap/instruments/prometheus_instrument.py index 6d1b11e..5f5d5c1 100644 --- a/lite_bootstrap/instruments/prometheus_instrument.py +++ b/lite_bootstrap/instruments/prometheus_instrument.py @@ -16,7 +16,8 @@ class PrometheusConfig(BaseConfig): class PrometheusInstrument(BaseInstrument[PrometheusConfig]): not_ready_message = "prometheus_metrics_path is empty or not valid" - def is_ready(self) -> bool: - return bool(self.bootstrap_config.prometheus_metrics_path) and is_valid_path( - self.bootstrap_config.prometheus_metrics_path + @classmethod + def is_configured(cls, bootstrap_config: "PrometheusConfig") -> bool: + return bool(bootstrap_config.prometheus_metrics_path) and is_valid_path( + bootstrap_config.prometheus_metrics_path ) diff --git a/lite_bootstrap/instruments/pyroscope_instrument.py b/lite_bootstrap/instruments/pyroscope_instrument.py index f2fd2ec..c3631b4 100644 --- a/lite_bootstrap/instruments/pyroscope_instrument.py +++ b/lite_bootstrap/instruments/pyroscope_instrument.py @@ -23,15 +23,16 @@ class PyroscopeInstrument(BaseInstrument[PyroscopeConfig]): not_ready_message = "pyroscope_endpoint is empty" missing_dependency_message = "pyroscope is not installed" - def is_ready(self) -> bool: - return bool(self.bootstrap_config.pyroscope_endpoint) + @classmethod + def is_configured(cls, bootstrap_config: "PyroscopeConfig") -> bool: + return bool(bootstrap_config.pyroscope_endpoint) @staticmethod def check_dependencies() -> bool: return import_checker.is_pyroscope_installed def bootstrap(self) -> None: - # is_ready() guarantees pyroscope_endpoint is set; assert documents the precondition + # is_configured() guarantees pyroscope_endpoint is set; assert documents the precondition # for type narrowing and for direct callers that bypass the bootstrapper. assert self.bootstrap_config.pyroscope_endpoint is not None namespace = self.bootstrap_config.opentelemetry_namespace diff --git a/lite_bootstrap/instruments/sentry_instrument.py b/lite_bootstrap/instruments/sentry_instrument.py index d7faa6e..0d6f3f9 100644 --- a/lite_bootstrap/instruments/sentry_instrument.py +++ b/lite_bootstrap/instruments/sentry_instrument.py @@ -96,8 +96,9 @@ class SentryInstrument(BaseInstrument[SentryConfig]): not_ready_message = "sentry_dsn is empty" missing_dependency_message = "sentry_sdk is not installed" - def is_ready(self) -> bool: - return bool(self.bootstrap_config.sentry_dsn) + @classmethod + def is_configured(cls, bootstrap_config: "SentryConfig") -> bool: + return bool(bootstrap_config.sentry_dsn) @staticmethod def check_dependencies() -> bool: diff --git a/tests/instruments/test_cors_instrument.py b/tests/instruments/test_cors_instrument.py index 7ea6e98..c7183db 100644 --- a/tests/instruments/test_cors_instrument.py +++ b/tests/instruments/test_cors_instrument.py @@ -1,30 +1,28 @@ from lite_bootstrap.instruments.cors_instrument import CorsConfig, CorsInstrument -def test_cors_instrument_not_ready_without_origins_or_regex() -> None: - instrument = CorsInstrument(bootstrap_config=CorsConfig()) - assert not instrument.is_ready() - assert instrument.not_ready_message == "cors_allowed_origins or cors_allowed_origin_regex must be provided" +def test_cors_instrument_not_configured_without_origins_or_regex() -> None: + config = CorsConfig() + assert not CorsInstrument.is_configured(config) + assert CorsInstrument.not_ready_message == "cors_allowed_origins or cors_allowed_origin_regex must be provided" -def test_cors_instrument_ready_with_origins() -> None: - instrument = CorsInstrument(bootstrap_config=CorsConfig(cors_allowed_origins=["http://test"])) - assert instrument.is_ready() +def test_cors_instrument_configured_with_origins() -> None: + config = CorsConfig(cors_allowed_origins=["http://test"]) + assert CorsInstrument.is_configured(config) -def test_cors_instrument_ready_with_regex() -> None: - instrument = CorsInstrument(bootstrap_config=CorsConfig(cors_allowed_origin_regex=r"https?://.*")) - assert instrument.is_ready() +def test_cors_instrument_configured_with_regex() -> None: + config = CorsConfig(cors_allowed_origin_regex=r"https?://.*") + assert CorsInstrument.is_configured(config) -def test_cors_instrument_ready_with_both() -> None: - instrument = CorsInstrument( - bootstrap_config=CorsConfig( - cors_allowed_origins=["http://test"], - cors_allowed_origin_regex=r"https?://.*", - ), +def test_cors_instrument_configured_with_both() -> None: + config = CorsConfig( + cors_allowed_origins=["http://test"], + cors_allowed_origin_regex=r"https?://.*", ) - assert instrument.is_ready() + assert CorsInstrument.is_configured(config) def test_cors_instrument_config_defaults() -> None: diff --git a/tests/instruments/test_healthchecks_instrument.py b/tests/instruments/test_healthchecks_instrument.py index 16e07a8..74d945b 100644 --- a/tests/instruments/test_healthchecks_instrument.py +++ b/tests/instruments/test_healthchecks_instrument.py @@ -1,15 +1,15 @@ from lite_bootstrap.instruments.healthchecks_instrument import HealthChecksConfig, HealthChecksInstrument -def test_healthchecks_instrument_ready_by_default() -> None: - instrument = HealthChecksInstrument(bootstrap_config=HealthChecksConfig()) - assert instrument.is_ready() +def test_healthchecks_instrument_configured_by_default() -> None: + config = HealthChecksConfig() + assert HealthChecksInstrument.is_configured(config) -def test_healthchecks_instrument_not_ready_when_disabled() -> None: - instrument = HealthChecksInstrument(bootstrap_config=HealthChecksConfig(health_checks_enabled=False)) - assert not instrument.is_ready() - assert instrument.not_ready_message == "health_checks_enabled is False" +def test_healthchecks_instrument_not_configured_when_disabled() -> None: + config = HealthChecksConfig(health_checks_enabled=False) + assert not HealthChecksInstrument.is_configured(config) + assert HealthChecksInstrument.not_ready_message == "health_checks_enabled is False" def test_healthchecks_render_data_default() -> None: diff --git a/tests/instruments/test_prometheus_instrument.py b/tests/instruments/test_prometheus_instrument.py index a802d3a..9f650e8 100644 --- a/tests/instruments/test_prometheus_instrument.py +++ b/tests/instruments/test_prometheus_instrument.py @@ -1,28 +1,26 @@ from lite_bootstrap.instruments.prometheus_instrument import PrometheusConfig, PrometheusInstrument -def test_prometheus_instrument_ready_with_default_path() -> None: - instrument = PrometheusInstrument(bootstrap_config=PrometheusConfig()) - assert instrument.is_ready() +def test_prometheus_instrument_configured_with_default_path() -> None: + config = PrometheusConfig() + assert PrometheusInstrument.is_configured(config) -def test_prometheus_instrument_not_ready_with_empty_path() -> None: - instrument = PrometheusInstrument(bootstrap_config=PrometheusConfig(prometheus_metrics_path="")) - assert not instrument.is_ready() - assert instrument.not_ready_message == "prometheus_metrics_path is empty or not valid" +def test_prometheus_instrument_not_configured_with_empty_path() -> None: + config = PrometheusConfig(prometheus_metrics_path="") + assert not PrometheusInstrument.is_configured(config) + assert PrometheusInstrument.not_ready_message == "prometheus_metrics_path is empty or not valid" -def test_prometheus_instrument_not_ready_with_invalid_path() -> None: +def test_prometheus_instrument_not_configured_with_invalid_path() -> None: # No leading slash → invalid per is_valid_path regex. - instrument = PrometheusInstrument(bootstrap_config=PrometheusConfig(prometheus_metrics_path="metrics")) - assert not instrument.is_ready() + config = PrometheusConfig(prometheus_metrics_path="metrics") + assert not PrometheusInstrument.is_configured(config) -def test_prometheus_instrument_ready_with_custom_valid_path() -> None: - instrument = PrometheusInstrument( - bootstrap_config=PrometheusConfig(prometheus_metrics_path="/custom-metrics/"), - ) - assert instrument.is_ready() +def test_prometheus_instrument_configured_with_custom_valid_path() -> None: + config = PrometheusConfig(prometheus_metrics_path="/custom-metrics/") + assert PrometheusInstrument.is_configured(config) def test_prometheus_config_defaults() -> None: diff --git a/tests/instruments/test_pyroscope_instrument.py b/tests/instruments/test_pyroscope_instrument.py index b7a77dc..f400fdd 100644 --- a/tests/instruments/test_pyroscope_instrument.py +++ b/tests/instruments/test_pyroscope_instrument.py @@ -23,14 +23,14 @@ def _make_config(endpoint: str | None = "http://pyroscope:4040") -> PyroscopeCon return PyroscopeConfig(service_name="test-service", pyroscope_endpoint=endpoint) -def test_pyroscope_instrument_not_ready_without_endpoint() -> None: - instrument = PyroscopeInstrument(bootstrap_config=_make_config(endpoint=None)) - assert not instrument.is_ready() +def test_pyroscope_instrument_not_configured_without_endpoint() -> None: + config = _make_config(endpoint=None) + assert not PyroscopeInstrument.is_configured(config) -def test_pyroscope_instrument_is_ready() -> None: - instrument = PyroscopeInstrument(bootstrap_config=_make_config()) - assert instrument.is_ready() +def test_pyroscope_instrument_is_configured() -> None: + config = _make_config() + assert PyroscopeInstrument.is_configured(config) def test_pyroscope_check_dependencies() -> None: diff --git a/tests/instruments/test_swagger_instrument.py b/tests/instruments/test_swagger_instrument.py index 10fee98..83e39c7 100644 --- a/tests/instruments/test_swagger_instrument.py +++ b/tests/instruments/test_swagger_instrument.py @@ -1,9 +1,9 @@ from lite_bootstrap.instruments.swagger_instrument import SwaggerConfig, SwaggerInstrument -def test_swagger_instrument_ready_by_default() -> None: - instrument = SwaggerInstrument(bootstrap_config=SwaggerConfig()) - assert instrument.is_ready() +def test_swagger_instrument_configured_by_default() -> None: + config = SwaggerConfig() + assert SwaggerInstrument.is_configured(config) def test_swagger_config_defaults() -> None: diff --git a/tests/test_fastmcp_bootstrap.py b/tests/test_fastmcp_bootstrap.py index 7a8c991..0cbb35a 100644 --- a/tests/test_fastmcp_bootstrap.py +++ b/tests/test_fastmcp_bootstrap.py @@ -240,16 +240,18 @@ def test_fastmcp_logging_middleware_disabled_via_flag() -> None: @pytest.mark.parametrize( - "package_name", + ("package_name", "extra_config"), [ - "sentry_sdk", - "structlog", - "prometheus_client", + ("sentry_sdk", {"sentry_dsn": "https://testdsn@localhost/1"}), + ("structlog", {}), + ("prometheus_client", {}), ], ) -def test_fastmcp_bootstrapper_with_missing_instrument_dependency(package_name: str) -> None: +def test_fastmcp_bootstrapper_with_missing_instrument_dependency( + package_name: str, extra_config: dict[str, typing.Any] +) -> None: with emulate_package_missing(package_name), pytest.warns(UserWarning, match=package_name): - FastMcpBootstrapper(bootstrap_config=FastMcpConfig()) + FastMcpBootstrapper(bootstrap_config=FastMcpConfig(**extra_config)) def test_fastmcp_bootstrap_without_prometheus_client() -> None: diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index 0157c77..0ce7197 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -7,9 +7,10 @@ from lite_bootstrap import ( FreeBootstrapper, FreeConfig, - InstrumentNotReadyWarning, TeardownError, ) +from lite_bootstrap.instruments.logging_instrument import LoggingInstrument +from lite_bootstrap.instruments.pyroscope_instrument import PyroscopeInstrument from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing @@ -37,20 +38,19 @@ def test_free_bootstrap(free_bootstrapper_config: FreeConfig) -> None: def test_free_bootstrap_logging_disabled() -> None: - with pytest.warns(InstrumentNotReadyWarning) as records: - FreeBootstrapper( - bootstrap_config=FreeConfig( - logging_enabled=False, - opentelemetry_instrumentors=[CustomInstrumentor()], - opentelemetry_log_traces=True, - sentry_dsn="https://testdsn@localhost/1", - sentry_additional_params={"transport": SentryTestTransport()}, - logging_buffer_capacity=0, - ), - ) - messages = [str(r.message) for r in records] - assert "LoggingInstrument is not ready: logging_enabled is False" in messages - assert "PyroscopeInstrument is not ready: pyroscope_endpoint is empty" in messages + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeConfig( + logging_enabled=False, + opentelemetry_instrumentors=[CustomInstrumentor()], + opentelemetry_log_traces=True, + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + logging_buffer_capacity=0, + ), + ) + skipped_classes = {cls for cls, _ in bootstrapper.skipped_instruments} + assert LoggingInstrument in skipped_classes + assert PyroscopeInstrument in skipped_classes def test_teardown_error_isolation(free_bootstrapper_config: FreeConfig) -> None: @@ -139,3 +139,18 @@ def test_bootstrap_is_idempotent(free_bootstrapper_config: FreeConfig) -> None: first.bootstrap.assert_called_once() second.bootstrap.assert_called_once() assert bootstrapper.is_bootstrapped + + +def test_free_bootstrap_emits_summary_log() -> None: + with capture_logs() as cap_logs: + FreeBootstrapper( + bootstrap_config=FreeConfig( + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + ), + ) + summary_entries = [e for e in cap_logs if "FreeBootstrapper" in e.get("event", "")] + assert summary_entries, "expected a summary log entry mentioning FreeBootstrapper" + summary_event = summary_entries[-1]["event"] + assert "configured=" in summary_event + assert "skipped=" in summary_event From f404d0a000dde6b645964a9c39c2319a5c71443e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 09:52:36 +0300 Subject: [PATCH 2/8] docs: add instrument skip rework implementation plan --- .../2026-06-01-instrument-skip-rework.md | 939 ++++++++++++++++++ lite_bootstrap/bootstrappers/base.py | 28 +- 2 files changed, 952 insertions(+), 15 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-01-instrument-skip-rework.md diff --git a/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md b/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md new file mode 100644 index 0000000..a907f42 --- /dev/null +++ b/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md @@ -0,0 +1,939 @@ +# Instrument Skip Rework Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace `InstrumentNotReadyWarning` with a pre-instantiation `is_configured` classmethod check. Skipped-due-to-config becomes silent at the warning level; structured `bootstrapper.skipped_instruments` introspection + one INFO summary log provide diagnostic visibility. `InstrumentDependencyMissingWarning` continues to fire for the genuine "configured but dependency missing" deployment-surprise case. + +**Architecture:** `BaseInstrument.is_ready(self) -> bool` (instance method) is removed and replaced with `BaseInstrument.is_configured(cls, bootstrap_config) -> bool` (classmethod). The bootstrapper's `_register_or_skip` helper is inlined; the new flow checks `is_configured(config)` BEFORE `check_dependencies()` BEFORE instantiation. Silent skip on is_configured False; warning on check_dependencies False; otherwise instantiate. After the loop, emit one INFO log summarizing configured + skipped instruments. Bootstrapper-level `is_ready` methods (framework availability checks like "fastapi is not installed") are unchanged. + +**Tech Stack:** Python 3.10+, dataclasses, stdlib `warnings` + `logging`, pytest (caplog), structlog (transitive). + +**Parent spec:** `docs/superpowers/specs/2026-06-01-instrument-skip-rework-design.md` + +--- + +## File Structure + +15 production files modified, 6 test files modified, 2 docs modified, 0 new files. + +**Production (12 instrument-side + 3 bootstrapper-side files):** + +| File | Change | +|------|--------| +| `lite_bootstrap/instruments/base.py` | `BaseInstrument.is_ready` → `is_configured` classmethod | +| `lite_bootstrap/instruments/cors_instrument.py` | `CorsInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/healthchecks_instrument.py` | `HealthChecksInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/logging_instrument.py` | `LoggingInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/opentelemetry_instrument.py` | `OpenTelemetryInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/prometheus_instrument.py` | `PrometheusInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/pyroscope_instrument.py` | `PyroscopeInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/sentry_instrument.py` | `SentryInstrument.is_ready` → `is_configured` | +| `lite_bootstrap/instruments/swagger_instrument.py` | (none — uses default) | +| `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` | `FastStreamOpenTelemetryInstrument.is_ready` and `FastStreamPrometheusInstrument.is_ready` → `is_configured`. `FastStreamBootstrapper.is_ready` (bootstrapper-level) **unchanged**. | +| `lite_bootstrap/bootstrappers/litestar_bootstrapper.py` | `LitestarSwaggerInstrument.is_ready` → `is_configured`. `LitestarBootstrapper.is_ready` **unchanged**. | +| `lite_bootstrap/bootstrappers/base.py` | Replace `_register_or_skip` with inline flow; add `skipped_instruments`; emit summary log; remove `InstrumentNotReadyWarning` import. | +| `lite_bootstrap/exceptions.py` | Remove `InstrumentNotReadyWarning` class. | +| `lite_bootstrap/__init__.py` | Remove `InstrumentNotReadyWarning` import + `__all__` entry. | + +**Tests (6 files):** + +| File | Change | +|------|--------| +| `tests/test_free_bootstrap.py` | `test_free_bootstrap_logging_disabled` rewritten. New `test_bootstrap_emits_summary_log`. | +| `tests/instruments/test_cors_instrument.py` | `is_ready()` → `is_configured(config)` | +| `tests/instruments/test_healthchecks_instrument.py` | same | +| `tests/instruments/test_prometheus_instrument.py` | same | +| `tests/instruments/test_pyroscope_instrument.py` | same | +| `tests/instruments/test_swagger_instrument.py` | same | + +**Docs:** + +| File | Change | +|------|--------| +| `docs/introduction/configuration.md` | Revise the PR #86 warning subclasses section (introduced ~31 lines; updated to reflect single dep-missing warning + `skipped_instruments` + summary log). | +| `CLAUDE.md` | Update "Optional dependencies" key design decision to mention `is_configured` ordering. | + +--- + +## Pre-flight grep verification + +Before starting, confirm scope matches reality: + +```bash +grep -rn "def is_ready\|\.is_ready(" lite_bootstrap/ tests/ --include="*.py" +``` + +Expected count: +- 8 `def is_ready(self)` in `lite_bootstrap/instruments/` (one per base instrument file). +- 3 `def is_ready(self)` instrument-level overrides in `lite_bootstrap/bootstrappers/` (FastStream OTel, FastStream Prometheus, Litestar Swagger). +- 5 `def is_ready(self)` BOOTSTRAPPER-LEVEL methods (FastAPI, Litestar, FastStream, Free, FastMcp Bootstrappers) — these are NOT migrated. +- 1 `def is_ready(self)` abstract on `BaseBootstrapper` — NOT migrated. +- 2 `self.is_ready()` calls in `bootstrappers/base.py` (one in `__init__` bootstrapper check at line 34, one in `_register_or_skip` instrument check at line 57) — only the line 57 call migrates. +- ~14 `instrument.is_ready()` calls in test files — all migrate. + +```bash +grep -rn "not_ready_message" lite_bootstrap/ tests/ --include="*.py" +``` + +Expected: `not_ready_message` is preserved as a class attribute (used in `skipped_instruments` tuples and bootstrapper-level `BootstrapperNotReadyError`). Only its emission in the deleted `InstrumentNotReadyWarning` goes away. + +```bash +grep -rn "InstrumentNotReadyWarning" lite_bootstrap/ tests/ --include="*.py" +``` + +Expected: 5 matches (declaration in `exceptions.py`, import + emission in `bootstrappers/base.py`, import + export in `__init__.py`, import + usage in `tests/test_free_bootstrap.py`). All are removed by this plan. + +--- + +## Task 1: Create branch + +**Files:** (no files; git only) + +- [ ] **Step 1: Branch off `main`** (NOT the current feat/fastmcp-bootstrapper branch — this refactor is independent) + +```bash +git checkout main +git pull --ff-only origin main +git checkout -b refactor/instrument-skip-rework +``` + +Expected: `Switched to a new branch 'refactor/instrument-skip-rework'`. + +Note: the design spec (`docs/superpowers/specs/2026-06-01-instrument-skip-rework-design.md`) was committed to `feat/fastmcp-bootstrapper`. This plan does not depend on it being on main; the plan is self-contained. + +--- + +## Task 2: Migrate `BaseInstrument` to `is_configured` classmethod + +**File:** `lite_bootstrap/instruments/base.py` + +- [ ] **Step 1: Replace the `BaseInstrument` body** + +Current (lines 32-47): + +```python +@dataclasses.dataclass(kw_only=True, slots=True) +class BaseInstrument(typing.Generic[ConfigT]): + bootstrap_config: ConfigT + not_ready_message = "" + missing_dependency_message = "" + + def bootstrap(self) -> None: ... + + def teardown(self) -> None: ... + + def is_ready(self) -> bool: + return True + + @staticmethod + def check_dependencies() -> bool: + return True +``` + +Replace with: + +```python +@dataclasses.dataclass(kw_only=True, slots=True) +class BaseInstrument(typing.Generic[ConfigT]): + bootstrap_config: ConfigT + not_ready_message = "" + missing_dependency_message = "" + + def bootstrap(self) -> None: ... + + def teardown(self) -> None: ... + + @classmethod + def is_configured(cls, bootstrap_config: ConfigT) -> bool: + """Return True if config indicates this instrument should be active. Default: always active.""" + return True + + @staticmethod + def check_dependencies() -> bool: + return True +``` + +Two changes: +1. Remove the `is_ready` instance method. +2. Add `is_configured` classmethod with default `return True`. + +The class attribute `not_ready_message` stays. + +--- + +## Task 3: Migrate each base instrument's `is_ready` to `is_configured` + +Eight instrument files, mechanical migration. Each step is one file. + +- [ ] **Step 1: `lite_bootstrap/instruments/cors_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.cors_allowed_origins) or bool( + self.bootstrap_config.cors_allowed_origin_regex, + ) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "CorsConfig") -> bool: + return bool(bootstrap_config.cors_allowed_origins) or bool(bootstrap_config.cors_allowed_origin_regex) +``` + +- [ ] **Step 2: `lite_bootstrap/instruments/healthchecks_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return self.bootstrap_config.health_checks_enabled +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "HealthChecksConfig") -> bool: + return bootstrap_config.health_checks_enabled +``` + +- [ ] **Step 3: `lite_bootstrap/instruments/logging_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return self.bootstrap_config.logging_enabled +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "LoggingConfig") -> bool: + return bootstrap_config.logging_enabled +``` + +- [ ] **Step 4: `lite_bootstrap/instruments/opentelemetry_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.opentelemetry_endpoint or self.bootstrap_config.opentelemetry_log_traces) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "OpenTelemetryConfig") -> bool: + return bool(bootstrap_config.opentelemetry_endpoint or bootstrap_config.opentelemetry_log_traces) +``` + +- [ ] **Step 5: `lite_bootstrap/instruments/prometheus_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.prometheus_metrics_path) and is_valid_path( + self.bootstrap_config.prometheus_metrics_path + ) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "PrometheusConfig") -> bool: + return bool(bootstrap_config.prometheus_metrics_path) and is_valid_path(bootstrap_config.prometheus_metrics_path) +``` + +- [ ] **Step 6: `lite_bootstrap/instruments/pyroscope_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.pyroscope_endpoint) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "PyroscopeConfig") -> bool: + return bool(bootstrap_config.pyroscope_endpoint) +``` + +- [ ] **Step 7: `lite_bootstrap/instruments/sentry_instrument.py`** + +Find: + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.sentry_dsn) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "SentryConfig") -> bool: + return bool(bootstrap_config.sentry_dsn) +``` + +- [ ] **Step 8: `lite_bootstrap/instruments/swagger_instrument.py`** + +`SwaggerInstrument` doesn't override `is_ready` (uses the default `return True` from the base). No change needed in this file. + +- [ ] **Step 9: Smoke check — instruments module imports cleanly** + +```bash +uv run python -c "from lite_bootstrap.instruments.cors_instrument import CorsInstrument, CorsConfig; assert CorsInstrument.is_configured(CorsConfig()) is False; print('ok')" +``` + +Expected: `ok`. If `AttributeError: type object 'CorsInstrument' has no attribute 'is_configured'` — Task 3 has a missed file. + +--- + +## Task 4: Migrate framework instrument-level overrides + +Three instrument-level `is_ready` overrides live in bootstrapper files. Migrate them. + +- [ ] **Step 1: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` — FastStreamOpenTelemetryInstrument** + +Find (around line 128-129): + +```python + def is_ready(self) -> bool: + return super().is_ready() and bool(self.bootstrap_config.opentelemetry_middleware_cls) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: + return super().is_configured(bootstrap_config) and bool(bootstrap_config.opentelemetry_middleware_cls) +``` + +- [ ] **Step 2: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` — FastStreamPrometheusInstrument** + +Find (around line 151-155): + +```python + def is_ready(self) -> bool: + return ( + super().is_ready() + and import_checker.is_prometheus_client_installed + and bool(self.bootstrap_config.prometheus_middleware_cls) + ) +``` + +Replace with (dropping the dead `import_checker.is_prometheus_client_installed` conjunct per DES-5): + +```python + @classmethod + def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: + return super().is_configured(bootstrap_config) and bool(bootstrap_config.prometheus_middleware_cls) +``` + +The `import_checker.is_prometheus_client_installed` check is dead because `_register_or_skip` will call `check_dependencies()` AFTER `is_configured()` returns True, and `check_dependencies()` already checks `import_checker.is_prometheus_client_installed`. + +- [ ] **Step 3: `lite_bootstrap/bootstrappers/litestar_bootstrapper.py` — LitestarSwaggerInstrument** + +Find (around line 221, the instrument-level — NOT the bootstrapper-level at line 276): + +```python + def is_ready(self) -> bool: + return bool(self.bootstrap_config.swagger_path) and is_valid_path(self.bootstrap_config.swagger_path) +``` + +Replace with: + +```python + @classmethod + def is_configured(cls, bootstrap_config: "LitestarConfig") -> bool: + return bool(bootstrap_config.swagger_path) and is_valid_path(bootstrap_config.swagger_path) +``` + +- [ ] **Step 4: Sanity grep — no leftover instrument-level is_ready** + +```bash +grep -n "def is_ready" lite_bootstrap/instruments/ lite_bootstrap/bootstrappers/ +``` + +Expected matches: only bootstrapper-level overrides (`FastAPIBootstrapper.is_ready` in `fastapi_bootstrapper.py`, `LitestarBootstrapper.is_ready` at `litestar_bootstrapper.py:276`, `FastStreamBootstrapper.is_ready` at `faststream_bootstrapper.py:186`, `FreeBootstrapper.is_ready` in `free_bootstrapper.py`, `FastMcpBootstrapper.is_ready` in `fastmcp_bootstrapper.py`, and the abstract `BaseBootstrapper.is_ready`). No instrument-level matches. + +--- + +## Task 5: Rewrite `BaseBootstrapper` flow + +**File:** `lite_bootstrap/bootstrappers/base.py` + +- [ ] **Step 1: Remove `InstrumentNotReadyWarning` import** + +Find (lines 6-11): + +```python +from lite_bootstrap.exceptions import ( + BootstrapperNotReadyError, + InstrumentDependencyMissingWarning, + InstrumentNotReadyWarning, + TeardownError, +) +``` + +Replace with: + +```python +from lite_bootstrap.exceptions import ( + BootstrapperNotReadyError, + InstrumentDependencyMissingWarning, + TeardownError, +) +``` + +- [ ] **Step 2: Inline the new flow in `__init__` and add `skipped_instruments` attribute** + +Find the existing class header + `__init__` + `_register_or_skip` (lines 27-64): + +```python +class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): + instruments_types: typing.ClassVar[list[type[BaseInstrument]]] + instruments: list[BaseInstrument] + bootstrap_config: BaseConfig + + def __init__(self, bootstrap_config: BaseConfig) -> None: + self.is_bootstrapped = False + if not self.is_ready(): + msg = f"{type(self).__name__} is not ready: {self.not_ready_message}" + raise BootstrapperNotReadyError(msg) + + self.bootstrap_config = bootstrap_config + self.instruments = [] + for instrument_type in self.instruments_types: + if (instrument := self._register_or_skip(instrument_type)) is not None: + self.instruments.append(instrument) + + def _register_or_skip(self, instrument_type: type[BaseInstrument]) -> BaseInstrument | None: + # Check dependencies before instantiation: an instrument's __init__ + # may reference symbols gated behind an optional import (e.g. a + # default_factory that calls into the missing package), which would + # raise NameError before the check_dependencies skip could run. + if not instrument_type.check_dependencies(): + warnings.warn( + instrument_type.missing_dependency_message, + category=InstrumentDependencyMissingWarning, + stacklevel=4, + ) + return None + instrument = instrument_type(bootstrap_config=self.bootstrap_config) + if not instrument.is_ready(): + warnings.warn( + f"{instrument_type.__name__} is not ready: {instrument.not_ready_message}", + category=InstrumentNotReadyWarning, + stacklevel=4, + ) + return None + return instrument +``` + +Replace with: + +```python +class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): + instruments_types: typing.ClassVar[list[type[BaseInstrument]]] + instruments: list[BaseInstrument] + skipped_instruments: list[tuple[type[BaseInstrument], str]] + bootstrap_config: BaseConfig + + def __init__(self, bootstrap_config: BaseConfig) -> None: + self.is_bootstrapped = False + if not self.is_ready(): + msg = f"{type(self).__name__} is not ready: {self.not_ready_message}" + raise BootstrapperNotReadyError(msg) + + self.bootstrap_config = bootstrap_config + self.instruments = [] + self.skipped_instruments = [] + for instrument_type in self.instruments_types: + # Config-level skip first: silent (no warning). Runs before instantiation so a + # missing-optional-dep doesn't fail in a dataclass default_factory before we + # can decide the user opted out. + if not instrument_type.is_configured(self.bootstrap_config): + self.skipped_instruments.append((instrument_type, instrument_type.not_ready_message)) + continue + # Dep-missing for a CONFIGURED instrument is a genuine deployment surprise. + if not instrument_type.check_dependencies(): + warnings.warn( + instrument_type.missing_dependency_message, + category=InstrumentDependencyMissingWarning, + stacklevel=3, + ) + continue + self.instruments.append(instrument_type(bootstrap_config=self.bootstrap_config)) + + logger.info( + f"{type(self).__name__}: " + f"configured={[type(i).__name__ for i in self.instruments]}, " + f"skipped={[(cls.__name__, reason) for cls, reason in self.skipped_instruments]}" + ) +``` + +Five changes: +1. Add `skipped_instruments: list[tuple[type[BaseInstrument], str]]` class-level annotation. +2. Initialize `self.skipped_instruments = []` in `__init__`. +3. Inline the new flow: `is_configured` first (silent skip + append to skipped_instruments), then `check_dependencies` (warning, no skipped_instruments entry), then instantiate. +4. Delete the `_register_or_skip` helper method entirely. +5. Emit one `logger.info(...)` summary after the loop. + +The `stacklevel=3` is reduced from `stacklevel=4` because the warning call is now in `__init__` directly (one fewer frame than via `_register_or_skip`). + +The `logger` reference is the existing module-level logger declared at line 16-21 (structlog if available, stdlib otherwise) — no new import needed. + +The `not_ready_message` is accessed on the **class** (not an instance), since `is_configured` runs without instantiation. + +- [ ] **Step 3: Smoke check — bootstrapper module imports cleanly** + +```bash +uv run python -c "from lite_bootstrap.bootstrappers.base import BaseBootstrapper; print('ok')" +``` + +Expected: `ok`. If `ImportError: cannot import name 'InstrumentNotReadyWarning'` — Task 5 Step 1 wasn't applied. + +--- + +## Task 6: Migrate tests to new API + +Six test files. Mechanical `is_ready()` → `is_configured(config)` migration plus the `test_free_bootstrap_logging_disabled` rewrite. + +- [ ] **Step 1: `tests/instruments/test_cors_instrument.py`** + +The file has several `instrument.is_ready()` calls. Find each `assert instrument.is_ready()` and `assert not instrument.is_ready()` pattern and rewrite to use the classmethod on the config used. + +Concretely, the pattern: + +```python +def test_cors_instrument_not_ready_without_origins_or_regex() -> None: + instrument = CorsInstrument(bootstrap_config=CorsConfig()) + assert not instrument.is_ready() + assert instrument.not_ready_message == "cors_allowed_origins or cors_allowed_origin_regex must be provided" +``` + +becomes: + +```python +def test_cors_instrument_not_configured_without_origins_or_regex() -> None: + config = CorsConfig() + assert not CorsInstrument.is_configured(config) + assert CorsInstrument.not_ready_message == "cors_allowed_origins or cors_allowed_origin_regex must be provided" +``` + +Apply the same pattern to all `is_ready()` calls in the file. Rename the test function from `*_not_ready_*` to `*_not_configured_*` (and `*_ready_*` to `*_configured_*` for positive cases) to match the new semantics. + +- [ ] **Step 2: `tests/instruments/test_healthchecks_instrument.py`** + +Same pattern. Rename `*_ready_*` → `*_configured_*`. Construct config, pass to `HealthChecksInstrument.is_configured(config)`. + +- [ ] **Step 3: `tests/instruments/test_prometheus_instrument.py`** + +Same pattern. Use `PrometheusInstrument.is_configured(config)`. + +- [ ] **Step 4: `tests/instruments/test_pyroscope_instrument.py`** + +Same pattern. Use `PyroscopeInstrument.is_configured(config)`. + +- [ ] **Step 5: `tests/instruments/test_swagger_instrument.py`** + +Same pattern. Use `SwaggerInstrument.is_configured(config)`. (The default returns True, so positive cases simply assert True.) + +- [ ] **Step 6: `tests/test_free_bootstrap.py::test_free_bootstrap_logging_disabled`** + +Find the current test: + +```python +def test_free_bootstrap_logging_disabled() -> None: + with pytest.warns(InstrumentNotReadyWarning) as records: + FreeBootstrapper( + bootstrap_config=FreeBootstrapperConfig( + logging_enabled=False, + opentelemetry_instrumentors=[CustomInstrumentor()], + opentelemetry_log_traces=True, + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + logging_buffer_capacity=0, + ), + ) + messages = [str(r.message) for r in records] + assert "LoggingInstrument is not ready: logging_enabled is False" in messages + assert "PyroscopeInstrument is not ready: pyroscope_endpoint is empty" in messages +``` + +Replace with: + +```python +def test_free_bootstrap_logging_disabled() -> None: + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeBootstrapperConfig( + logging_enabled=False, + opentelemetry_instrumentors=[CustomInstrumentor()], + opentelemetry_log_traces=True, + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + logging_buffer_capacity=0, + ), + ) + skipped_classes = {cls for cls, _ in bootstrapper.skipped_instruments} + assert LoggingInstrument in skipped_classes + assert PyroscopeInstrument in skipped_classes +``` + +Also at the top of the file: +- Add `from lite_bootstrap.instruments.logging_instrument import LoggingInstrument` +- Add `from lite_bootstrap.instruments.pyroscope_instrument import PyroscopeInstrument` +- Remove `InstrumentNotReadyWarning` from the `from lite_bootstrap import (...)` import block + +- [ ] **Step 7: Verify all instrument tests pass** + +```bash +just test -- tests/instruments/ tests/test_free_bootstrap.py -v +``` + +Expected: all pass. If any test references `is_ready()` it's a missed migration — fix and re-run. + +--- + +## Task 7: Add summary-log test + +- [ ] **Step 1: Append to `tests/test_free_bootstrap.py`** + +Add: + +```python +def test_free_bootstrap_emits_summary_log(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.INFO, logger="lite_bootstrap.bootstrappers.base"): + FreeBootstrapper( + bootstrap_config=FreeBootstrapperConfig( + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + ), + ) + summary_records = [r for r in caplog.records if "FreeBootstrapper" in r.getMessage()] + assert summary_records, "expected a summary log line mentioning FreeBootstrapper" + summary = summary_records[-1].getMessage() + assert "configured=" in summary + assert "skipped=" in summary +``` + +Also add `import logging` at the top if not already present, and add `pytest` to imports if needed (already in test_free_bootstrap.py as `import pytest`). + +- [ ] **Step 2: Run the new test** + +```bash +just test -- tests/test_free_bootstrap.py::test_free_bootstrap_emits_summary_log -v +``` + +Expected: PASS. + +--- + +## Task 8: Remove `InstrumentNotReadyWarning` class + +- [ ] **Step 1: `lite_bootstrap/exceptions.py`** + +Find (lines 30-31 approx): + +```python +class InstrumentNotReadyWarning(InstrumentSkippedWarning): + """Emitted when an instrument is skipped because its config indicates it should not run.""" +``` + +Delete those two lines and the blank line immediately before/after as appropriate. Make sure `InstrumentSkippedWarning` and `InstrumentDependencyMissingWarning` definitions remain. + +- [ ] **Step 2: `lite_bootstrap/__init__.py`** + +Find and update the import block (lines 6-14 approx): + +```python +from lite_bootstrap.exceptions import ( + BootstrapperNotReadyError, + ConfigurationError, + InstrumentDependencyMissingWarning, + InstrumentNotReadyWarning, + InstrumentSkippedWarning, + LiteBootstrapError, + TeardownError, +) +``` + +Remove the `InstrumentNotReadyWarning,` line so it becomes: + +```python +from lite_bootstrap.exceptions import ( + BootstrapperNotReadyError, + ConfigurationError, + InstrumentDependencyMissingWarning, + InstrumentSkippedWarning, + LiteBootstrapError, + TeardownError, +) +``` + +Find the `__all__` list and remove `"InstrumentNotReadyWarning",` (alphabetically between `InstrumentDependencyMissingWarning` and `InstrumentSkippedWarning`). + +- [ ] **Step 3: Sanity grep — no leftover references** + +```bash +grep -rn "InstrumentNotReadyWarning" lite_bootstrap/ tests/ --include="*.py" +``` + +Expected: zero matches. If any remain, they're missed migrations. + +- [ ] **Step 4: Full test suite** + +```bash +just test +``` + +Expected: all tests pass. Existing dep-missing tests (e.g., `test_fastapi_bootstrapper_with_missing_instrument_dependency`) still fire `InstrumentDependencyMissingWarning` and continue to pass. + +```bash +just lint +``` + +Expected: clean. The `_register_or_skip` removal also removes any reference to its name; no orphan symbols. + +--- + +## Task 9: Update docs + +- [ ] **Step 1: `CLAUDE.md` — update Optional dependencies design-decision bullet** + +Find the "Key design decisions" section, the bullet starting with `**Optional dependencies**:`. Append after the existing sentence about Pyright (or replace the trailing static-analyzer note): + +Add a new bullet immediately after the "Optional dependencies" one: + +```markdown +- **Instrument skip ordering**: `BaseBootstrapper.__init__` runs `instrument_type.is_configured(config)` first (silent skip if the user's config indicates the instrument shouldn't run — populates `bootstrapper.skipped_instruments`); then `check_dependencies()` (emits `InstrumentDependencyMissingWarning` only for configured-but-dep-missing — the genuine deployment surprise); then instantiates. One `logger.info` summary line at the end lists configured + skipped instruments. +``` + +- [ ] **Step 2: `docs/introduction/configuration.md` — revise the warning-subclasses section** + +Open the file and locate the section that PR #86 introduced (search for `InstrumentNotReadyWarning` or `InstrumentSkippedWarning`). This section documents how users can filter / capture the warning subclasses. + +Replace the section content with text reflecting the new behavior: +- Only `InstrumentDependencyMissingWarning` is emitted by the bootstrapper. +- It fires only when the instrument is configured AND its optional dependency is missing. +- Instruments skipped due to config (`is_configured` False) appear in `bootstrapper.skipped_instruments: list[tuple[type, str]]` and in the INFO-level summary log. + +Keep `InstrumentSkippedWarning` as the documented base for forward-compatibility (if additional skip categories arise in future). + +Remove any code samples referencing `InstrumentNotReadyWarning`. + +- [ ] **Step 3: Verify docs build** + +```bash +uv run --with mkdocs --with mkdocs-material mkdocs build --strict > /dev/null 2>&1; echo "exit: $?" +``` + +Expected: `exit: 0`. (The audit's earlier docs/superpowers/ exclusion remains in mkdocs.yml.) + +--- + +## Task 10: Final verification and commit + +- [ ] **Step 1: Run the full test suite once more** + +```bash +just test +``` + +Expected: all pass (count = 129 prior + 1 new summary-log test = 130). + +- [ ] **Step 2: Run lint** + +```bash +just lint +``` + +Expected: clean. + +- [ ] **Step 3: Pre-flight grep for any leftover old-API references** + +```bash +grep -rn "def is_ready\|\.is_ready(" lite_bootstrap/ tests/ --include="*.py" | grep -v "Bootstrapper\b" +``` + +Expected: only the abstract `def is_ready(self) -> bool: ...` on `BaseBootstrapper` (line 74) and the bootstrapper-level overrides in framework files. Any instrument-level match is a missed migration. + +```bash +grep -rn "InstrumentNotReadyWarning" . +``` + +Expected: only matches in `docs/superpowers/specs/` and `docs/superpowers/plans/` (this plan and the spec). Production code and tests should have zero matches. + +- [ ] **Step 4: Commit** + +Stage all touched files explicitly: + +```bash +git add \ + lite_bootstrap/instruments/base.py \ + lite_bootstrap/instruments/cors_instrument.py \ + lite_bootstrap/instruments/healthchecks_instrument.py \ + lite_bootstrap/instruments/logging_instrument.py \ + lite_bootstrap/instruments/opentelemetry_instrument.py \ + lite_bootstrap/instruments/prometheus_instrument.py \ + lite_bootstrap/instruments/pyroscope_instrument.py \ + lite_bootstrap/instruments/sentry_instrument.py \ + lite_bootstrap/bootstrappers/base.py \ + lite_bootstrap/bootstrappers/faststream_bootstrapper.py \ + lite_bootstrap/bootstrappers/litestar_bootstrapper.py \ + lite_bootstrap/exceptions.py \ + lite_bootstrap/__init__.py \ + tests/test_free_bootstrap.py \ + tests/instruments/test_cors_instrument.py \ + tests/instruments/test_healthchecks_instrument.py \ + tests/instruments/test_prometheus_instrument.py \ + tests/instruments/test_pyroscope_instrument.py \ + tests/instruments/test_swagger_instrument.py \ + CLAUDE.md \ + docs/introduction/configuration.md +git commit -m "$(cat <<'EOF' +refactor: replace InstrumentNotReadyWarning with is_configured classmethod + summary log + +PR #86 introduced InstrumentNotReadyWarning, escalating the not-ready +skip path from a (silently dropped) logger.info call to a real +warnings.warn. The escalation surfaced expected-path events as +warnings: every service using a subset of available instruments emits +N warnings on bootstrap, and lite-bootstrap's own tests can't suppress +without breaking the warning assertions. + +Replace is_ready (instance method, ran after instantiation) with +is_configured (classmethod taking config, runs before instantiation). +The bootstrapper now: + +1. is_configured(config) False → silent skip; append to new + bootstrapper.skipped_instruments: list[tuple[type, str]]. +2. check_dependencies() False → InstrumentDependencyMissingWarning + (kept; this is the genuine "configured but dep missing" deployment + surprise). +3. Otherwise instantiate and register. + +After the loop, emit one INFO-level summary log listing configured + +skipped instruments. Default Python logging suppresses INFO, so silent +by default with an opt-in path via logging.basicConfig. + +PR #88's constraint (check_dependencies before instantiation, because +some instruments have default_factory that NameErrors when optional +extras aren't installed) is preserved: is_configured is a classmethod, +so it runs without instantiation. + +Removed: +- InstrumentNotReadyWarning class (hard removal; 4 weeks old; exports + go too) +- BaseInstrument.is_ready instance method (replaced by is_configured) +- The _register_or_skip helper (flow inlined) +- The dead `import_checker.is_prometheus_client_installed` conjunct in + FastStreamPrometheusInstrument.is_configured (covered by + check_dependencies()). + +Kept: +- InstrumentSkippedWarning (base for forward-compat) +- InstrumentDependencyMissingWarning (still useful for the genuine + deployment surprise) +- not_ready_message class attribute (used in skipped_instruments + tuples and the summary log) +- Bootstrapper-level is_ready methods (framework availability checks; + unrelated) + +Tests migrated to call XInstrument.is_configured(config) instead of +instrument.is_ready(). test_free_bootstrap_logging_disabled rewritten +to assert on bootstrapper.skipped_instruments. New +test_free_bootstrap_emits_summary_log pins the summary log behavior +via caplog. + +Closes design doc 2026-06-01-instrument-skip-rework-design.md. +EOF +)" +``` + +--- + +## Task 11: Push + open PR + +- [ ] **Step 1: Push** + +```bash +git push -u origin refactor/instrument-skip-rework +``` + +- [ ] **Step 2: Open PR** + +```bash +gh pr create --title "refactor: replace InstrumentNotReadyWarning with is_configured classmethod + summary log" --body "$(cat <<'EOF' +## Summary +Replaces the post-PR-#86 `InstrumentNotReadyWarning` (which fired on every config-opt-out path, generating noise in every service) with a pre-instantiation `is_configured` classmethod check, structured `bootstrapper.skipped_instruments` introspection, and one INFO-level summary log. + +- `is_ready` (instance method) → `is_configured` (classmethod taking config). Runs **before** `check_dependencies()`, before instantiation — preserves PR #88's no-instantiation-on-missing-dep constraint. +- `InstrumentNotReadyWarning` removed. Skipped-due-to-config is silent; appears in `bootstrapper.skipped_instruments: list[tuple[type, str]]`. +- `InstrumentDependencyMissingWarning` retained and now fires ONLY for genuine deployment surprises (instrument is configured but its optional package is missing). +- One `logger.info(...)` summary at the end of `__init__` lists `configured=[...]` and `skipped=[...]`. Default Python logging suppresses INFO so silent by default. + +15 production files modified, 6 test files migrated, 2 docs updated. Bootstrapper-level `is_ready` methods (framework availability checks like "fastapi is not installed") are unchanged. + +Closes design `docs/superpowers/specs/2026-06-01-instrument-skip-rework-design.md`. + +## Backward compatibility +- `InstrumentNotReadyWarning` is publicly exported and is **hard-removed**. Users importing the name will get `ImportError`. 4 weeks old (PR #86); acceptable churn. +- `is_ready` instance method on instruments is removed. User code calling `instrument.is_ready()` migrates to `type(instrument).is_configured(config)`. Library-internal lifecycle; rare in user code. + +## Test plan +- [x] `just test` — full suite (130 expected; 129 prior + 1 new summary-log test). +- [x] `just lint` — clean. +- [x] Existing dep-missing tests (`test_*_bootstrapper_with_missing_instrument_dependency`) still pass: `InstrumentDependencyMissingWarning` continues to fire for the configured + dep-missing case. +- [ ] Reviewer: confirm the inline `__init__` flow correctly populates `skipped_instruments` only for `is_configured`-False instruments (not for dep-missing ones, which only get the warning). + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +--- + +## Self-Review + +Spec coverage check against `docs/superpowers/specs/2026-06-01-instrument-skip-rework-design.md`: + +| Spec section | Task | +|--------------|------| +| API change: `is_configured` classmethod replaces `is_ready` | Task 2 (base) + Task 3 (8 instruments) + Task 4 (3 framework overrides) | +| Bootstrapper flow reorder (is_configured → check_dependencies → instantiate) | Task 5, Step 2 | +| `skipped_instruments` attribute | Task 5, Step 2 | +| Summary log | Task 5, Step 2 | +| `is_ready` instance method removed | Task 2, Task 3, Task 4 | +| `not_ready_message` retained as class attribute | (preserved by inaction — no edits to this field across the migration) | +| `InstrumentNotReadyWarning` class removed | Task 8, Step 1 | +| `__init__.py` export removed | Task 8, Step 2 | +| Existing dep-missing tests continue to work | Task 10, Step 1 (verified by full test run) | +| New test: summary log via caplog | Task 7 | +| `test_free_bootstrap_logging_disabled` rewritten | Task 6, Step 6 | +| Instrument tests migrated (5 files from PR10) | Task 6, Steps 1-5 | +| Docs: `docs/introduction/configuration.md` | Task 9, Step 2 | +| Docs: `CLAUDE.md` | Task 9, Step 1 | +| Pre-flight grep | "Pre-flight grep verification" section + Task 4 Step 4 + Task 8 Step 3 + Task 10 Step 3 | + +All spec items have a corresponding task. No placeholders. + +Type consistency check: `is_configured(cls, bootstrap_config: ConfigT) -> bool` is the signature used throughout (Task 2 introduces, Task 3/4 apply). `skipped_instruments: list[tuple[type[BaseInstrument], str]]` is the type used in Task 5's class annotation and the test reads it as `{cls for cls, _ in bootstrapper.skipped_instruments}` (Task 6 Step 6). + +Cross-PR awareness: the `import_checker.is_prometheus_client_installed` conjunct dropped in Task 4 Step 2 is documented in the audit as DES-5; this PR opportunistically completes that cleanup since the migration is touching the same method body. + +Risk assessment: medium. The change is mechanical but cross-cutting (~20 files). The full test suite is the safety net. If a single instrument's `is_configured` migration is wrong (e.g., typo in field name), only that instrument's tests fail — no cascade. The bootstrapper flow change is the highest-risk single point; Task 5's smoke test catches the import case, full-suite run catches the behavioral case. diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index 64d12e3..b059822 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -15,23 +15,21 @@ try: import structlog - _structlog_available = True -except ImportError: - _structlog_available = False - + def _get_logger() -> typing.Any: # noqa: ANN401 + """Get a fresh structlog proxy each call. + + We deliberately avoid a module-level cached logger because structlog's + `cache_logger_on_first_use=True` (set by LoggingInstrument.bootstrap) memoizes the + BoundLogger and its processor chain on first use — making it impossible for + `structlog.testing.capture_logs()` to override the binding after the cache is set. + Returning a fresh proxy per call keeps the structlog pipeline reactive to config changes. + """ + return structlog.get_logger(__name__) -def _get_logger() -> typing.Any: # noqa: ANN401 - """Get a fresh logger instance each call. +except ImportError: - We deliberately avoid a module-level cached logger because structlog's - `cache_logger_on_first_use=True` (set by LoggingInstrument.bootstrap) memoizes the - BoundLogger and its processor chain on first use — making it impossible for - `structlog.testing.capture_logs()` to override the binding after the cache is set. - Returning a fresh proxy per call keeps the structlog pipeline reactive to config changes. - """ - if _structlog_available: - return structlog.get_logger(__name__) - return logging.getLogger(__name__) + def _get_logger() -> typing.Any: # noqa: ANN401 + return logging.getLogger(__name__) InstrumentT = typing.TypeVar("InstrumentT", bound=BaseInstrument) From 335848b2703cfdff47ddad57cbe1772f1c79c31a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 10:37:44 +0300 Subject: [PATCH 3/8] refactor: use stdlib logging in BaseBootstrapper; add build_summary() Replace the _get_logger() structlog indirection with a module-level logging.getLogger(__name__). The two log sites (INFO summary in __init__, WARNING teardown error) describe bootstrapper lifecycle, not application events, so stdlib logging is appropriate. Removes a global-structlog-state landmine introduced by the previous fresh-per-call workaround. Add BaseBootstrapper.build_summary() returning a multi-line human-readable description of configured + skipped instruments. __init__ uses it for the INFO summary; users can call it post-construction for debugging. Migrate the two test sites that used structlog.testing.capture_logs() to pytest's caplog fixture. --- lite_bootstrap/bootstrappers/base.py | 45 +++++++++++++--------------- tests/test_free_bootstrap.py | 42 +++++++++++++++++++------- 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index b059822..ba64891 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -12,24 +12,7 @@ from lite_bootstrap.types import ApplicationT -try: - import structlog - - def _get_logger() -> typing.Any: # noqa: ANN401 - """Get a fresh structlog proxy each call. - - We deliberately avoid a module-level cached logger because structlog's - `cache_logger_on_first_use=True` (set by LoggingInstrument.bootstrap) memoizes the - BoundLogger and its processor chain on first use — making it impossible for - `structlog.testing.capture_logs()` to override the binding after the cache is set. - Returning a fresh proxy per call keeps the structlog pipeline reactive to config changes. - """ - return structlog.get_logger(__name__) - -except ImportError: - - def _get_logger() -> typing.Any: # noqa: ANN401 - return logging.getLogger(__name__) +logger = logging.getLogger(__name__) InstrumentT = typing.TypeVar("InstrumentT", bound=BaseInstrument) @@ -41,6 +24,24 @@ class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): skipped_instruments: list[tuple[type[BaseInstrument], str]] bootstrap_config: BaseConfig + def build_summary(self) -> str: + """Return a multi-line human-readable summary of configured + skipped instruments. + + Useful for INFO-level diagnostic logging (called once by ``__init__``) and for + post-construction debugging (e.g. from a REPL or a health endpoint). + """ + lines = [f"{type(self).__name__}:", " configured:"] + if self.instruments: + lines.extend(f" - {type(i).__name__}" for i in self.instruments) + else: + lines.append(" (none)") + lines.append(" skipped:") + if self.skipped_instruments: + lines.extend(f" - {cls.__name__}: {reason}" for cls, reason in self.skipped_instruments) + else: + lines.append(" (none)") + return "\n".join(lines) + def __init__(self, bootstrap_config: BaseConfig) -> None: self.is_bootstrapped = False if not self.is_ready(): @@ -67,11 +68,7 @@ def __init__(self, bootstrap_config: BaseConfig) -> None: continue self.instruments.append(instrument_type(bootstrap_config=self.bootstrap_config)) - _get_logger().info( - f"{type(self).__name__}: " - f"configured={[type(i).__name__ for i in self.instruments]}, " - f"skipped={[(cls.__name__, reason) for cls, reason in self.skipped_instruments]}" - ) + logger.info(self.build_summary()) @property @abc.abstractmethod @@ -101,7 +98,7 @@ def teardown(self) -> None: one_instrument.teardown() except Exception as e: # noqa: BLE001, PERF203 name = type(one_instrument).__name__ - _get_logger().warning(f"Error tearing down {name}: {e}") + logger.warning("Error tearing down %s: %s", name, e) errors.append((name, e)) if errors: raise TeardownError(errors) from errors[0][1] diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index 0ce7197..62f9a90 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -1,8 +1,8 @@ +import logging from unittest.mock import MagicMock import pytest import structlog -from structlog.testing import capture_logs from lite_bootstrap import ( FreeBootstrapper, @@ -53,7 +53,7 @@ def test_free_bootstrap_logging_disabled() -> None: assert PyroscopeInstrument in skipped_classes -def test_teardown_error_isolation(free_bootstrapper_config: FreeConfig) -> None: +def test_teardown_error_isolation(free_bootstrapper_config: FreeConfig, caplog: pytest.LogCaptureFixture) -> None: bootstrapper = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) bootstrapper.bootstrap() @@ -63,13 +63,16 @@ def test_teardown_error_isolation(free_bootstrapper_config: FreeConfig) -> None: good = MagicMock() bootstrapper.instruments = [bad, good] - with capture_logs() as cap_logs, pytest.raises(TeardownError, match="boom") as excinfo: + with ( + caplog.at_level(logging.WARNING, logger="lite_bootstrap.bootstrappers.base"), + pytest.raises(TeardownError, match="boom") as excinfo, + ): bootstrapper.teardown() # Both instruments attempted teardown despite the error (LIFO: good first, bad second). good.teardown.assert_called_once() bad.teardown.assert_called_once() - assert any("boom" in entry.get("event", "") for entry in cap_logs) + assert any("boom" in r.message for r in caplog.records) assert excinfo.value.errors == [("MagicMock", excinfo.value.__cause__)] @@ -141,16 +144,33 @@ def test_bootstrap_is_idempotent(free_bootstrapper_config: FreeConfig) -> None: assert bootstrapper.is_bootstrapped -def test_free_bootstrap_emits_summary_log() -> None: - with capture_logs() as cap_logs: +def test_free_bootstrap_emits_summary_log(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.INFO, logger="lite_bootstrap.bootstrappers.base"): FreeBootstrapper( bootstrap_config=FreeConfig( sentry_dsn="https://testdsn@localhost/1", sentry_additional_params={"transport": SentryTestTransport()}, ), ) - summary_entries = [e for e in cap_logs if "FreeBootstrapper" in e.get("event", "")] - assert summary_entries, "expected a summary log entry mentioning FreeBootstrapper" - summary_event = summary_entries[-1]["event"] - assert "configured=" in summary_event - assert "skipped=" in summary_event + summary_records = [r for r in caplog.records if "FreeBootstrapper" in r.message] + assert summary_records, "expected a summary log entry mentioning FreeBootstrapper" + summary = summary_records[-1].message + assert "configured:" in summary + assert "skipped:" in summary + + +def test_build_summary_format() -> None: + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeConfig( + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + logging_enabled=False, + logging_buffer_capacity=0, + ), + ) + summary = bootstrapper.build_summary() + assert summary.startswith("FreeBootstrapper:") + assert " configured:" in summary + assert " skipped:" in summary + assert " - SentryInstrument" in summary + assert " - LoggingInstrument: logging_enabled is False" in summary From fa135d25a84458ff836c4b44d68abb4fe26bb561 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 10:50:15 +0300 Subject: [PATCH 4/8] test: lock in silent-skip contract for config-driven instrument skip Asserts that constructing a bootstrapper with instruments whose config indicates they should not run emits zero UserWarnings. Without this regression test, a future change that re-introduces warnings.warn to the is_configured=False branch would pass CI. --- tests/test_free_bootstrap.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index 62f9a90..8219938 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -1,4 +1,5 @@ import logging +import warnings from unittest.mock import MagicMock import pytest @@ -174,3 +175,15 @@ def test_build_summary_format() -> None: assert " skipped:" in summary assert " - SentryInstrument" in summary assert " - LoggingInstrument: logging_enabled is False" in summary + + +def test_config_skip_emits_no_warning() -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error") # any UserWarning becomes a test failure + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeConfig( + logging_enabled=False, + logging_buffer_capacity=0, + ), + ) + assert LoggingInstrument in {cls for cls, _ in bootstrapper.skipped_instruments} From 41d83bbf69de68a6f38d9defb80ecfe90e1e8992 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 10:53:59 +0300 Subject: [PATCH 5/8] docs: document build_summary() and remove _get_logger() rationale CLAUDE.md and configuration.md now describe the build_summary() method and the stdlib logging backend. The previous instrument-skip-rework plan gets a supersession note pointing at the new design. --- CLAUDE.md | 2 +- docs/introduction/configuration.md | 2 ++ docs/superpowers/plans/2026-06-01-instrument-skip-rework.md | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 54a1b43..28a8012 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ BaseBootstrapper (abc.ABC) ### Key design decisions - **Optional dependencies**: Each instrument checks for its optional package via `import_checker.py` (`importlib.util.find_spec`). Instruments are skipped silently if the package is absent. Optional packages are imported inside `if import_checker.is_X_installed:` blocks; static analyzers that don't model this guard will report spurious "possibly unbound" diagnostics — the project uses `ty` which handles the pattern correctly. -- **Instrument skip ordering**: `BaseBootstrapper.__init__` runs `instrument_type.is_configured(config)` first (silent skip if the user's config indicates the instrument shouldn't run — populates `bootstrapper.skipped_instruments: list[tuple[type, str]]`); then `check_dependencies()` (emits `InstrumentDependencyMissingWarning` only for configured-but-dep-missing — the genuine deployment surprise); then instantiates. One `logger.info` summary line at the end lists configured + skipped instruments. The bootstrappers/base.py logger is obtained fresh per call (`_get_logger()`) instead of cached at module level, so `structlog.testing.capture_logs()` can override processors at test time even after `LoggingInstrument.bootstrap()` sets `cache_logger_on_first_use=True`. +- **Instrument skip ordering**: `BaseBootstrapper.__init__` runs `instrument_type.is_configured(config)` first (silent skip if the user's config indicates the instrument shouldn't run — populates `bootstrapper.skipped_instruments: list[tuple[type, str]]`); then `check_dependencies()` (emits `InstrumentDependencyMissingWarning` only for configured-but-dep-missing — the genuine deployment surprise); then instantiates. One `logger.info` summary line at the end lists configured + skipped instruments via `BaseBootstrapper.build_summary()`; that method is also publicly callable for post-construction debugging. Uses stdlib `logging` so it composes cleanly with the user's logging setup and with pytest's `caplog`. - **Frozen configs, non-frozen instruments**: All `*Config` classes are `@dataclasses.dataclass(kw_only=True, frozen=True)`. All `*Instrument` classes lose `frozen=True` because two instruments (`LoggingInstrument`, `OpenTelemetryInstrument`) cache mutable runtime state (`_logger_factory`, `_tracer_provider`); Python's dataclass rules require the whole hierarchy to be non-frozen. `from_dict()` and `from_object()` filter unknown keys before constructing. - **`FastAPIConfig.application` uses an `UnsetType` sentinel**: shared in `lite_bootstrap/types.py` as `UnsetType` + `UNSET` (singleton). `FastAPIConfig.__post_init__` checks `isinstance(self.application, UnsetType)` and replaces with a constructed `FastAPI()` via `object.__setattr__` (config stays frozen for user-facing immutability). A one-line comment in `__post_init__` documents the freeze bypass. - **Instrument registry**: `BaseBootstrapper` holds a list of instrument instances; it calls `bootstrap()` on each in order and `teardown()` in reverse during shutdown. diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index 63a7a8c..e381dda 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -221,3 +221,5 @@ bootstrapper = FastAPIBootstrapper(bootstrap_config=config) for cls, reason in bootstrapper.skipped_instruments: print(f"{cls.__name__}: {reason}") ``` + +To get a human-readable view of the same information at any later point (e.g. for debugging from a REPL or a health endpoint), call `bootstrapper.build_summary()`. It returns the multi-line string that the INFO summary log emits — useful when log levels are filtered or when you want to render the bootstrapper state inline. diff --git a/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md b/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md index a907f42..6e428e2 100644 --- a/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md +++ b/docs/superpowers/plans/2026-06-01-instrument-skip-rework.md @@ -1,5 +1,7 @@ # Instrument Skip Rework Implementation Plan +> **Note (2026-06-02):** the `_get_logger()` fresh-per-call decision documented below was revised by `docs/superpowers/specs/2026-06-02-stdlib-logging-and-build-summary-design.md`. The summary-log goal is unchanged; the implementation switched to stdlib `logging` with a public `build_summary()` method. + > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Replace `InstrumentNotReadyWarning` with a pre-instantiation `is_configured` classmethod check. Skipped-due-to-config becomes silent at the warning level; structured `bootstrapper.skipped_instruments` introspection + one INFO summary log provide diagnostic visibility. `InstrumentDependencyMissingWarning` continues to fire for the genuine "configured but dependency missing" deployment-surprise case. From c14c45561c66967e4299335c07eb7b205e12cb9c Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 17:43:16 +0300 Subject: [PATCH 6/8] refactor: tighten build_summary() docstring and summary-log test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a one-liner to build_summary()'s docstring noting that calling it before __init__ completes raises AttributeError — important context for anyone wiring it to a health endpoint that might be invoked during a bootstrapper construction failure path. Tighten test_free_bootstrap_emits_summary_log to assert on " configured:" and " skipped:" (with the two-space indent) so it matches the precision of test_build_summary_format. --- lite_bootstrap/bootstrappers/base.py | 2 ++ tests/test_free_bootstrap.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index ba64891..06e74b1 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -29,6 +29,8 @@ def build_summary(self) -> str: Useful for INFO-level diagnostic logging (called once by ``__init__``) and for post-construction debugging (e.g. from a REPL or a health endpoint). + + Raises ``AttributeError`` if called before ``__init__`` completes. """ lines = [f"{type(self).__name__}:", " configured:"] if self.instruments: diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index 8219938..f1881ff 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -156,8 +156,8 @@ def test_free_bootstrap_emits_summary_log(caplog: pytest.LogCaptureFixture) -> N summary_records = [r for r in caplog.records if "FreeBootstrapper" in r.message] assert summary_records, "expected a summary log entry mentioning FreeBootstrapper" summary = summary_records[-1].message - assert "configured:" in summary - assert "skipped:" in summary + assert " configured:" in summary + assert " skipped:" in summary def test_build_summary_format() -> None: From 4f051d6eb89a49a2271e35da81045ad57a69a2e8 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 17:48:23 +0300 Subject: [PATCH 7/8] test: cover build_summary() empty branches and silence faststream warning leak Add test_build_summary_renders_none_for_empty_sections to exercise the (none) fallback for empty instruments and skipped_instruments lists, closing the 1-line gap reported by coverage on bootstrappers/base.py:44. Wrap the opentelemetry-missing construction in test_faststream_bootstrap_without_opentelemetry with pytest.warns so the expected InstrumentDependencyMissingWarning is captured at the call site instead of leaking into the test session output. --- tests/test_faststream_bootstrap.py | 3 ++- tests/test_free_bootstrap.py | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index e81edec..46f1463 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -164,7 +164,8 @@ def test_faststream_bootstrap_without_opentelemetry(broker: RedisBroker) -> None "opentelemetry", ["lite_bootstrap.bootstrappers.faststream_bootstrapper"], ): - bootstrapper = FastStreamBootstrapper(bootstrap_config=bootstrap_config) + with pytest.warns(UserWarning, match="opentelemetry"): + bootstrapper = FastStreamBootstrapper(bootstrap_config=bootstrap_config) bootstrapper.bootstrap() diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index f1881ff..4c3788d 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -187,3 +187,16 @@ def test_config_skip_emits_no_warning() -> None: ), ) assert LoggingInstrument in {cls for cls, _ in bootstrapper.skipped_instruments} + + +def test_build_summary_renders_none_for_empty_sections() -> None: + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeConfig( + sentry_dsn="https://testdsn@localhost/1", + sentry_additional_params={"transport": SentryTestTransport()}, + ), + ) + bootstrapper.instruments = [] + bootstrapper.skipped_instruments = [] + summary = bootstrapper.build_summary() + assert summary == "FreeBootstrapper:\n configured:\n (none)\n skipped:\n (none)" From 86b43ef4b0af5c7df309ad7756565b31dfa6a61b Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 2 Jun 2026 17:57:11 +0300 Subject: [PATCH 8/8] refactor: guard summary log with isEnabledFor; revert overspecified docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap the __init__ summary log in `if logger.isEnabledFor(logging.INFO)` so build_summary() doesn't run when INFO is filtered. Bootstrap fires once per service so the savings are negligible, but the guard is the conventional stdlib pattern and matters more if a subclass overrides build_summary() with something expensive. Drop the "Raises AttributeError if called before __init__ completes" line from the docstring — it documented an implementation detail rather than a contract. The "post-construction debugging" phrasing in the prior sentence already conveys the constraint. --- lite_bootstrap/bootstrappers/base.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index 06e74b1..f00fcc1 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -29,8 +29,6 @@ def build_summary(self) -> str: Useful for INFO-level diagnostic logging (called once by ``__init__``) and for post-construction debugging (e.g. from a REPL or a health endpoint). - - Raises ``AttributeError`` if called before ``__init__`` completes. """ lines = [f"{type(self).__name__}:", " configured:"] if self.instruments: @@ -70,7 +68,8 @@ def __init__(self, bootstrap_config: BaseConfig) -> None: continue self.instruments.append(instrument_type(bootstrap_config=self.bootstrap_config)) - logger.info(self.build_summary()) + if logger.isEnabledFor(logging.INFO): + logger.info(self.build_summary()) @property @abc.abstractmethod