address Thomas' feedback

Author: vlad-scherbich

ddtrace/profiling/collector/_lock.pyx

@@ -4,7 +4,6 @@
 # would cause Cython compilation errors since they aren't valid C types.
 
 import _thread
-import logging
 import os.path
 import sys
 import time
@@ -30,9 +29,10 @@ from ddtrace.profiling.collector._task cimport (
     get_task as _c_get_task,
     initialize_gevent_support as _c_initialize_gevent_support,
 )
+from ddtrace.internal.logger import get_logger
 
 
-LOG = logging.getLogger(__name__)
+log = get_logger(__name__)
 
 
 ACQUIRE_RELEASE_CO_NAMES: frozenset[str] = frozenset(["_acquire", "_release"])
@@ -122,7 +122,7 @@ class _ProfiledLock:
         # and should not generate profile samples to avoid double-counting
         self.is_internal: bool = is_internal
 
-    # gevent compatibility — gevent's _patch_existing_locks() calls
+    # gevent compatibility — gevent's _patch_existing_locks calls
     # type(threading.RLock()) to determine the RLock type, then scans gc.get_objects()
     # for instances of that type. When our wrapper is on threading.RLock, the type is
     # _ProfiledLock, so ALL profiled lock instances match. gevent then checks each for
@@ -540,14 +540,16 @@ class LockCollector(collector.CaptureSamplerCollector):
         patched_module_id: int = id(self.MODULE)
 
         def _on_module_reimport(new_module: ModuleType) -> None:
-            nonlocal patched_module_id
+            nonlocal patched_module_
             if id(new_module) == patched_module_id:
                 return
