Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(gevent): run after-in-child hooks after reinit #4070

Merged
merged 15 commits into from
Aug 19, 2022
Merged
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ddtrace/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
from ._logger import configure_ddtrace_logger
from ddtrace.internal.module import ModuleWatchdog


ModuleWatchdog.install()
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved

from ._logger import configure_ddtrace_logger # noqa: E402


# configure ddtrace logger before other modules log
20 changes: 20 additions & 0 deletions ddtrace/internal/forksafe.py
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@
import typing
import weakref

from ddtrace.internal.module import ModuleWatchdog
from ddtrace.vendor import wrapt


@@ -22,6 +23,25 @@
_soft = False


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)


ModuleWatchdog.register_module_hook("gevent.hub", patch_gevent_hub_reinit)


def ddtrace_after_in_child():
# type: () -> None
global _registry
48 changes: 31 additions & 17 deletions ddtrace/internal/module.py
Original file line number Diff line number Diff line change
@@ -122,26 +122,24 @@ def _resolve(path):
# https://github.com/GrahamDumpleton/wrapt/blob/df0e62c2740143cceb6cafea4c306dae1c559ef8/src/wrapt/importer.py

if PY2:
import pkgutil
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved

find_spec = ModuleSpec = None
Loader = object

find_loader = pkgutil.find_loader

else:
from importlib.abc import Loader
from importlib.machinery import ModuleSpec
from importlib.util import find_spec

def find_loader(fullname):
# type: (str) -> Optional[Loader]
return getattr(find_spec(fullname), "loader", None)

# DEV: This is used by Python 2 only
class _ImportHookLoader(object):
def __init__(self, callback):
# type: (Callable[[ModuleType], None]) -> None
self.callback = callback

def load_module(self, fullname):
# type: (str) -> ModuleType
module = sys.modules[fullname]
self.callback(module)

return module
LEGACY_DICT_COPY = sys.version_info < (3, 6)


class _ImportHookChainedLoader(Loader):
@@ -283,6 +281,18 @@ def __delitem__(self, name):

def __getattribute__(self, name):
# type: (str) -> Any
if LEGACY_DICT_COPY and name == "keys":
# This is a potential attempt to make a copy of sys.modules using
# dict(sys.modules) on a Python version that uses the C API to
# perform the operation. Since we are an instance of a dict, this
# means that we will end up looking like the empty dict, so we take
# this chance to actually look like sys.modules.
# NOTE: This is a potential source of memory leaks. However, we
# expect this to occur only on defunct Python versions, and only
# during special code executions, like test runs.
super(ModuleWatchdog, self).clear()
super(ModuleWatchdog, self).update(self._modules)

try:
return super(ModuleWatchdog, self).__getattribute__("_modules").__getattribute__(name)
except AttributeError:
@@ -308,16 +318,20 @@ def find_module(self, fullname, path=None):
self._finding.add(fullname)

try:
if PY2:
__import__(fullname)
return _ImportHookLoader(self.after_import)

loader = getattr(find_spec(fullname), "loader", None)
loader = find_loader(fullname)
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved
if loader is not None:
if not isinstance(loader, _ImportHookChainedLoader):
loader = _ImportHookChainedLoader(loader)

loader.add_callback(type(self), self.after_import)
if PY2:
# With Python 2 we don't get all the finder invoked, so we
# make sure we register all the callbacks at the earliest
# opportunity.
for finder in sys.meta_path:
if isinstance(finder, ModuleWatchdog):
loader.add_callback(type(finder), finder.after_import)
else:
loader.add_callback(type(self), self.after_import)

return loader

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[build-system]
requires = ["setuptools >= 40.6.0", "setuptools_scm[toml] >=4,<6.1", "cython", "cmake >= 3.14", "ninja"]
requires = ["setuptools >= 40.6.0,<64", "setuptools_scm[toml] >=4,<6.1", "cython", "cmake >= 3.14", "ninja"]
brettlangdon marked this conversation as resolved.
Show resolved Hide resolved
build-backend = "setuptools.build_meta"

[tool.setuptools_scm]
1 change: 1 addition & 0 deletions riotfile.py
Original file line number Diff line number Diff line change
@@ -246,6 +246,7 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
"structlog": latest,
# httpretty v1.0 drops python 2.7 support
"httpretty": "==0.9.7",
"gevent": latest,
},
)
],
30 changes: 27 additions & 3 deletions tests/internal/test_module.py
Original file line number Diff line number Diff line change
@@ -9,6 +9,21 @@
import tests.test_module


@pytest.fixture(autouse=True, scope="module")
def ensure_no_module_watchdog():
# DEV: The library might use the ModuleWatchdog and install it at a very
# early stage. This fixture ensures that the watchdog is not installed
# before the tests start.
was_installed = ModuleWatchdog.is_installed()
if was_installed:
ModuleWatchdog.uninstall()

yield

if was_installed:
ModuleWatchdog.install()
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved


@pytest.fixture
def module_watchdog():
ModuleWatchdog.install()
@@ -65,7 +80,10 @@ def test_import_origin_hook_for_module_not_yet_imported():
path = os.getenv("MODULE_ORIGIN")
hook = mock.Mock()

ModuleWatchdog.install()
try:
ModuleWatchdog.install()
except RuntimeError:
pass
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved

ModuleWatchdog.register_origin_hook(path, hook)

@@ -100,7 +118,10 @@ def test_import_module_hook_for_module_not_yet_imported():
name = "tests.test_module"
hook = mock.Mock()

ModuleWatchdog.install()
try:
ModuleWatchdog.install()
except RuntimeError:
pass

ModuleWatchdog.register_module_hook(name, hook)

@@ -136,7 +157,10 @@ def test_module_deleted():
path = os.getenv("MODULE_ORIGIN")
hook = mock.Mock()

ModuleWatchdog.install()
try:
ModuleWatchdog.install()
except RuntimeError:
pass

ModuleWatchdog.register_origin_hook(path, hook)
ModuleWatchdog.register_module_hook(name, hook)
57 changes: 57 additions & 0 deletions tests/tracer/test_forksafe.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import sys

import pytest
import six
@@ -285,3 +286,59 @@ def fn():
pid, status = os.waitpid(child, 0)
exit_code = os.WEXITSTATUS(status)
assert exit_code == 42


@pytest.mark.subprocess(
out="C\nT\nC\nT\nC\nT\n" if sys.platform == "darwin" else "C\nC\nC\nT\nT\nT\n",
P403n1x87 marked this conversation as resolved.
Show resolved Hide resolved
err=None,
)
def test_gevent_reinit_patch():
import os
import sys

from ddtrace.internal import forksafe
from ddtrace.internal.periodic import PeriodicService

class TestService(PeriodicService):
def __init__(self):
super(TestService, self).__init__(interval=1.0)

def periodic(self):
print("T")

service = TestService()
service.start()

def restart_service():
global service

service.stop()
service = TestService()
service.start()

forksafe.register(restart_service)

import gevent # noqa

def run_child():
global service

# We mimic what gunicorn does in child processes
gevent.monkey.patch_all()
gevent.hub.reinit()

print("C")

gevent.sleep(1.1)

service.stop()

def fork_workers(num):
for _ in range(num):
if os.fork() == 0:
run_child()
sys.exit(0)

fork_workers(3)

service.stop()