fix: warn when bootstrap loses the set_tracer_provider race - #264
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #227.
OpenTelemetryInstrument.bootstrap()builds a provider and callsset_tracer_provider, which theSDK enforces as set-once per process. When the application installed its own provider first the
call is refused, and the SDK's complaint goes to a logger
_silence_otel_loggers()disabled twolines earlier. Tracing still works, because every bootstrapper reads
get_tracer_provider()whenit wires middleware, so the only casualty is invisible: the provider we built keeps the configured
exporter, sampler and resource, nothing will ever feed it a span, and its
BatchSpanProcessorworker thread and collector connection stay alive until
teardown().This makes that audible. It is a diagnostic, not a behaviour change.
UserWarning, matching the insecure-endpoint warning inOpenTelemetryConfig.__post_init__.Deliberately not
InstrumentSkippedWarning: the instrument did bootstrap and tracing works, andpytest_configureescalates that class to an error, which would turn a diagnostic into a failure.Still not proposed, per the issue: skipping provider construction when one already exists, which
would silently discard an explicit
opentelemetry_endpoint.The test suite had to change first
The suite never reset the process-global provider, so the first test to bootstrap owned it and
every later test silently lost the race. Measured with the warning in and no test changes:
70 of 331 tests emit it, across six files, and one fails outright
(
test_swagger_warning_points_at_the_bootstrap_call_site, whosewarning_source_files(...) == [__file__]suddenly sees two warnings).An autouse fixture in
tests/conftest.pynow clears the global after each test, so each one startswhere a fresh process does. That drops the warners from 71 invocations to 2 and makes the suite
order-independent, which it was not before: 333 pass under
-p no:randomlyand under random order.It reaches into
opentelemetry.trace._TRACER_PROVIDERand_TRACER_PROVIDER_SET_ONCEbecause theSDK offers no public reset. Both names are spelled identically at the 1.28 floor and at 1.44, and
if upstream ever moves them the fixture raises
AttributeErrorrather than passing silently.The two remaining warners are both correct, and each is now deliberate:
test_two_free_bootstrappers_both_bootstrapgenuinely bootstraps twice, so it asserts thewarning with
pytest.warns.test_pyroscope_otel_adds_span_processor_when_configuredwas a false positive: it patchedset_tracer_providerout, so the global was never set and the identity check could not hold.The patch existed to stop that test polluting the global, which the fixture now handles properly,
so it is gone.
Tests
Written failing first.
test_bootstrap_warns_when_a_tracer_provider_is_already_installedpre-installs a provider, then pins the exact message and, through
warning_source_files, that thewarning is attributed to the caller's frame rather than to lite_bootstrap's own
bootstrap().test_bootstrap_is_silent_when_it_installs_the_tracer_providercovers the other branch, which the100% gate requires.
Verified
eof-fixer --check,ruff format --check,ruff check --no-fix,ty checkclean.333 passed in both orders. Coverage 100.00%, gate satisfied.
The floors legs run
scripts/floor_smoke.py, not pytest, so the conftest fixture never executesthere; what does is the library change, and
get_tracer_provideris public and exported at the1.28 API floor. Ran the
freetarget locally at Python 3.10 with opentelemetry-api and -sdk pinnedto 1.28.0:
floor smoke OK: free on 3.10.21, with the span emitted and the new branch quiet.