From fad234470c5f5a64ce6a1b39f7a8d798e4ed0643 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Tue, 16 Aug 2022 14:18:00 +0100 Subject: [PATCH 1/3] chore: enable ModuleWatchdog globally This change installs an instance of the ModuleWatchodog for registering module import hooks. --- ddtrace/__init__.py | 7 ++++- tests/internal/test_module.py | 31 +++++++++++++++++++-- tests/profiling/collector/test_threading.py | 8 +++--- tests/profiling/collector/test_traceback.py | 10 +++---- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/ddtrace/__init__.py b/ddtrace/__init__.py index 0250cf32381..6c3c65b3f2b 100644 --- a/ddtrace/__init__.py +++ b/ddtrace/__init__.py @@ -1,4 +1,9 @@ -from ._logger import configure_ddtrace_logger +from ddtrace.internal.module import ModuleWatchdog + + +ModuleWatchdog.install() + +from ._logger import configure_ddtrace_logger # noqa: E402 # configure ddtrace logger before other modules log diff --git a/tests/internal/test_module.py b/tests/internal/test_module.py index e76ffa8ad35..e31d7df8e76 100644 --- a/tests/internal/test_module.py +++ b/tests/internal/test_module.py @@ -9,6 +9,22 @@ 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() + + try: + yield + finally: + if was_installed: + ModuleWatchdog.install() + + @pytest.fixture def module_watchdog(): ModuleWatchdog.install() @@ -65,7 +81,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 ModuleWatchdog.register_origin_hook(path, hook) @@ -100,7 +119,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 +158,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) diff --git a/tests/profiling/collector/test_threading.py b/tests/profiling/collector/test_threading.py index e2288ba1be1..8b56941f483 100644 --- a/tests/profiling/collector/test_threading.py +++ b/tests/profiling/collector/test_threading.py @@ -72,7 +72,7 @@ def test_lock_acquire_events(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 65, "test_lock_acquire_events") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 65, "test_lock_acquire_events") assert event.sampling_pct == 100 @@ -187,7 +187,7 @@ def test_lock_release_events(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 180, "test_lock_release_events") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 180, "test_lock_release_events") assert event.sampling_pct == 100 @@ -217,7 +217,7 @@ def play_with_lock(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 200, "play_with_lock") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 200, "play_with_lock") assert event.sampling_pct == 100 assert event.task_id == t.ident assert event.task_name == "foobar" @@ -234,7 +234,7 @@ def play_with_lock(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 201, "play_with_lock") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 201, "play_with_lock") assert event.sampling_pct == 100 assert event.task_id == t.ident assert event.task_name == "foobar" diff --git a/tests/profiling/collector/test_traceback.py b/tests/profiling/collector/test_traceback.py index ac9712522f6..f6d7fa3ff71 100644 --- a/tests/profiling/collector/test_traceback.py +++ b/tests/profiling/collector/test_traceback.py @@ -14,11 +14,9 @@ def test_check_traceback_to_frames(): exc_type, exc_value, traceback = sys.exc_info() frames, nframes = _traceback.traceback_to_frames(traceback, 10) assert nframes == 2 + + this_file = __file__.replace(".pyc", ".py") assert frames == [ - (__file__, 7, "_x"), - ( - __file__, - 15, - "test_check_traceback_to_frames", - ), + (this_file, 7, "_x"), + (this_file, 15, "test_check_traceback_to_frames"), ] From 80d0e8e1ae8801f045873b2235c39974c97221a1 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Tue, 16 Aug 2022 14:18:00 +0100 Subject: [PATCH 2/3] chore: enable ModuleWatchdog globally This change installs an instance of the ModuleWatchodog for registering module import hooks. --- ddtrace/__init__.py | 7 ++++- tests/internal/test_module.py | 31 +++++++++++++++++++-- tests/profiling/collector/test_threading.py | 8 +++--- tests/profiling/collector/test_traceback.py | 10 +++---- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/ddtrace/__init__.py b/ddtrace/__init__.py index 0250cf32381..6c3c65b3f2b 100644 --- a/ddtrace/__init__.py +++ b/ddtrace/__init__.py @@ -1,4 +1,9 @@ -from ._logger import configure_ddtrace_logger +from ddtrace.internal.module import ModuleWatchdog + + +ModuleWatchdog.install() + +from ._logger import configure_ddtrace_logger # noqa: E402 # configure ddtrace logger before other modules log diff --git a/tests/internal/test_module.py b/tests/internal/test_module.py index d9c0b11ecff..b78e5b40d39 100644 --- a/tests/internal/test_module.py +++ b/tests/internal/test_module.py @@ -9,6 +9,22 @@ 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() + + try: + yield + finally: + if was_installed: + ModuleWatchdog.install() + + @pytest.fixture def module_watchdog(): ModuleWatchdog.install() @@ -65,7 +81,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 ModuleWatchdog.register_origin_hook(path, hook) @@ -100,7 +119,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 +158,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) diff --git a/tests/profiling/collector/test_threading.py b/tests/profiling/collector/test_threading.py index e2288ba1be1..8b56941f483 100644 --- a/tests/profiling/collector/test_threading.py +++ b/tests/profiling/collector/test_threading.py @@ -72,7 +72,7 @@ def test_lock_acquire_events(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 65, "test_lock_acquire_events") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 65, "test_lock_acquire_events") assert event.sampling_pct == 100 @@ -187,7 +187,7 @@ def test_lock_release_events(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 180, "test_lock_release_events") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 180, "test_lock_release_events") assert event.sampling_pct == 100 @@ -217,7 +217,7 @@ def play_with_lock(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 200, "play_with_lock") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 200, "play_with_lock") assert event.sampling_pct == 100 assert event.task_id == t.ident assert event.task_name == "foobar" @@ -234,7 +234,7 @@ def play_with_lock(): # It's called through pytest so I'm sure it's gonna be that long, right? assert len(event.frames) > 3 assert event.nframes > 3 - assert event.frames[0] == (__file__, 201, "play_with_lock") + assert event.frames[0] == (__file__.replace(".pyc", ".py"), 201, "play_with_lock") assert event.sampling_pct == 100 assert event.task_id == t.ident assert event.task_name == "foobar" diff --git a/tests/profiling/collector/test_traceback.py b/tests/profiling/collector/test_traceback.py index ac9712522f6..f6d7fa3ff71 100644 --- a/tests/profiling/collector/test_traceback.py +++ b/tests/profiling/collector/test_traceback.py @@ -14,11 +14,9 @@ def test_check_traceback_to_frames(): exc_type, exc_value, traceback = sys.exc_info() frames, nframes = _traceback.traceback_to_frames(traceback, 10) assert nframes == 2 + + this_file = __file__.replace(".pyc", ".py") assert frames == [ - (__file__, 7, "_x"), - ( - __file__, - 15, - "test_check_traceback_to_frames", - ), + (this_file, 7, "_x"), + (this_file, 15, "test_check_traceback_to_frames"), ] From d944db2ecf10674caf31fc4100f78cfd82bd9879 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Wed, 17 Aug 2022 10:36:50 +0100 Subject: [PATCH 3/3] clean up tests --- tests/internal/test_module.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/internal/test_module.py b/tests/internal/test_module.py index b78e5b40d39..855f571339d 100644 --- a/tests/internal/test_module.py +++ b/tests/internal/test_module.py @@ -81,11 +81,6 @@ def test_import_origin_hook_for_module_not_yet_imported(): path = os.getenv("MODULE_ORIGIN") hook = mock.Mock() - try: - ModuleWatchdog.install() - except RuntimeError: - pass - ModuleWatchdog.register_origin_hook(path, hook) hook.assert_not_called() @@ -119,11 +114,6 @@ def test_import_module_hook_for_module_not_yet_imported(): name = "tests.test_module" hook = mock.Mock() - try: - ModuleWatchdog.install() - except RuntimeError: - pass - ModuleWatchdog.register_module_hook(name, hook) hook.assert_not_called() @@ -158,11 +148,6 @@ def test_module_deleted(): path = os.getenv("MODULE_ORIGIN") hook = mock.Mock() - try: - ModuleWatchdog.install() - except RuntimeError: - pass - ModuleWatchdog.register_origin_hook(path, hook) ModuleWatchdog.register_module_hook(name, hook) @@ -352,3 +337,5 @@ def test_module_watchdog_dict_shallow_copy(): # Ensure that they match assert original_modules == new_modules + + ModuleWatchdog.uninstall()