Automatically attach to non-propagating loggers#10303
Closed
haxtibal wants to merge 1 commit intopytest-dev:mainfrom
Closed
Automatically attach to non-propagating loggers#10303haxtibal wants to merge 1 commit intopytest-dev:mainfrom
haxtibal wants to merge 1 commit intopytest-dev:mainfrom
Conversation
Currently, pytest attaches its log handlers only to the root logger, and relies on log propagation to catch logs from loggers deeper in the hierarchy. In consequence we miss logs if one of those child loggers has logger.propagate set to False by user code. This affects caplog, report logging and live logging. This commit showcases a possible fix, where we attach not only to the root logger, but also to non-propagating loggers as they occur. To do so we patch the standard libraries logging module with our own descriptor that intercepts assignments to logger.propagate. This enables us to inject and remove our pytest handlers right on time, even during executing test calls, with neglible performance impact. Attempt to fix pytest-dev#3697.
haxtibal
force-pushed
the
bugfix/attach_non_propagating_loggers
branch
from
e0afbee to
13a1f37
Compare
bluetech
pushed a commit
to bluetech/pytest
that referenced
this pull request
Apr 10, 2026
…f non-propagating loggers Demonstrate the bug/missing functionality of reporting logs emitted by non-propagating loggers (i.e. loggers which don't reach the root logger, where we currently attach our handler). Refs pytest-dev#3697 [ran: extracted from pytest-dev#10303 and changed to use HookRecorder]
Member
|
Thanks for the PR. I think the monkeypatching interceptor approach is a bit too complex and hacky for pytest. It has advantage in low overhead and being able to handle test-time changes to I adopted your test case and propose a simpler approach (#14375), which is less efficient and doesn't handle the dynamic change case, but I think is acceptable. |
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
2 participants
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.
Currently, pytest attaches its log handlers only to the root logger, and relies on log propagation to catch logs from loggers deeper in the hierarchy. In consequence we miss logs if one of those child loggers has
logger.propagateset toFalseby user code. This affects caplog, report logging and live logging.This commit showcases a possible fix, where we attach not only to the root logger, but also to non-propagating loggers as they occur. To do so we patch the standard libraries logging module with our own descriptor that intercepts assignments to
logger.propagate. This enables us to inject and remove our pytest handlers right on time, even during executing test calls, with negligible performance impact.Attempts to fix #3697.
PS: I'm aware the PR needs extra caution regarding thread safety and compatibility. Before considering such safe guards, I'd like to hear opinions on the approach in general. Feel free to close the PR right away if you think patching the standard library is against all rules.