diff --git a/ddtrace/bootstrap/sitecustomize.py b/ddtrace/bootstrap/sitecustomize.py index 3834c6f0acf..c2a8e8c572b 100644 --- a/ddtrace/bootstrap/sitecustomize.py +++ b/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)) diff --git a/ddtrace/internal/telemetry/writer.py b/ddtrace/internal/telemetry/writer.py index 2efcc440f08..5a8cd667804 100644 --- a/ddtrace/internal/telemetry/writer.py +++ b/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): diff --git a/docs/configuration.rst b/docs/configuration.rst index 12333c43f8b..789d78f72ea 100644 --- a/docs/configuration.rst +++ b/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/ diff --git a/releasenotes/notes/gevent-compatibility-0fe0623c602d7617.yaml b/releasenotes/notes/gevent-compatibility-0fe0623c602d7617.yaml new file mode 100644 index 00000000000..624c295e1b2 --- /dev/null +++ b/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". diff --git a/riotfile.py b/riotfile.py index e0397dc6a47..acc113fad1e 100644 --- a/riotfile.py +++ b/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]}, ), ], diff --git a/tests/contrib/gunicorn/test_gunicorn.py b/tests/contrib/gunicorn/test_gunicorn.py index 3b24e084b78..5b493658621 100644 --- a/tests/contrib/gunicorn/test_gunicorn.py +++ b/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,11 +189,13 @@ 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): @@ -199,7 +203,7 @@ def test_no_known_errors_occur(gunicorn_server_settings, tmp_path): 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)