-            LOG.warning(
-                "%s: target module %r was re-imported (id %#x -> %#x); "
-                "re-applying lock profiling patches. "
-                "This typically happens when gevent is installed and cleanup_loaded_modules() "
-                "discards the previously-patched module from sys.modules.",
+            log.warning(
+                (
+                    "%s: target module %r was re-imported (id %#x -> %#x); "
+                    "re-applying lock profiling patches. "
+                    "This typically happens when gevent is installed and cleanup_loaded_modules() "
+                    "discards the previously-patched module from sys.modules."
+                ),
                 type(self).__name__,
                 module_name,
                 patched_module_id,
@@ -568,17 +570,14 @@ class LockCollector(collector.CaptureSamplerCollector):
         super(LockCollector, self)._stop_service()  # type: ignore[safe-super]
         self.unpatch()
         if self._reimport_hook is not None:
-            try:
-                ModuleWatchdog.unregister_module_hook(self.MODULE.__name__, self._reimport_hook)
-            except Exception:  # nosec B110
-                pass
+            ModuleWatchdog.unregister_module_hook(self.MODULE.__name__, self._reimport_hook)
             self._reimport_hook = None
 
     def patch(self) -> None:
         """Patch the module for tracking lock allocation."""
         original_lock: Callable[..., Any] = self._get_patch_target()
         if isinstance(original_lock, _LockAllocatorWrapper):
-            LOG.debug(
+            log.debug(
                 "%s: %s.%s is already patched, skipping to avoid double-wrapping.",
                 type(self).__name__,
                 self.MODULE.__name__,

ddtrace/profiling/profiler.py

@@ -347,10 +347,7 @@ def _stop_service(self, flush: bool = True, join: bool = True) -> None:
         # Prevent doing more initialisation now that we are shutting down.
         if self._collectors_on_import:
             for module, hook in self._collectors_on_import:
-                try:
-                    ModuleWatchdog.unregister_module_hook(module, hook)
-                except (ValueError, Exception):
-                    pass
+                ModuleWatchdog.unregister_module_hook(module, hook)
             self._collectors_on_import = None
 
         if self._scheduler is not None:

releasenotes/notes/fix-lock-profiler-module-cloning-4c524e7d7d5c5967.yaml

@@ -2,8 +2,4 @@
 fixes:
   - |
     profiling: Fixes an issue where the lock profiler silently stopped capturing
-    lock events when running under ``ddtrace-run`` with gevent installed. The
-    module cloning mechanism discarded the patched ``threading`` and ``asyncio``
-    modules, causing user code to import fresh, unpatched modules. The lock
-    profiler now re-applies patches automatically when target modules are
-    re-imported.
+    lock events when running under ``ddtrace-run`` with `gevent` installed.

tests/profiling/collector/test_threading.py

@@ -194,6 +194,9 @@ def test_patch():
     assert collector._original_lock == threading.Lock
 
 
+# Run in a subprocess: pops threading from sys.modules to simulate gevent's
+# cleanup_loaded_modules(), which would corrupt ModuleWatchdog state in-process.
+@pytest.mark.subprocess()
 def test_lock_patching_survives_module_reimport():
     """Test that lock patches are re-applied when threading is re-imported.
 
@@ -202,56 +205,63 @@ def test_lock_patching_survives_module_reimport():
     fresh, unpatched module. Without the fix, user code would get native locks.
     """
     import importlib
+    import sys
+    import threading
+
+    from ddtrace.profiling.collector._lock import LockAllocatorWrapper
+    from ddtrace.profiling.collector._lock import _ProfiledLock
+    from ddtrace.profiling.collector.threading import ThreadingLockCollector
 
     collector = ThreadingLockCollector()
     collector.start()
 
-    # Verify patching works on the current module
     assert isinstance(threading.Lock, LockAllocatorWrapper)
 
     # Simulate cleanup_loaded_modules(): remove threading from sys.modules
     old_threading = sys.modules.pop("threading")
-    try:
-        # Re-import threading — this is what user code does after cloning
-        new_threading = importlib.import_module("threading")
-        assert new_threading is not old_threading, "Should be a different module object"
+    # Re-import threading — this is what user code does after cloning
+    new_threading = importlib.import_module("threading")
+    assert new_threading is not old_threading, "Should be a different module object"
+
+    # The fix: patches should have been re-applied to the new module
+    assert isinstance(new_threading.Lock, LockAllocatorWrapper), (
+        "Lock on re-imported threading module should be patched. "
+        "This fails without the ModuleWatchdog re-import hook fix."
+    )
 
-        # The fix: patches should have been re-applied to the new module
-        assert isinstance(new_threading.Lock, LockAllocatorWrapper), (
-            "Lock on re-imported threading module should be patched. "
-            "This fails without the ModuleWatchdog re-import hook fix."
-        )
+    # User-created locks from the new module should be profiled
+    lock = new_threading.Lock()
+    assert isinstance(lock, _ProfiledLock), "Locks created from re-imported threading should be profiled"
 
-        # User-created locks from the new module should be profiled
-        lock = new_threading.Lock()
-        assert isinstance(lock, _ProfiledLock), "Locks created from re-imported threading should be profiled"
-    finally:
-        # Restore original module to not break other tests
-        sys.modules["threading"] = old_threading
-        collector.stop()
+    collector.stop()
 
 
+# Run in a subprocess: pops threading from sys.modules to simulate gevent's
+# cleanup_loaded_modules(), which would corrupt ModuleWatchdog state in-process.
+@pytest.mark.subprocess()
 def test_lock_unpatch_after_module_reimport():
     """Test that stop/unpatch works correctly after module has been swapped."""
     import importlib
+    import sys
+    import threading
+
+    from ddtrace.profiling.collector._lock import LockAllocatorWrapper
+    from ddtrace.profiling.collector.threading import ThreadingLockCollector
 
     collector = ThreadingLockCollector()
     collector.start()
     assert isinstance(threading.Lock, LockAllocatorWrapper)
 
     # Simulate module swap
-    old_threading = sys.modules.pop("threading")
-    try:
-        new_threading = importlib.import_module("threading")
-        assert isinstance(new_threading.Lock, LockAllocatorWrapper)
+    sys.modules.pop("threading")
+    new_threading = importlib.import_module("threading")
+    assert isinstance(new_threading.Lock, LockAllocatorWrapper)
 
-        # Stop should unpatch the current (new) module
-        collector.stop()
-        assert not isinstance(new_threading.Lock, LockAllocatorWrapper), (
-            "After stop, the new threading module should be unpatched"
-        )
-    finally:
-        sys.modules["threading"] = old_threading
+    # Stop should unpatch the current (new) module
+    collector.stop()
+    assert not isinstance(new_threading.Lock, LockAllocatorWrapper), (
+        "After stop, the new threading module should be unpatched"
+    )
 
 
 @pytest.mark.parametrize(