fix(gevent): fix gevent compatibility with "module cloning" approach

ddtrace/bootstrap/sitecustomize.py

@@ -2,30 +2,108 @@
 Bootstrapping code that is run when using the `ddtrace-run` Python entrypoint
 Add all monkey-patching that needs to run by default here
 """
-import logging
-import os
 import sys
-from typing import Any
-from typing import Dict
 
 
-# Perform gevent patching as early as possible in the application before
-# importing more of the library internals.
-if os.environ.get("DD_GEVENT_PATCH_ALL", "false").lower() in ("true", "1"):
-    import gevent.monkey
+MODULES_LOADED_AT_STARTUP = frozenset(sys.modules.keys())
+MODULES_THAT_TRIGGER_CLEANUP_WHEN_INSTALLED = ("gevent",)
 
-    gevent.monkey.patch_all()
 
+import os  # noqa
+
+
+MODULES_TO_NOT_CLEANUP = {"atexit", "asyncio", "attr", "concurrent", "ddtrace", "logging"}
+if sys.version_info <= (2, 7):
+    MODULES_TO_NOT_CLEANUP |= {"encodings", "codecs"}
+    import imp
+
+    _unloaded_modules = []
+
+    def is_installed(module_name):
+        try:
+            imp.find_module(module_name)
+        except ImportError:
+            return False
+        return True
+
+
+else:
+    import importlib
+
+    def is_installed(module_name):
+        return importlib.util.find_spec(module_name)
+
+
+def should_cleanup_loaded_modules():
+    dd_unload_sitecustomize_modules = os.getenv("DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE", default="0").lower()
+    if dd_unload_sitecustomize_modules not in ("1", "auto"):
+        return False
+    elif dd_unload_sitecustomize_modules == "auto" and not any(
+        is_installed(module_name) for module_name in MODULES_THAT_TRIGGER_CLEANUP_WHEN_INSTALLED
+    ):
+        return False
+    return True
+
+
+def cleanup_loaded_modules(aggressive=False):
+    """
+    "Aggressive" here means "cleanup absolutely every module that has been loaded since startup".
+    Non-aggressive cleanup entails leaving untouched certain modules
+    This distinction is necessary because this function is used both to prepare for gevent monkeypatching
+    (requiring aggressive cleanup) and to implement "module cloning" (requiring non-aggressive cleanup)
+    """
+    # Figuring out modules_loaded_since_startup is necessary because sys.modules has more in it than just what's in
+    # import statements in this file, and unloading some of them can break the interpreter.
+    modules_loaded_since_startup = set(_ for _ in sys.modules if _ not in MODULES_LOADED_AT_STARTUP)
+    # Unload all the modules that we have imported, except for ddtrace and a few
+    # others that don't like being cloned.
+    # Doing so will allow ddtrace to continue using its local references to modules unpatched by
+    # gevent, while avoiding conflicts with user-application code potentially running
+    # `gevent.monkey.patch_all()` and thus gevent-patched versions of the same modules.
+    for module_name in modules_loaded_since_startup:
+        if aggressive:
+            del sys.modules[module_name]
+            continue
+
+        for module_to_not_cleanup in MODULES_TO_NOT_CLEANUP:
+            if module_name == module_to_not_cleanup:
+                break
+            elif module_name.startswith("%s." % module_to_not_cleanup):
+                break
+        else:
+            del sys.modules[module_name]
+
+
+will_run_module_cloning = should_cleanup_loaded_modules()
+if not will_run_module_cloning:
+    # Perform gevent patching as early as possible in the application before
+    # importing more of the library internals.
+    if os.environ.get("DD_GEVENT_PATCH_ALL", "false").lower() in ("true", "1"):
+        # successfully running `gevent.monkey.patch_all()` this late into
+        # sitecustomize requires aggressive module unloading beforehand.
+        # gevent's documentation strongly warns against calling monkey.patch_all() anywhere other
+        # than the first line of the program. since that's what we're doing here,
+        # we cleanup aggressively beforehand to replicate the conditions at program start
+        # as closely as possible.
+        cleanup_loaded_modules(aggressive=True)
+        import gevent.monkey
+
+        gevent.monkey.patch_all()
+
+import logging  # noqa
+import os  # noqa
+from typing import Any  # noqa
+from typing import Dict  # noqa
 
 from ddtrace import config  # noqa
-from ddtrace.debugging._config import config as debugger_config
+from ddtrace.debugging._config import config as debugger_config  # noqa
 from ddtrace.internal.logger import get_logger  # noqa
-from ddtrace.internal.runtime.runtime_metrics import RuntimeWorker
+from ddtrace.internal.runtime.runtime_metrics import RuntimeWorker  # noqa
 from ddtrace.internal.utils.formats import asbool  # noqa
-from ddtrace.internal.utils.formats import parse_tags_str
+from ddtrace.internal.utils.formats import parse_tags_str  # noqa
 from ddtrace.tracer import DD_LOG_FORMAT  # noqa
-from ddtrace.tracer import debug_mode
-from ddtrace.vendor.debtcollector import deprecate
+from ddtrace.tracer import debug_mode  # noqa
+from ddtrace.vendor.debtcollector import deprecate  # noqa
 
 
 if config.logs_injection:
@@ -107,12 +185,23 @@ def update_patched_modules():
     if not opts:
         tracer.configure(**opts)
 
+    # We need to clean up after we have imported everything we need from
+    # ddtrace, but before we register the patch-on-import hooks for the
+    # integrations. This is because registering a hook for a module
+    # that is already imported causes the module to be patched immediately.
+    # So if we unload the module after registering hooks, we effectively
+    # remove the patching, thus breaking the tracer integration.
+    if will_run_module_cloning:
+        cleanup_loaded_modules()
     if trace_enabled:
         update_patched_modules()
         from ddtrace import patch_all
 
         patch_all(**EXTRA_PATCHED_MODULES)
 
+    # Only the import of the original sitecustomize.py is allowed after this
+    # point.
+
     if "DD_TRACE_GLOBAL_TAGS" in os.environ:
         env_tags = os.getenv("DD_TRACE_GLOBAL_TAGS")
         tracer.set_tags(parse_tags_str(env_tags))

ddtrace/internal/telemetry/writer.py

@@ -160,6 +160,7 @@ def on_shutdown(self):
     def _stop_service(self, *args, **kwargs):
         # type: (...) -> None
         super(TelemetryWriter, self)._stop_service(*args, **kwargs)
+        # TODO: Call this with an atexit hook
         self.join()
 
     def add_event(self, payload, payload_type):

docs/configuration.rst

@@ -223,10 +223,10 @@ The following environment variables for the tracer are supported:
    DD_SPAN_SAMPLING_RULES:
      type: string
      description: |
-         A JSON array of objects. Each object must have a "name" and/or "service" field, while the "max_per_second" and "sample_rate" fields are optional. 
+         A JSON array of objects. Each object must have a "name" and/or "service" field, while the "max_per_second" and "sample_rate" fields are optional.
          The "sample_rate" value must be between 0.0 and 1.0 (inclusive), and will default to 1.0 (100% sampled).
-         The "max_per_second" value must be >= 0 and will default to no limit. 
-         The "service" and "name" fields can be glob patterns: 
+         The "max_per_second" value must be >= 0 and will default to no limit.
+         The "service" and "name" fields can be glob patterns:
          "*" matches any substring, including the empty string,
          "?" matches exactly one of any character, and any other character matches exactly one of itself.
 
@@ -238,11 +238,11 @@ The following environment variables for the tracer are supported:
    DD_SPAN_SAMPLING_RULES_FILE:
      type: string
      description: |
-         A path to a JSON file containing span sampling rules organized as JSON array of objects. 
-         For the rules each object must have a "name" and/or "service" field, and the "sample_rate" field is optional. 
+         A path to a JSON file containing span sampling rules organized as JSON array of objects.
+         For the rules each object must have a "name" and/or "service" field, and the "sample_rate" field is optional.
          The "sample_rate" value must be between 0.0 and 1.0 (inclusive), and will default to 1.0 (100% sampled).
-         The "max_per_second" value must be >= 0 and will default to no limit. 
-         The "service" and "name" fields are glob patterns, where "glob" means: 
+         The "max_per_second" value must be >= 0 and will default to no limit.
+         The "service" and "name" fields are glob patterns, where "glob" means:
          "*" matches any substring, including the empty string,
          "?" matches exactly one of any character, and any other character matches exactly one of itself.
 
@@ -287,7 +287,7 @@ The following environment variables for the tracer are supported:
          The supported values are ``datadog``, ``b3multi``, and ``b3 single header``, and ``none``.
 
          When checking inbound request headers we will take the first valid trace context in the order provided.
-         When ``none`` is the only propagator listed, propagation is disabled. 
+         When ``none`` is the only propagator listed, propagation is disabled.
 
          All provided styles are injected into the headers of outbound requests.
 
@@ -308,7 +308,7 @@ The following environment variables for the tracer are supported:
          The supported values are ``datadog``, ``b3multi``, and ``b3 single header``, and ``none``.
 
          When checking inbound request headers we will take the first valid trace context in the order provided.
-         When ``none`` is the only propagator listed, extraction is disabled. 
+         When ``none`` is the only propagator listed, extraction is disabled.
 
          Example: ``DD_TRACE_PROPAGATION_STYLE="datadog,b3"`` to check for both ``x-datadog-*`` and ``x-b3-*``
          headers when parsing incoming request headers for a trace context. In addition, to inject both ``x-datadog-*`` and ``x-b3-*``
@@ -328,7 +328,7 @@ The following environment variables for the tracer are supported:
          The supported values are ``datadog``, ``b3multi``, and ``b3 single header``, and ``none``.
 
          All provided styles are injected into the headers of outbound requests.
-         When ``none`` is the only propagator listed, injection is disabled. 
+         When ``none`` is the only propagator listed, injection is disabled.
 
          Example: ``DD_TRACE_PROPAGATION_STYLE_INJECT="datadog,b3multi"`` to inject both ``x-datadog-*`` and ``x-b3-*``
          headers into outbound requests.
@@ -473,6 +473,17 @@ The following environment variables for the tracer are supported:
      default: "DES,Blowfish,RC2,RC4,IDEA"
      description: Weak cipher algorithms that should be reported, comma separated.
 
+   DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE:
+     type: String
+     default: "0"
+     description: |
+        Controls whether module cloning logic is executed by ``ddtrace-run``. Module cloning involves saving copies of dependency modules for internal use by ``ddtrace``
+        that will be unaffected by future imports of and changes to those modules by application code. Valid values for this variable are ``1``, ``0``, and ``auto``. ``1`` tells
+        ``ddtrace`` to run its module cloning logic unconditionally, ``0`` tells it not to run that logic, and ``auto`` tells it to run module cloning logic only if ``gevent``
+        is accessible from the application's runtime.
+     version_added:
+        v1.9.0:
+
 .. _Unified Service Tagging: https://docs.datadoghq.com/getting_started/tagging/unified_service_tagging/
 
 

releasenotes/notes/gevent-compatibility-0fe0623c602d7617.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    gevent: This fix resolves incompatibility under 3.8>=Python<=3.10 between ``ddtrace-run`` and applications that depend on ``gevent``, for example ``gunicorn`` servers. It accomplishes this by
+    keeping copies that have not been monkey patched by ``gevent`` of most modules used by ``ddtrace``. This "module cloning" logic can be controlled by the environment
+    variable ``DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE``. Valid values for this variable are "1", "0", and "auto". "1" tells ``ddtrace`` to run its module cloning logic
+    unconditionally, "0" tells it never to run that logic, and "auto" tells it to run module cloning logic *only if* ``gevent`` is accessible from the application's runtime.
+    The default value is "0".

riotfile.py

@@ -2604,7 +2604,7 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
             pkgs={"requests": latest, "gevent": latest},
             venvs=[
                 Venv(
-                    pys=select_pys(min_version="3.5"),
+                    pys=select_pys(min_version="3.8"),
                     pkgs={"gunicorn": ["==19.10.0", "==20.0.4", latest]},
                 ),
             ],

tests/contrib/gunicorn/test_gunicorn.py

@@ -76,6 +76,7 @@ def _gunicorn_settings_factory(
     patch_gevent=None,  # type: Optional[bool]
     import_sitecustomize_in_app=None,  # type: Optional[bool]
     start_service_in_hook_named="post_fork",  # type: str
+    enable_module_cloning=False,  # type: bool
 ):
     # type: (...) -> GunicornServerSettings
     """Factory for creating gunicorn settings with simple defaults if settings are not defined."""
@@ -85,6 +86,7 @@ def _gunicorn_settings_factory(
         env["DD_GEVENT_PATCH_ALL"] = str(patch_gevent)
     if import_sitecustomize_in_app is not None:
         env["_DD_TEST_IMPORT_SITECUSTOMIZE"] = str(import_sitecustomize_in_app)
+    env["DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE"] = "1" if enable_module_cloning else "0"
     env["DD_REMOTECONFIG_POLL_SECONDS"] = str(SERVICE_INTERVAL)
     env["DD_PROFILING_UPLOAD_INTERVAL"] = str(SERVICE_INTERVAL)
     return GunicornServerSettings(
@@ -167,10 +169,10 @@ def gunicorn_server(gunicorn_server_settings, tmp_path):
         server_process.wait()
 
 
-SETTINGS_GEVENT_DDTRACERUN_PATCH = _gunicorn_settings_factory(
-    worker_class="gevent",
-    patch_gevent=True,
+SETTINGS_GEVENT_DDTRACERUN_MODULE_CLONE = _gunicorn_settings_factory(
+    worker_class="gevent", patch_gevent=False, enable_module_cloning=True
 )
+SETTINGS_GEVENT_DDTRACERUN_PATCH = _gunicorn_settings_factory(worker_class="gevent", patch_gevent=True)
 SETTINGS_GEVENT_APPIMPORT_PATCH_POSTWORKERSERVICE = _gunicorn_settings_factory(
     worker_class="gevent",
     use_ddtracerun=False,
@@ -187,19 +189,21 @@ def gunicorn_server(gunicorn_server_settings, tmp_path):
 )
 
 
+@pytest.mark.skipif(sys.version_info > (3, 10), reason="Gunicorn is only supported up to 3.10")
 @pytest.mark.parametrize(
     "gunicorn_server_settings",
     [
         SETTINGS_GEVENT_APPIMPORT_PATCH_POSTWORKERSERVICE,
         SETTINGS_GEVENT_POSTWORKERIMPORT_PATCH_POSTWORKERSERVICE,
+        SETTINGS_GEVENT_DDTRACERUN_MODULE_CLONE,
     ],
 )
 def test_no_known_errors_occur(gunicorn_server_settings, tmp_path):
     with gunicorn_server(gunicorn_server_settings, tmp_path) as context:
         server_process, client = context
         r = client.get("/")
     assert_no_profiler_error(server_process)
-    assert_remoteconfig_started_successfully(r)
+    assert_remoteconfig_started_successfully(r, gunicorn_server_settings.env["DD_GEVENT_PATCH_ALL"] == "True")
 
 
 @pytest.mark.parametrize(
@@ -213,6 +217,6 @@ def test_profiler_error_occurs_under_gevent_worker(gunicorn_server_settings, tmp
         server_process, client = context
         r = client.get("/")
     # this particular error does not manifest in 3.8 and older
-    if sys.version_info[1] > 8:
+    if sys.version_info >= (3, 9):
         assert MOST_DIRECT_KNOWN_GUNICORN_RELATED_PROFILER_ERROR_SIGNAL in server_process.stderr.read()
     assert_remoteconfig_started_successfully(r)