From befc80b31d088274f43188288a8a6158d1d84c9f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 14 Jan 2025 13:59:35 -0500 Subject: [PATCH] fix: resolve asyncio event loop bug when gevent is installed [backport 2.19] (#11921) Backport b769e1eb19b4cba48d03995c75e432c41c264c81 from #11904 to 2.19. ## Description Reverts a change ([4e312788f9dc4a7c65e2e69c2094be3313a6f858](https://github.com/DataDog/dd-trace-py/commit/4e312788f9dc4a7c65e2e69c2094be3313a6f858)) 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](https://github.com/DataDog/dd-trace-py/commit/dc000ae8397a7d8e1c0fb569622ee9d2e42082ac) 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) Co-authored-by: Munir Abdinur --- .github/workflows/test_frameworks.yml | 117 ------------------ ddtrace/internal/compat.py | 16 --- ...cio-incompatiblities-246028676b10bea9.yaml | 4 + tests/contrib/asyncio/test_lazyimport.py | 13 ++ 4 files changed, 17 insertions(+), 133 deletions(-) create mode 100644 releasenotes/notes/resolves-gevent-asyncio-incompatiblities-246028676b10bea9.yaml diff --git a/.github/workflows/test_frameworks.yml b/.github/workflows/test_frameworks.yml index 809dee38234..2e1502b4d3d 100644 --- a/.github/workflows/test_frameworks.yml +++ b/.github/workflows/test_frameworks.yml @@ -111,68 +111,6 @@ jobs: if: needs.needs-run.outputs.outcome == 'success' run: cat debugger-expl.txt - sanic-testsuite: - strategy: - matrix: - include: - # TODO: profiling fails with a timeout error - #- suffix: Profiling - # profiling: 1 - # iast: 0 - # appsec: 0 - - suffix: IAST - profiling: 0 - iast: 1 - appsec: 0 - - suffix: APPSEC - profiling: 0 - iast: 0 - appsec: 1 - - suffix: Tracer only - profiling: 0 - iast: 0 - appsec: 0 - name: Sanic 24.6 (with ${{ matrix.suffix }}) - runs-on: ubuntu-20.04 - needs: needs-run - timeout-minutes: 15 - env: - DD_PROFILING_ENABLED: ${{ matrix.profiling }} - DD_IAST_ENABLED: ${{ matrix.iast }} - DD_APPSEC_ENABLED: ${{ matrix.appsec }} - DD_TESTING_RAISE: true - CMAKE_BUILD_PARALLEL_LEVEL: 12 - DD_DEBUGGER_EXPL_OUTPUT_FILE: debugger-expl.txt - defaults: - run: - working-directory: sanic - steps: - - uses: actions/checkout@v4 - if: needs.needs-run.outputs.outcome == 'success' - with: - persist-credentials: false - path: ddtrace - - uses: actions/checkout@v4 - if: needs.needs-run.outputs.outcome == 'success' - with: - persist-credentials: false - repository: sanic-org/sanic - ref: v24.6.0 - path: sanic - - uses: actions/setup-python@v5 - if: needs.needs-run.outputs.outcome == 'success' - with: - python-version: "3.11" - - name: Install sanic and dependencies required to run tests - if: needs.needs-run.outputs.outcome == 'success' - run: pip3 install '.[test]' aioquic - - name: Install ddtrace - if: needs.needs-run.outputs.outcome == 'success' - run: pip3 install ../ddtrace - - name: Run tests - if: needs.needs-run.outputs.outcome == 'success' - 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" - django-testsuite: strategy: matrix: @@ -963,58 +901,3 @@ jobs: - name: Debugger exploration results if: needs.needs-run.outputs.outcome == 'success' run: cat debugger-expl.txt - - beautifulsoup-testsuite-4_12_3: - strategy: - matrix: - include: - # TODO: profiling is disabled due to a bug in the profiler paths - # - suffix: Profiling - # profiling: 1 - # iast: 0 - # appsec: 0 - - suffix: IAST - profiling: 0 - iast: 1 - appsec: 0 - - suffix: APPSEC - profiling: 0 - iast: 0 - appsec: 1 - - suffix: Tracer only - profiling: 0 - iast: 0 - appsec: 0 - name: Beautifulsoup 4.12.3 (with ${{ matrix.suffix }}) - runs-on: "ubuntu-latest" - needs: needs-run - env: - DD_TESTING_RAISE: true - DD_PROFILING_ENABLED: ${{ matrix.profiling }} - DD_IAST_ENABLED: ${{ matrix.iast }} - DD_APPSEC_ENABLED: ${{ matrix.appsec }} - CMAKE_BUILD_PARALLEL_LEVEL: 12 - DD_DEBUGGER_EXPL_OUTPUT_FILE: debugger-expl.txt - steps: - - uses: actions/setup-python@v5 - if: needs.needs-run.outputs.outcome == 'success' - with: - python-version: '3.9' - - uses: actions/checkout@v4 - if: needs.needs-run.outputs.outcome == 'success' - with: - persist-credentials: false - path: ddtrace - - name: Checkout beautifulsoup - if: needs.needs-run.outputs.outcome == 'success' - run: | - git clone -b 4.12.3 https://git.launchpad.net/beautifulsoup - - name: Install ddtrace - if: needs.needs-run.outputs.outcome == 'success' - run: pip3 install ./ddtrace - - name: Pytest fix - if: needs.needs-run.outputs.outcome == 'success' - run: pip install pytest==8.2.1 - - name: Run tests - if: needs.needs-run.outputs.outcome == 'success' - run: cd beautifulsoup && ddtrace-run pytest diff --git a/ddtrace/internal/compat.py b/ddtrace/internal/compat.py index 457618dc393..6ebe450583e 100644 --- a/ddtrace/internal/compat.py +++ b/ddtrace/internal/compat.py @@ -56,32 +56,16 @@ def ensure_text(s, encoding="utf-8", errors="ignore") -> str: if isinstance(s, str): return s - if isinstance(s, bytes): return s.decode(encoding, errors) - - # Skip the check for Mock objects as they are used in tests - from unittest.mock import Mock - - if isinstance(s, Mock): - return str(s) - raise TypeError("Expected str or bytes but received %r" % (s.__class__)) def ensure_binary(s, encoding="utf-8", errors="ignore") -> bytes: if isinstance(s, bytes): return s - - # Skip the check for Mock objects as they are used in tests - from unittest.mock import Mock - - if isinstance(s, Mock): - return bytes(s) - if not isinstance(s, str): raise TypeError("Expected str or bytes but received %r" % (s.__class__)) - return s.encode(encoding, errors) diff --git a/releasenotes/notes/resolves-gevent-asyncio-incompatiblities-246028676b10bea9.yaml b/releasenotes/notes/resolves-gevent-asyncio-incompatiblities-246028676b10bea9.yaml new file mode 100644 index 00000000000..08a5448b3dc --- /dev/null +++ b/releasenotes/notes/resolves-gevent-asyncio-incompatiblities-246028676b10bea9.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + asyncio: Resolves an issue where asyncio event loops fail to register when ``ddtrace-run``/``import ddtrace.auto`` is used and gevent is installed. diff --git a/tests/contrib/asyncio/test_lazyimport.py b/tests/contrib/asyncio/test_lazyimport.py index adca84973db..07c96bc799f 100644 --- a/tests/contrib/asyncio/test_lazyimport.py +++ b/tests/contrib/asyncio/test_lazyimport.py @@ -16,3 +16,16 @@ def test_lazy_import(): assert tracer.context_provider.active() is span span.finish() assert tracer.context_provider.active() is None + + +@pytest.mark.subprocess() +def test_asyncio_not_imported_by_auto_instrumentation(): + # Module unloading is not supported for asyncio, a simple workaround + # is to ensure asyncio is not imported by ddtrace.auto or ddtrace-run. + # If asyncio is imported by ddtrace.auto the asyncio event loop with fail + # to register new loops in some platforms (e.g. Ubuntuu). + import sys + + import ddtrace.auto # noqa: F401 + + assert "asyncio" not in sys.modules