From c6e7058d4c8d5919ade7814303ce17a962ef7fbd Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 12 Sep 2026 12:03:01 +0300 Subject: [PATCH] fix: attribute instrument bootstrap() warnings to the user's call site (#202) The three warnings raised while an instrument bootstraps carried a literal `stacklevel`, which encodes a frame depth that is not constant. Measured on `main` before the change: opentelemetry_instrument.py -> bootstrappers/fastapi_bootstrapper.py:118 fastapi_bootstrapper.py -> bootstrappers/base.py:137 Both are lite-bootstrap's own source. They differ by one frame because `FastAPISwaggerInstrument.bootstrap()` is called straight from the loop in `BaseBootstrapper.bootstrap()` while `FastAPIOpenTelemetryInstrument.bootstrap()` calls `super().bootstrap()` first, so no single literal can be right for both, and any instrument that grows or loses a `super()` call shifts its own attribution silently. Route all three through `warn_at_caller`, which walks out to the first frame outside `lite_bootstrap`. On this path that frame is the user's `bootstrap()` call, which is the line they can act on: the warnings say a dependency is missing or that `swagger_path` is being ignored, and the fix for both is in the config they passed, not inside the instrument. The helper needed one addition, a `category` parameter, since the OpenTelemetry warnings are `InstrumentDependencyMissingWarning` rather than plain `UserWarning`. Two invariant tests pin both depths through the bootstrapper, one per shape; either alone would pass against a literal. The existing OpenTelemetry test covers the instrument called on its own, where the walk already stopped at the right frame, and its "what breaks it" paragraph is updated to name the helper. Out of scope, per the issue: the two warnings reached from `BaseBootstrapper.__init__` are correct today and keep their literal `stacklevel`, and `helpers/fastapi_helpers.py` warns from inside a request handler where no user frame exists at all. Closes #202 --- AGENTS.md | 16 ++++-- .../bootstrappers/fastapi_bootstrapper.py | 6 +-- lite_bootstrap/helpers/warn.py | 18 ++++--- .../instruments/opentelemetry_instrument.py | 8 +-- tests/conftest.py | 9 ++++ .../test_opentelemetry_instrument.py | 15 +++--- tests/test_fastapi_bootstrap.py | 49 +++++++++++++++++-- 7 files changed, 88 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8423534..0baac8f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,10 +56,18 @@ Four rules that are not visible in the code that follows them: re-export both from `__init__.py` if the old name was exported. It is a class assignment, not a subclass, so `isinstance` still holds. `FreeBootstrapperConfig`, `OpentelemetryConfig` and `IGNORED_STRUCTLOG_ATTRIBUTES` exist for this reason. -- **A warning raised while a config is being built goes through `warn_at_caller`.** No literal - `stacklevel=` reaches the user from a `__post_init__`: the depth is that config's MRO chain plus - the `__init__` `dataclasses` generates. Warnings raised outside config construction keep their - literal `stacklevel`: their target is the bootstrapper's caller, not the user. +- **A warning raised while a config is built or an instrument is bootstrapped goes through + `warn_at_caller`.** No literal `stacklevel=` reaches the user on either path: from a + `__post_init__` the depth is that config's MRO chain plus the `__init__` `dataclasses` generates, + and from an instrument's `bootstrap()` it is one frame deeper whenever that instrument calls + `super().bootstrap()` first. `warn_at_caller` walks out to the first frame outside + `lite_bootstrap` instead, which on the bootstrap path is the user's `bootstrap()` call. Three + literal `stacklevel=` sites survive, all outside those two paths and all deliberate: the + dep-missing warning in `BaseBootstrapper._select_instruments` (`stacklevel=4`) and the + double-attach warning in `_attach_teardown_once` (`stacklevel=3`, one frame shallower because + the subclass `__init__` calls it directly rather than through `BaseBootstrapper.__init__`), both + a fixed depth below the user and scoped out by #202; and `helpers/fastapi_helpers.py`, which + warns while serving a request, with no user frame anywhere on the stack. - **Sentinels on a user's app get a `_lite_bootstrap_` prefix.** Set a direct attribute on the app object; never squat in a framework namespace like Starlette's `application.state`. Read it with `getattr(target, name, default)` (no SLF violation); write it with `# noqa: SLF001`. diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index bcd7ac9..78c6ca3 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -1,7 +1,6 @@ import contextlib import dataclasses import typing -import warnings from lite_bootstrap import import_checker from lite_bootstrap.bootstrappers.base import BaseBootstrapper @@ -158,10 +157,7 @@ def bootstrap(self) -> None: config = self.bootstrap_config application = config.app if config.swagger_path != application.docs_url: - warnings.warn( - f"swagger_path differs from docs_url, {application.docs_url} will be used for docs path", - stacklevel=2, - ) + warn_at_caller(f"swagger_path differs from docs_url, {application.docs_url} will be used for docs path") if config.swagger_offline_docs: enable_offline_docs(application, static_path=config.swagger_static_path) diff --git a/lite_bootstrap/helpers/warn.py b/lite_bootstrap/helpers/warn.py index 211b76c..4b15394 100644 --- a/lite_bootstrap/helpers/warn.py +++ b/lite_bootstrap/helpers/warn.py @@ -5,11 +5,11 @@ _DATACLASS_GENERATED_INIT: typing.Final = "" -_CONSTRUCTION_MODULES: typing.Final = frozenset({__name__.split(".")[0], "dataclasses"}) +_INTERNAL_MODULES: typing.Final = frozenset({__name__.split(".")[0], "dataclasses"}) def _is_internal_frame(frame: types.FrameType) -> bool: - """Return True while the frame is still part of building a config rather than asking for one. + """Return True while the frame still belongs to lite-bootstrap rather than to its user. `dataclasses` is in the set because `replace()` calls the constructor itself. The `__init__` it generates reports its file as ``, in the defining class's module rather than this one. @@ -17,18 +17,20 @@ def _is_internal_frame(frame: types.FrameType) -> bool: if frame.f_code.co_name == "__init__" and frame.f_code.co_filename == _DATACLASS_GENERATED_INIT: return True module_name: str = frame.f_globals.get("__name__", "") - return module_name.split(".", maxsplit=1)[0] in _CONSTRUCTION_MODULES + return module_name.split(".", maxsplit=1)[0] in _INTERNAL_MODULES -def warn_at_caller(message: str) -> None: - """Warn at the nearest frame outside the machinery that builds a config. +def warn_at_caller(message: str, category: type[Warning] = UserWarning) -> None: + """Warn at the nearest frame outside lite-bootstrap's own machinery. - Config validation runs inside a `__post_init__` cascade whose depth is the length of that - config's MRO chain, so no literal `stacklevel` can name the frame that built the config. + Both call paths that reach here have a depth no literal `stacklevel` can name: a config's + `__post_init__` cascade is as deep as that config's MRO chain plus the `__init__` + `dataclasses` generates, and an instrument's `bootstrap()` sits one frame deeper when it + calls `super().bootstrap()` first. """ stacklevel = 1 frame: types.FrameType | None = inspect.currentframe() while frame is not None and _is_internal_frame(frame): stacklevel += 1 frame = frame.f_back - warnings.warn(message, stacklevel=stacklevel) + warnings.warn(message, category=category, stacklevel=stacklevel) diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index 1e83f8a..d30d03f 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -3,7 +3,6 @@ import os import typing import urllib.parse -import warnings from lite_bootstrap import import_checker from lite_bootstrap.exceptions import InstrumentDependencyMissingWarning, collect_teardown_errors @@ -195,14 +194,12 @@ def _build_span_exporter(self) -> "SpanExporter | None": Only call this once opentelemetry_endpoint is set: both warnings claim that it is. """ config = self.bootstrap_config - # stacklevel counts this frame as well as bootstrap()'s, to land on bootstrap()'s caller. if config.opentelemetry_exporter_protocol == "grpc": if not import_checker.is_otlp_grpc_exporter_installed: - warnings.warn( + warn_at_caller( "opentelemetry_endpoint is set but the gRPC OTLP exporter is not installed; " "spans will not be exported. Install lite-bootstrap[otl].", category=InstrumentDependencyMissingWarning, - stacklevel=3, ) return None return OTLPGrpcSpanExporter( @@ -210,11 +207,10 @@ def _build_span_exporter(self) -> "SpanExporter | None": insecure=config.opentelemetry_insecure, ) if not import_checker.is_otlp_http_exporter_installed: - warnings.warn( + warn_at_caller( "opentelemetry_endpoint is set but the HTTP OTLP exporter is not installed; " "spans will not be exported. Install lite-bootstrap[otl-http].", category=InstrumentDependencyMissingWarning, - stacklevel=3, ) return None return OTLPHttpSpanExporter(endpoint=config.opentelemetry_endpoint) diff --git a/tests/conftest.py b/tests/conftest.py index 26c039a..d798f14 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -60,6 +60,15 @@ def logging_mock() -> LoggingMock: return LoggingMock() +def warning_source_files(caught: typing.Iterable[warnings.WarningMessage], category: type[Warning]) -> list[str]: + """Return the file each recorded warning of ``category`` was attributed to, in the order raised. + + Attribution is what the stacklevel rules decide, so a test that pins it compares whole lists: + a warning raised from the wrong frame and one raised twice are different bugs. + """ + return [one_warning.filename for one_warning in caught if issubclass(one_warning.category, category)] + + @contextlib.contextmanager def emulate_package_missing(package_name: str) -> typing.Iterator[None]: old_module = sys.modules[package_name] diff --git a/tests/instruments/test_opentelemetry_instrument.py b/tests/instruments/test_opentelemetry_instrument.py index d164620..0ab0c78 100644 --- a/tests/instruments/test_opentelemetry_instrument.py +++ b/tests/instruments/test_opentelemetry_instrument.py @@ -15,7 +15,7 @@ OpenTelemetryConfig, OpenTelemetryInstrument, ) -from tests.conftest import CustomInstrumentor, emulate_package_missing_with_module_reload +from tests.conftest import CustomInstrumentor, emulate_package_missing_with_module_reload, warning_source_files def test_opentelemetry_instrument() -> None: @@ -306,10 +306,12 @@ def test_missing_exporter_warning_points_at_the_caller_of_bootstrap( ) -> None: """INVARIANT: the missing-exporter warning is attributed to the frame that called bootstrap(). - The stacklevel is a literal, so it counts frames that only exist by convention: any helper - extracted out of bootstrap() moves the warning one frame deeper, onto lite_bootstrap's own - source, and nothing but this test notices. Both transports are pinned because each warns - from its own branch and a refactor can reshape one without the other. + Replacing warn_at_caller with a literal stacklevel breaks it: the literal counts frames that + only exist by convention, so any helper extracted out of bootstrap() moves the warning one + frame deeper, onto lite_bootstrap's own source, and nothing but this test notices. Both + transports are pinned because each warns from its own branch and a refactor can reshape one + without the other. This covers the instrument called on its own; the bootstrapper path, where + the frame to skip past is lite_bootstrap's own, is pinned in tests/test_fastapi_bootstrap.py. """ instrument = OpenTelemetryInstrument( bootstrap_config=OpenTelemetryConfig(opentelemetry_endpoint=endpoint, opentelemetry_exporter_protocol=protocol) @@ -324,5 +326,4 @@ def test_missing_exporter_warning_points_at_the_caller_of_bootstrap( finally: instrument.teardown() - matching = [w for w in caught if issubclass(w.category, InstrumentDependencyMissingWarning)] - assert [w.filename for w in matching] == [__file__] + assert warning_source_files(caught, InstrumentDependencyMissingWarning) == [__file__] diff --git a/tests/test_fastapi_bootstrap.py b/tests/test_fastapi_bootstrap.py index c3a0d8a..7dd78ed 100644 --- a/tests/test_fastapi_bootstrap.py +++ b/tests/test_fastapi_bootstrap.py @@ -1,6 +1,7 @@ import dataclasses import logging import warnings +from unittest.mock import patch import fastapi import pytest @@ -8,10 +9,10 @@ from starlette import status from starlette.testclient import TestClient -from lite_bootstrap import FastAPIBootstrapper, FastAPIConfig -from lite_bootstrap.exceptions import ConfigurationError +from lite_bootstrap import FastAPIBootstrapper, FastAPIConfig, import_checker +from lite_bootstrap.exceptions import ConfigurationError, InstrumentDependencyMissingWarning from lite_bootstrap.types import UNSET -from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing +from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing, warning_source_files logger = structlog.getLogger(__name__) @@ -189,3 +190,45 @@ def test_second_fastapi_bootstrapper_bootstrap_raises(fastapi_config: FastAPICon ) finally: first.teardown() + + +def test_swagger_warning_points_at_the_bootstrap_call_site(fastapi_config: FastAPIConfig) -> None: + """INVARIANT: a warning raised while an instrument bootstraps names the user's bootstrap() line. + + FastAPISwaggerInstrument.bootstrap() is called straight from the loop in + BaseBootstrapper.bootstrap(), one frame shallower than an instrument that calls + super().bootstrap() first. A literal stacklevel pins one of those two depths and misses the + other, naming lite_bootstrap's own source instead. This test and the OpenTelemetry one below + are a pair: each covers one depth, and either passing alone proves nothing. + """ + new_config = dataclasses.replace(fastapi_config, application=fastapi.FastAPI(docs_url="/custom-docs/")) + bootstrapper = FastAPIBootstrapper(bootstrap_config=new_config) + try: + with pytest.warns(UserWarning, match="swagger_path differs from docs_url") as caught: + bootstrapper.bootstrap() + finally: + bootstrapper.teardown() + + assert warning_source_files(caught, UserWarning) == [__file__] + + +def test_missing_exporter_warning_points_at_the_bootstrap_call_site(fastapi_config: FastAPIConfig) -> None: + """INVARIANT: the deeper super().bootstrap() shape names the user's bootstrap() line as well. + + FastAPIOpenTelemetryInstrument.bootstrap() calls super().bootstrap() before the warning is + raised, so the user's frame sits one deeper than for the swagger instrument above. Any + instrument that grows or loses a super() call shifts that depth again; only a rule that finds + the first frame outside lite_bootstrap survives it. + """ + new_config = dataclasses.replace(fastapi_config, opentelemetry_endpoint="localhost:4317") + bootstrapper = FastAPIBootstrapper(bootstrap_config=new_config) + try: + with ( + patch.object(import_checker, "is_otlp_grpc_exporter_installed", False), + pytest.warns(InstrumentDependencyMissingWarning) as caught, + ): + bootstrapper.bootstrap() + finally: + bootstrapper.teardown() + + assert warning_source_files(caught, InstrumentDependencyMissingWarning) == [__file__]