fix(internal): remove nogevent compatibility layer (#5105)

This pull request removes the `nogevent` compatibility layer from the
library and tests. It also changes the behavior of some feature flags
related to gevent and adjusts a whole lot of tests to work with these
changes.

This change was manually tested on delancie-dummy staging and shown to
fix the issue we were investigating there. See noteboook 4782453 in the
ddstaging datadoghq account for a detailed look at the metrics collected
during that test.

## What Changed?

* `sitecustomize.py`: refactored module cloning logic; changed the
default of `DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE` to `auto` meaning it
runs when gevent is installed; unloaded a few additional modules like
`time` and `attrs` that were causing tests and manual checks to fail;
deprecated `DD_GEVENT_PATCH_ALL` flag
* Removed various hacks and layers that had intended to fix gevent
compatibility, including in `forksafe.py`, `periodic.py`,
`ddtrace_gevent_check.py`, and `nogevent.py`
* `profiling/`: adjusted all uses of the removed module `nogevent` to
work with `threading`
* Adjusted tests to work with these removals

We tried to separate some of these changes into other pull requests,
which you can see linked in the discussion below. Because of how
difficult it is to replicate the issue we're chasing outside of the
staging environment, we decided to minimize the number of variables
under test and combine the various changes into a single pull request.
This makes it a bit harder to review, which we've tried to mitigate with
the checklist above.

## Risk

The main risk in this change is the change to the default behavior of
module cloning. We've mitigated this risk with the automated test suite
as well as manual testing described in the notebook above.

**Why doesn't this change put all new behavior behind a feature flag and
leave the default behavior untouched?**

The main reason for this decision is pragmatic: it's really hard to test
for the issue this solves, requiring a turnaround time of about an hour
to get feedback from changes. The secondary reason is that the
`nogevent` layer is highly coupled to the rest of the library's code,
and putting it behind a feature flag is a significant and nontrivial
effort. The third reason is that full support of all of the various
configurations and combinations with other tools that gevent can be used
in is a goal that we could probably spend infinite time on if we chose
to. Given this, we need to intentionally set a goal that solves the
current and likely near-future issues as completely as possible, make it
the default behavior, and call this effort "done". @brettlangdon
@P403n1x87 @Yun-Kim and I are in agreement that the evidence in
noteboook 4782453 in the ddstaging datadoghq account is enough to
justify this change to the default behavior.

## Performance Testing

Performance testing with a sample flask application (notebook 4442578)
shows no immediately noticeable impact of tracing.

Dynamic instrumentation seems to cause slow-downs, and the reason has
been tracked down to joining service threads on shutdown. Avoiding the
joins cures the problem, but further testing is required to ensure that
DI still behaves as intended.

Profiling also shows a slow-down in the application response when
enabled. This seems to be due to retrieving the response from the agent
after the payload has been uploaded. A potential solution to this might
be offered by libdatadog.

The following are the details of the scenario used to measure the
performance under different configurations.
The application is the simple Flask app of the issue reproducer:

```
# app.py

import os
import time

from ddtrace.internal.remoteconfig import RemoteConfig
from flask import Flask

app = Flask(__name__)


def start():
    pid1 = os.fork()
    if pid1 == 0:
        os.setsid()
        x = 2
        while x > 0:
            time.sleep(0.2)
            x -= 1
    else:
        os.waitpid(pid1, 0)


@app.route("/")
def index():
    start()
    return "OK" if RemoteConfig._worker is not None else "NOK"
```

We can control what products to start with the following run.sh script

```
#!/bin/bash

source .venv/bin/activate

export DD_DYNAMIC_INSTRUMENTATION_ENABLED=true
export DD_PROFILING_ENABLED=false
export DD_TRACE_ENABLED=false

export DD_ENV=gab-testing
export DD_SERVICE=flask-gevent

ddtrace-run gunicorn -w 3 -k gevent app:app

deactivate
```

To run the app we create a virtual environment with

```
python3.9 -m venv .venv
source .venv/bin/activate
pip install flask gevent gunicorn
pip install -e path/to/dd-trace-py
deactivate
```

and then invoke the script, adjusting the exported variables as required

```
chmod +x run.sh
./run.sh
```

In another terminal we can check the average response time by sending
requests to the application while running. With the following simple k6
script

```
import http from 'k6/http';

export default function () {
  http.get('http://localhost:8000');
}
```

We invoke k6 with

```
k6 run -d 30s script.js
```

and look for this line in the output

```
http_req_duration..............: avg=335.68ms min=119.56ms med=418.76ms max=451.49ms p(90)
```

Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Gabriele N. Tornetta <[email protected]>
Co-authored-by: Juanjo Alvarez Martinez <[email protected]>
Co-authored-by: Gabriele N. Tornetta <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Brett Langdon <[email protected]>

Author: 5 people

ddtrace/auto.py

@@ -0,0 +1 @@
+import ddtrace.bootstrap.sitecustomize  # noqa

ddtrace/bootstrap/sitecustomize.py

@@ -5,109 +5,19 @@
 import sys
 
 
-MODULES_LOADED_AT_STARTUP = frozenset(sys.modules.keys())
-MODULES_THAT_TRIGGER_CLEANUP_WHEN_INSTALLED = ("gevent",)
-
-
-import os  # noqa
-
-
-"""
-The following modules cause problems when being unloaded/reloaded in module cloning.
-Notably, unloading the atexit module will remove all registered hooks which we use for cleaning up on tracer shutdown.
-The other listed modules internally maintain some state that does not coexist well if reloaded.
-"""
-MODULES_TO_NOT_CLEANUP = {"atexit", "asyncio", "attr", "concurrent", "ddtrace", "logging"}
-if sys.version_info < (3, 7):
-    MODULES_TO_NOT_CLEANUP |= {"typing"}  # required by older versions of Python
-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]
-    # Some versions of CPython import the time module during interpreter startup, which needs to be unloaded.
-    if "time" in sys.modules:
-        del sys.modules["time"]
-
-
-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()
+LOADED_MODULES = frozenset(sys.modules.keys())
 
 import logging  # noqa
 import os  # noqa
 from typing import Any  # noqa
 from typing import Dict  # noqa
