fix(ci_visibility): filter ddtrace logs at closed stream handlers during pytest teardown - #20101
Conversation
Codeowners resolved asResolved from the full PR diff against No remaining files require a CODEOWNERS review. |
Circular import analysis
|
Dependency direction analysis
|
🎉 All green!🧪 All tests passed 🔗 Commit SHA: ccb707a | Docs | View more details | Give us feedback! |
gnufede
changed the title
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The setting runs before the early enable guard. Thus, shutdown logs cannot reach a closed root stream in either plug-in path.
🤖 Datadog Autotest · Commit e662d76 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e662d76679
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…knowledge regression in release note
<!-- dd-meta
{"pullId":"cea9b0fd-5851-4c4f-b4b0-18a30ddd7584","source":"chat","resourceId":"c0096dbc-5d7d-4dc0-9dac-95643f43b151","workflowId":"f964c044-b498-4df8-a367-1861336af4f1","codeChangeId":"f964c044-b498-4df8-a367-1861336af4f1","sourceType":"slack"}
-->
## Description
Fix the closed-stream shutdown error in #16712 without the whole-process
loss of tracer log delivery exposed by the compatibility probes for
#20101.
- Replace the unconditional ddtrace propagation cutoff with per-handler
filters installed during pytest teardown. Only ddtrace records destined
for already-closed ordinary StreamHandlers are skipped; healthy root and
direct handlers continue receiving records.
- Rescan at session finish, unconfigure, and final cleanup, including
when instrumentation is disabled. Preserve propagation settings, handler
ownership, formatting, application records, and the global tracer's
lifetime.
- Exclude FileHandler and custom handler subclasses, preserve existing
filters, avoid duplicate filters across repeated pytest sessions, and
allow delivery again if a handler's stream is replaced.
- Retain the 29 subprocess compatibility/control cases added earlier,
replace the implementation-specific propagation assertion with
preservation tests, and add shutdown/root-file delivery, late
configuration, collection-error, repeated pytest.main(), and filter unit
tests.
- Update the existing customer-facing release note to describe preserved
diagnostic delivery.
## Testing
Python 3.13.13:
- scripts/run-tests --venv 3dc4202 -- -n 0 -k
'test_pytest_log_propagation or test_logging or
test_pytest_log_correlation or test_plugin': 181 passed on pytest 7.4.4.
- scripts/run-tests -s --venv 1b6f43f -- -n 0 -k
'test_pytest_log_propagation or test_logging or
test_pytest_log_correlation or test_plugin': 181 passed on pytest 8.4.2
in the final restored state.
- All 18 previously failing delivery probes now pass, alongside the
original shutdown regressions and
application/correlation/submission-handler controls.
- Negative control: temporarily disabled the closed-stream filter; all
three targeted fd-capture shutdown tests failed with ValueError: I/O
operation on closed file. Restored the filter before the final passing
run.
- Python formatting and Ruff checks passed through scripts/lint for all
four edited Python files; git diff --check passed.
- Scoped type checking passed for the logging helper and subprocess test
module. Wider typing reports existing diagnostics in the plugin,
imported dependencies, and an unchanged unreachable statement in
test_logging.py. Full lint remains limited by unavailable auxiliary
tools, including cython-lint.
## Risks
The protection is deliberately limited to existing standard stream
handlers at pytest teardown; custom/file handler behavior is unchanged.
Handlers installed after pytest returns remain the caller's
responsibility. The filter checks whether a stream is already closed at
dispatch, not arbitrary concurrent closure between filtering and
writing. The submission probe uses the real handler with a mock writer;
it does not validate remote intake delivery.
## Additional Notes
No global logging monkeypatch, no global exception suppression, and no
early shutdown of the global tracer. No commit, push, or remote PR
update performed.
---
PR by Bits - [View session in
Datadog](https://app.datadoghq.com/code/c0096dbc-5d7d-4dc0-9dac-95643f43b151)
Comment @DataDog to request changes
---------
Co-authored-by: datadog-bits <263423550+datadog-bits@users.noreply.github.com>
Co-authored-by: gnufede <412857+gnufede@users.noreply.github.com>
gnufede
changed the title
There was a problem hiding this comment.
More details
The filter blocks only ddtrace records when a standard stream is closed. Open streams and non-ddtrace records keep their current behavior.
🤖 Datadog Autotest · Commit 9cb2b3f · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
/merge -f --reason "unrelated system tests failures" |
|
View all feedbacks in Devflow UI.
The expected merge time in
Warning This change was merged without running any pre merge CI checks Reason: unrelated system tests failures |
3 participants
Description
Fixes #16712.
When the new pytest plugin is enabled (default since v4.5.0), ddtrace log records
emitted at interpreter shutdown via
Tracer._atexitpropagate to the root logger.If a user has configured a custom root logger with
logging.config.dictConfig(and
disable_existing_loggers: False) that installs aStreamHandleronsys.stdout, pytest closes that stream during teardown before theatexithandler fires. The handler then raises
ValueError: I/O operation on closed file,producing a
--- Logging error ---traceback at the end of the test session.The fix installs a per-handler filter on plain
logging.StreamHandlerinstancesduring pytest teardown:
handlers keep receiving records, including file output and log forwarding.
Propagation settings, handler ownership, formatting, and the global tracer's
lifetime are all preserved.
after capture cleanup),
pytest_sessionfinish, andpytest_unconfigure, sohandlers installed by later teardown hooks are covered.
FileHandlerand customStreamHandlersubclasses are excluded (file handlerscan reopen their streams; custom handlers own their error handling). Existing
filters are preserved, duplicate filters are avoided across repeated pytest
sessions, and delivery resumes if a handler's stream is replaced with an open
one.
Testing
Added subprocess regression tests in
tests/testing/internal/pytest/test_pytest_log_propagation.py(the bug only manifests at interpreter shutdown, which does not fire in
inline_run) and unit tests for the filter and scanning helper intests/testing/internal/test_logging.py.Risks
The protection is limited to existing standard stream handlers at pytest teardown;
custom/file handler behavior is unchanged. Handlers installed after pytest returns
remain the caller's responsibility. The filter checks whether a stream is already
closed at dispatch, not arbitrary concurrent closure between filtering and writing.