Skip to content

Commit b769e1e

Browse files
authored
fix: resolve asyncio event loop bug when gevent is installed (#11904)
## Description Reverts a change ([4e31278](4e31278)) that introduced a regression in asyncio event loops when gevent is installed. This issue cannot be reproduced on macOS; it was detected on Ubuntu 24. ## Background - In ddtrace v2.11.0, the ddtrace-py [introduced support](dc000ae) for crash tracking, and the crash tracker is started via ddtrace-run or by importing ddtrace.auto ([here](https://github.com/DataDog/dd-trace-py/blob/main/ddtrace/auto.py)). - Before the crash tracker is started, it [reads the agent URL](https://github.com/DataDog/dd-trace-py/blob/v2.11.0/ddtrace/internal/core/crashtracking.py#L31) using the [ensure_binary](https://github.com/DataDog/dd-trace-py/blob/a58f139e24d78a66468dbc7f67ec42c2bdfad8ee/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx#L50) function. - The ensure_binary function imports unittest.mock, which imports [asyncio](https://github.com/DataDog/dd-trace-py/blob/v2.11.0/ddtrace/internal/compat.py#L72) as a side effect. - After crash tracking is started, ddtrace unloads all modules that were imported during the setup of ddtrace features ([here](https://github.com/DataDog/dd-trace-py/blob/main/ddtrace/bootstrap/sitecustomize.py#L120C5-L120C27)). This includes asyncio. - At this point, asyncio has been added to and then removed from sys.modules. - When asyncio is imported in a user's application event loops fail to be set. This is seen in the script below (the script was run on Ubuntu 24 with gevent>=24) ### Script ``` import asyncio import time loop = asyncio.get_event_loop() loop_id = id(loop) new_loop = asyncio.new_event_loop() new_loop_id = id(new_loop) print(f"old: {loop_id} new: {new_loop_id}") asyncio.set_event_loop(new_loop) check = asyncio.get_event_loop() check_id = id(check) print(f"check: {id(check)}") while check_id != new_loop_id: print("MISMATCH") print(f"check: {id(check)}") time.sleep(1) check = asyncio.get_event_loop() check_id = id(check) ``` ### Output ``` MISMATCH check: 131621237174144 MISMATCH check: 131621237174144 MISMATCH check: 131621237174144 MISMATCH check: 131621237174144 MISMATCH check: 131621237174144 MISMATCH ``` ## Next steps Investigate module unloading and asyncio incompatibility ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent d1dcc35 commit b769e1e

File tree

4 files changed

+17
-133
lines changed

4 files changed

+17
-133
lines changed

.github/workflows/test_frameworks.yml

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -111,68 +111,6 @@ jobs:
111111
if: needs.needs-run.outputs.outcome == 'success'
112112
run: cat debugger-expl.txt
113113

114-
sanic-testsuite:
115-
strategy:
116-
matrix:
117-
include:
118-
# TODO: profiling fails with a timeout error
119-
#- suffix: Profiling
120-
# profiling: 1
121-
# iast: 0
122-
# appsec: 0
123-
- suffix: IAST
124-
profiling: 0
125-
iast: 1
126-
appsec: 0
127-
- suffix: APPSEC
128-
profiling: 0
129-
iast: 0
130-
appsec: 1
131-
- suffix: Tracer only
132-
profiling: 0
133-
iast: 0
134-
appsec: 0
135-
name: Sanic 24.6 (with ${{ matrix.suffix }})
136-
runs-on: ubuntu-20.04
137-
needs: needs-run
138-
timeout-minutes: 15
139-
env:
140-
DD_PROFILING_ENABLED: ${{ matrix.profiling }}
141-
DD_IAST_ENABLED: ${{ matrix.iast }}
142-
DD_APPSEC_ENABLED: ${{ matrix.appsec }}
143-
DD_TESTING_RAISE: true
144-
CMAKE_BUILD_PARALLEL_LEVEL: 12
145-
DD_DEBUGGER_EXPL_OUTPUT_FILE: debugger-expl.txt
146-
defaults:
147-
run:
148-
working-directory: sanic
149-
steps:
150-
- uses: actions/checkout@v4
151-
if: needs.needs-run.outputs.outcome == 'success'
152-
with:
153-
persist-credentials: false
154-
path: ddtrace
155-
- uses: actions/checkout@v4
156-
if: needs.needs-run.outputs.outcome == 'success'
157-
with:
158-
persist-credentials: false
159-
repository: sanic-org/sanic
160-
ref: v24.6.0
161-
path: sanic
162-
- uses: actions/setup-python@v5
163-
if: needs.needs-run.outputs.outcome == 'success'
164-
with:
165-
python-version: "3.11"
166-
- name: Install sanic and dependencies required to run tests
167-
if: needs.needs-run.outputs.outcome == 'success'
168-
run: pip3 install '.[test]' aioquic
169-
- name: Install ddtrace
170-
if: needs.needs-run.outputs.outcome == 'success'
171-
run: pip3 install ../ddtrace
172-
- name: Run tests
173-
if: needs.needs-run.outputs.outcome == 'success'
174-
run: ddtrace-run pytest -k "not test_reloader and not test_reload_listeners and not test_no_exceptions_when_cancel_pending_request and not test_add_signal and not test_ode_removes and not test_skip_touchup and not test_dispatch_signal_triggers and not test_keep_alive_connection_context and not test_redirect_with_params and not test_keep_alive_client_timeout and not test_logger_vhosts and not test_ssl_in_multiprocess_mode"
175-
176114
django-testsuite:
177115
strategy:
178116
matrix:
@@ -963,58 +901,3 @@ jobs:
963901
- name: Debugger exploration results
964902
if: needs.needs-run.outputs.outcome == 'success'
965903
run: cat debugger-expl.txt
966-
967-
beautifulsoup-testsuite-4_12_3:
968-
strategy:
969-
matrix:
970-
include:
971-
# TODO: profiling is disabled due to a bug in the profiler paths
972-
# - suffix: Profiling
973-
# profiling: 1
974-
# iast: 0
975-
# appsec: 0
976-
- suffix: IAST
977-
profiling: 0
978-
iast: 1
979-
appsec: 0
980-
- suffix: APPSEC
981-
profiling: 0
982-
iast: 0
983-
appsec: 1
984-
- suffix: Tracer only
985-
profiling: 0
986-
iast: 0
987-
appsec: 0
988-
name: Beautifulsoup 4.12.3 (with ${{ matrix.suffix }})
989-
runs-on: "ubuntu-latest"
990-
needs: needs-run
991-
env:
992-
DD_TESTING_RAISE: true
993-
DD_PROFILING_ENABLED: ${{ matrix.profiling }}
994-
DD_IAST_ENABLED: ${{ matrix.iast }}
995-
DD_APPSEC_ENABLED: ${{ matrix.appsec }}
996-
CMAKE_BUILD_PARALLEL_LEVEL: 12
997-
DD_DEBUGGER_EXPL_OUTPUT_FILE: debugger-expl.txt
998-
steps:
999-
- uses: actions/setup-python@v5
1000-
if: needs.needs-run.outputs.outcome == 'success'
1001-
with:
1002-
python-version: '3.9'
1003-
- uses: actions/checkout@v4
1004-
if: needs.needs-run.outputs.outcome == 'success'
1005-
with:
1006-
persist-credentials: false
1007-
path: ddtrace
1008-
- name: Checkout beautifulsoup
1009-
if: needs.needs-run.outputs.outcome == 'success'
1010-
run: |
1011-
git clone -b 4.12.3 https://git.launchpad.net/beautifulsoup
1012-
- name: Install ddtrace
1013-
if: needs.needs-run.outputs.outcome == 'success'
1014-
run: pip3 install ./ddtrace
1015-
- name: Pytest fix
1016-
if: needs.needs-run.outputs.outcome == 'success'
1017-
run: pip install pytest==8.2.1
1018-
- name: Run tests
1019-
if: needs.needs-run.outputs.outcome == 'success'
1020-
run: cd beautifulsoup && ddtrace-run pytest

ddtrace/internal/compat.py

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,32 +56,16 @@
5656
def ensure_text(s, encoding="utf-8", errors="ignore") -> str:
5757
if isinstance(s, str):
5858
return s
59-
6059
if isinstance(s, bytes):
6160
return s.decode(encoding, errors)
62-
63-
# Skip the check for Mock objects as they are used in tests
64-
from unittest.mock import Mock
65-
66-
if isinstance(s, Mock):
67-
return str(s)
68-
6961
raise TypeError("Expected str or bytes but received %r" % (s.__class__))
7062

7163

7264
def ensure_binary(s, encoding="utf-8", errors="ignore") -> bytes:
7365
if isinstance(s, bytes):
7466
return s
75-
76-
# Skip the check for Mock objects as they are used in tests
77-
from unittest.mock import Mock
78-
79-
if isinstance(s, Mock):
80-
return bytes(s)
81-
8267
if not isinstance(s, str):
8368
raise TypeError("Expected str or bytes but received %r" % (s.__class__))
84-
8569
return s.encode(encoding, errors)
8670

8771

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
asyncio: Resolves an issue where asyncio event loops fail to register when ``ddtrace-run``/``import ddtrace.auto`` is used and gevent is installed.

tests/contrib/asyncio/test_lazyimport.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,16 @@ def test_lazy_import():
1616
assert tracer.context_provider.active() is span
1717
span.finish()
1818
assert tracer.context_provider.active() is None
19+
20+
21+
@pytest.mark.subprocess()
22+
def test_asyncio_not_imported_by_auto_instrumentation():
23+
# Module unloading is not supported for asyncio, a simple workaround
24+
# is to ensure asyncio is not imported by ddtrace.auto or ddtrace-run.
25+
# If asyncio is imported by ddtrace.auto the asyncio event loop with fail
26+
# to register new loops in some platforms (e.g. Ubuntuu).
27+
import sys
28+
29+
import ddtrace.auto # noqa: F401
30+
31+
assert "asyncio" not in sys.modules

0 commit comments

Comments
 (0)