+import warnings  # noqa
 
 from ddtrace import config  # noqa
 from ddtrace.debugging._config import config as debugger_config  # noqa
+from ddtrace.internal.compat import PY2  # noqa
 from ddtrace.internal.logger import get_logger  # noqa
+from ddtrace.internal.module import find_loader  # noqa
 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  # noqa
@@ -142,6 +52,25 @@ def cleanup_loaded_modules(aggressive=False):
 
 log = get_logger(__name__)
 
+if os.environ.get("DD_GEVENT_PATCH_ALL") is not None:
+    deprecate(
+        "The environment variable DD_GEVENT_PATCH_ALL is deprecated and will be removed in a future version. ",
+        postfix="There is no special configuration necessary to make ddtrace work with gevent if using ddtrace-run. "
+        "If not using ddtrace-run, import ddtrace.auto before calling gevent.monkey.patch_all().",
+        removal_version="2.0.0",
+    )
+if "gevent" in sys.modules or "gevent.monkey" in sys.modules:
+    import gevent.monkey  # noqa
+
+    if gevent.monkey.is_module_patched("threading"):
+        warnings.warn(
+            "Loading ddtrace after gevent.monkey.patch_all() is not supported and is "
+            "likely to break the application. Use ddtrace-run to fix this, or "
+            "import ddtrace.auto before calling gevent.monkey.patch_all().",
+            RuntimeWarning,
+        )
+
+
 EXTRA_PATCHED_MODULES = {
     "bottle": True,
     "django": True,
@@ -162,6 +91,52 @@ def update_patched_modules():
         EXTRA_PATCHED_MODULES[module] = asbool(should_patch)
 
 
+if PY2:
+    _unloaded_modules = []
+
+
+def is_module_installed(module_name):
+    return find_loader(module_name) is not None
+
+
+def cleanup_loaded_modules():
+    MODULES_REQUIRING_CLEANUP = ("gevent",)
+    do_cleanup = os.getenv("DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE", default="auto").lower()
+    if do_cleanup == "auto":
+        do_cleanup = any(is_module_installed(m) for m in MODULES_REQUIRING_CLEANUP)
+
+    if not asbool(do_cleanup):
+        return
+
+    # Unload all the modules that we have imported, except for the ddtrace one.
+    # NB: this means that every `import threading` anywhere in `ddtrace/` code
+    # uses a copy of that module that is distinct from the copy that user code
+    # gets when it does `import threading`. The same applies to every module
+    # not in `KEEP_MODULES`.
+    KEEP_MODULES = frozenset(["atexit", "ddtrace", "asyncio", "concurrent", "typing", "logging", "attr"])
+    for m in list(_ for _ in sys.modules if _ not in LOADED_MODULES):
+        if any(m == _ or m.startswith(_ + ".") for _ in KEEP_MODULES):
+            continue
+
+        if PY2:
+            KEEP_MODULES_PY2 = frozenset(["encodings", "codecs"])
+            if any(m == _ or m.startswith(_ + ".") for _ in KEEP_MODULES_PY2):
+                continue
+            # Store a reference to deleted modules to avoid them being garbage
+            # collected
+            _unloaded_modules.append(sys.modules[m])
+
+        del sys.modules[m]
+
+    # TODO: The better strategy is to identify the core modues in LOADED_MODULES
+    # that should not be unloaded, and then unload as much as possible.
+    UNLOAD_MODULES = frozenset(["time"])
+    for u in UNLOAD_MODULES:
+        for m in list(sys.modules):
+            if m == u or m.startswith(u + "."):
+                del sys.modules[m]
+
+
 try:
     from ddtrace import tracer
 
@@ -202,19 +177,18 @@ 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
 
+        # 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.
+        cleanup_loaded_modules()
+
         patch_all(**EXTRA_PATCHED_MODULES)
+    else:
+        cleanup_loaded_modules()
 
     # Only the import of the original sitecustomize.py is allowed after this
     # point.

ddtrace/contrib/gevent/__init__.py

@@ -5,11 +5,9 @@
 The integration patches the gevent internals to add context management logic.
 
 .. note::
-    If :ref:`ddtrace-run<ddtracerun>` is being used set ``DD_GEVENT_PATCH_ALL=true`` and
-    ``gevent.monkey.patch_all()`` will be called as early as possible in the application
-    to avoid patching conflicts.
-    If ``ddtrace-run`` is not being used then be sure to call ``gevent.monkey.patch_all``
-    before importing ``ddtrace`` and calling ``ddtrace.patch`` or ``ddtrace.patch_all``.
+    If ``ddtrace-run`` is not being used then be sure to ``import ddtrace.auto``
+    before calling ``gevent.monkey.patch_all``.
+    If ``ddtrace-run`` is being used then no additional configuration is required.
 
 
 The integration also configures the global tracer instance to use a gevent

ddtrace/contrib/gunicorn/__init__.py

@@ -1,33 +1,10 @@
 """
-**Note:** ``ddtrace-run`` and Python 2 are both not supported with `Gunicorn <https://gunicorn.org>`__.
+ddtrace works with Gunicorn.
 
-``ddtrace`` only supports Gunicorn's ``gevent`` worker type when configured as follows:
-
-- The application is running under a Python version >=3.6 and <=3.10
-- `ddtrace-run` is not used
-- The `DD_GEVENT_PATCH_ALL=1` environment variable is set
-- Gunicorn's ```post_fork`` <https://docs.gunicorn.org/en/stable/settings.html#post-fork>`__ hook does not import from
-  ``ddtrace``
-- ``import ddtrace.bootstrap.sitecustomize`` is called either in the application's main process or in the
-  ```post_worker_init`` <https://docs.gunicorn.org/en/stable/settings.html#post-worker-init>`__ hook.
-
-.. code-block:: python
-
-  # gunicorn.conf.py
-  def post_fork(server, worker):
-      # don't touch ddtrace here
-      pass
-
-  def post_worker_init(worker):
-      import ddtrace.bootstrap.sitecustomize
-
-  workers = 4
-  worker_class = "gevent"
-  bind = "8080"
-
-.. code-block:: bash
-
-  DD_GEVENT_PATCH_ALL=1 gunicorn --config gunicorn.conf.py path.to.my:app
+.. note::
+    If you cannot wrap your Gunicorn server with the ``ddtrace-run``command and
+    it uses ``gevent`` workers, be sure to ``import ddtrace.auto`` as early as
+    possible in your application's lifecycle.
 """
 
 

ddtrace/internal/forksafe.py

@@ -7,8 +7,6 @@
 import typing
 import weakref
 
-from ddtrace.internal.module import ModuleWatchdog
-from ddtrace.internal.utils.formats import asbool
 from ddtrace.vendor import wrapt
 
 
@@ -24,26 +22,6 @@
 _soft = True
 
 
-def patch_gevent_hub_reinit(module):
-    # The gevent hub is re-initialized *after* the after-in-child fork hooks are
-    # called, so we patch the gevent.hub.reinit function to ensure that the
-    # fork hooks run again after this further re-initialisation, if it is ever
-    # called.
-    from ddtrace.internal.wrapping import wrap
-
-    def wrapped_reinit(f, args, kwargs):
-        try:
-            return f(*args, **kwargs)
-        finally:
-            ddtrace_after_in_child()
-
-    wrap(module.reinit, wrapped_reinit)
-
-
-if asbool(os.getenv("_DD_TRACE_GEVENT_HUB_PATCHED", default=False)):
-    ModuleWatchdog.register_module_hook("gevent.hub", patch_gevent_hub_reinit)
-
-
 def ddtrace_after_in_child():
     # type: () -> None
     global _registry