diff --git a/ddtrace/profiling/collector/stack.pyx b/ddtrace/profiling/collector/stack.pyx index 77426e83464..f9bb099062b 100644 --- a/ddtrace/profiling/collector/stack.pyx +++ b/ddtrace/profiling/collector/stack.pyx @@ -2,7 +2,7 @@ from __future__ import absolute_import import sys -import threading as ddtrace_threading +import threading import typing import attr @@ -20,6 +20,10 @@ from ddtrace.profiling.collector import _traceback from ddtrace.profiling.collector import stack_event +# NOTE: Do not use LOG here. This code runs under a real OS thread and is unable to acquire any lock of the `logging` +# module without having gevent crashing our dedicated thread. + + # These are special features that might not be available depending on your Python version and platform FEATURES = { "cpu-time": False, @@ -292,12 +296,14 @@ cdef collect_threads(thread_id_ignore_list, thread_time, thread_span_links) with cdef stack_collect(ignore_profiler, thread_time, max_nframes, interval, wall_time, thread_span_links, collect_endpoint): - # Do not use `threading.enumerate` to not mess with locking (gevent!) - thread_id_ignore_list = { - thread_id - for thread_id, thread in ddtrace_threading._active.items() - if getattr(thread, "_ddtrace_profiling_ignore", False) - } if ignore_profiler else set() + + if ignore_profiler: + # Do not use `threading.enumerate` to not mess with locking (gevent!) + thread_id_ignore_list = {thread_id + for thread_id, thread in threading._active.items() + if getattr(thread, "_ddtrace_profiling_ignore", False)} + else: + thread_id_ignore_list = set() running_threads = collect_threads(thread_id_ignore_list, thread_time, thread_span_links) diff --git a/setup.py b/setup.py index 026d3c80b91..954667a2fa1 100644 --- a/setup.py +++ b/setup.py @@ -327,6 +327,7 @@ def get_exts_for(name): "ddtrace.appsec": ["rules.json"], "ddtrace.appsec.ddwaf": [os.path.join("libddwaf", "*", "lib", "libddwaf.*")], }, + py_modules=["ddtrace_gevent_check"], python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*", zip_safe=False, # enum34 is an enum backport for earlier versions of python @@ -377,6 +378,9 @@ def get_exts_for(name): "ddtrace = ddtrace.contrib.pytest.plugin", "ddtrace.pytest_bdd = ddtrace.contrib.pytest_bdd.plugin", ], + "gevent.plugins.monkey.did_patch_all": [ + "ddtrace_gevent_check = ddtrace_gevent_check:gevent_patch_all", + ], }, classifiers=[ "Programming Language :: Python", diff --git a/tests/commands/ddtrace_run_gevent.py b/tests/commands/ddtrace_run_gevent.py new file mode 100644 index 00000000000..1ab77c6221a --- /dev/null +++ b/tests/commands/ddtrace_run_gevent.py @@ -0,0 +1,9 @@ +import socket + +import gevent.socket + + +# https://stackoverflow.com/a/24770674 +if __name__ == "__main__": + assert socket.socket is gevent.socket.socket + print("Test success") diff --git a/tests/commands/test_runner.py b/tests/commands/test_runner.py index ccc8d43b029..f9ea896e705 100644 --- a/tests/commands/test_runner.py +++ b/tests/commands/test_runner.py @@ -253,7 +253,16 @@ def test_logs_injection(self): """Ensure logs injection works""" with self.override_env(dict(DD_LOGS_INJECTION="true", DD_CALL_BASIC_CONFIG="true")): out = subprocess.check_output(["ddtrace-run", "python", "tests/commands/ddtrace_run_logs_injection.py"]) - assert out.startswith(b"Test success"), out.decode() + assert out.startswith(b"Test success") + + def test_gevent_patch_all(self): + with self.override_env(dict(DD_GEVENT_PATCH_ALL="true")): + out = subprocess.check_output(["ddtrace-run", "python", "tests/commands/ddtrace_run_gevent.py"]) + assert out.startswith(b"Test success") + + with self.override_env(dict(DD_GEVENT_PATCH_ALL="1")): + out = subprocess.check_output(["ddtrace-run", "python", "tests/commands/ddtrace_run_gevent.py"]) + assert out.startswith(b"Test success") def test_debug_mode(self): with self.override_env(dict(DD_CALL_BASIC_CONFIG="true")): diff --git a/tests/contrib/gevent/test_monkeypatch.py b/tests/contrib/gevent/test_monkeypatch.py index a64f4d59380..bc1ea277ab9 100644 --- a/tests/contrib/gevent/test_monkeypatch.py +++ b/tests/contrib/gevent/test_monkeypatch.py @@ -17,7 +17,7 @@ def test_gevent_warning(monkeypatch): ) assert subp.wait() == 0 assert subp.stdout.read() == b"" - assert subp.stderr.read() == b"" + assert b"RuntimeWarning: Loading ddtrace before using gevent monkey patching" in subp.stderr.read() @pytest.mark.subprocess diff --git a/tests/profiling/collector/test_asyncio.py b/tests/profiling/collector/test_asyncio.py index d7ed9ab9e86..00eca3e23bd 100644 --- a/tests/profiling/collector/test_asyncio.py +++ b/tests/profiling/collector/test_asyncio.py @@ -2,8 +2,8 @@ import uuid import pytest -from six.moves import _thread +from ddtrace.internal import nogevent from ddtrace.profiling import recorder from ddtrace.profiling.collector import asyncio as collector_asyncio @@ -19,7 +19,7 @@ async def test_lock_acquire_events(): assert len(r.events[collector_asyncio.AsyncioLockReleaseEvent]) == 0 event = r.events[collector_asyncio.AsyncioLockAcquireEvent][0] assert event.lock_name == "test_asyncio.py:15" - assert event.thread_id == _thread.get_ident() + assert event.thread_id == nogevent.thread_get_ident() assert event.wait_time_ns >= 0 # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 @@ -40,7 +40,7 @@ async def test_asyncio_lock_release_events(): assert len(r.events[collector_asyncio.AsyncioLockReleaseEvent]) == 1 event = r.events[collector_asyncio.AsyncioLockReleaseEvent][0] assert event.lock_name == "test_asyncio.py:35" - assert event.thread_id == _thread.get_ident() + assert event.thread_id == nogevent.thread_get_ident() assert event.locked_for_ns >= 0 # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 diff --git a/tests/profiling/run.py b/tests/profiling/run.py index b7584fb735d..d9848caa55a 100644 --- a/tests/profiling/run.py +++ b/tests/profiling/run.py @@ -1,7 +1,15 @@ +import os import runpy import sys +if "DD_PROFILE_TEST_GEVENT" in os.environ: + from gevent import monkey + + monkey.patch_all() + print("=> gevent monkey patching done") + +# TODO Use gevent.monkey once https://github.com/gevent/gevent/pull/1440 is merged? module = sys.argv[1] del sys.argv[0] runpy.run_module(module, run_name="__main__") diff --git a/tests/profiling/simple_program_gevent.py b/tests/profiling/simple_program_gevent.py index d3606cac74b..d2d516be6ad 100644 --- a/tests/profiling/simple_program_gevent.py +++ b/tests/profiling/simple_program_gevent.py @@ -7,6 +7,7 @@ import time from ddtrace.profiling import bootstrap +# do not use ddtrace-run; the monkey-patching would be done too late import ddtrace.profiling.auto from ddtrace.profiling.collector import stack_event diff --git a/tests/profiling/test_gunicorn.py b/tests/profiling/test_gunicorn.py index 2b4edecb088..4171e9128e1 100644 --- a/tests/profiling/test_gunicorn.py +++ b/tests/profiling/test_gunicorn.py @@ -79,4 +79,5 @@ def test_gunicorn(gunicorn, tmp_path, monkeypatch): @pytest.mark.skipif(not TESTING_GEVENT, reason="Not testing gevent") def test_gunicorn_gevent(gunicorn, tmp_path, monkeypatch): # type: (...) -> None + monkeypatch.setenv("DD_GEVENT_PATCH_ALL", "1") _test_gunicorn(gunicorn, tmp_path, monkeypatch, "--worker-class", "gevent")