chore: remove some crash-by-design behaviors from internal/module.py (#…

…10234)

This change improves the auto-injection user experience by removing some
crash-by-design behaviors from `internal/module.py`. They're replaced
either with idempotency checks or warn+return logic.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

ddtrace/internal/module.py

@@ -344,7 +344,8 @@ def _remove_from_meta_path(cls) -> None:
         i = cls._find_in_meta_path()
 
         if i is None:
-            raise RuntimeError("%s is not installed" % cls.__name__)
+            log.warning("%s is not installed", cls.__name__)
+            return
 
         sys.meta_path.pop(i)
 
@@ -412,15 +413,14 @@ def find_spec(
 
     @classmethod
     def _check_installed(cls) -> None:
-        if not cls.is_installed():
-            raise RuntimeError("%s is not installed" % cls.__name__)
+        if cls.is_installed():
+            return
 
     @classmethod
     def install(cls) -> None:
         """Install the module watchdog."""
         if cls.is_installed():
-            raise RuntimeError("%s is already installed" % cls.__name__)
-
+            return
         cls._instance = cls()
         cls._instance._add_to_meta_path()
         log.debug("%s installed", cls)
@@ -555,7 +555,8 @@ def register_origin_hook(cls, origin: Path, hook: ModuleHookType) -> None:
         # change, then thread-safety might become a concern.
         resolved_path = _resolve(origin)
         if resolved_path is None:
-            raise ValueError("Cannot resolve module origin %s" % origin)
+            log.warning("Cannot resolve module origin %s", origin)
+            return
 
         path = str(resolved_path)
 
@@ -588,13 +589,15 @@ def unregister_origin_hook(cls, origin: Path, hook: ModuleHookType) -> None:
 
         resolved_path = _resolve(origin)
         if resolved_path is None:
-            raise ValueError("Module origin %s cannot be resolved", origin)
+            log.warning("Module origin %s cannot be resolved", origin)
+            return
 
         path = str(resolved_path)
 
         instance = t.cast(ModuleWatchdog, cls._instance)
         if path not in instance._hook_map:
-            raise ValueError("No hooks registered for origin %s" % origin)
+            log.warning("No hooks registered for origin %s", origin)
+            return
 
         try:
             if path in instance._hook_map:
@@ -603,7 +606,8 @@ def unregister_origin_hook(cls, origin: Path, hook: ModuleHookType) -> None:
                 if not hooks:
                     del instance._hook_map[path]
         except ValueError:
-            raise ValueError("Hook %r not registered for origin %s" % (hook, origin))
+            log.warning("Hook %r not registered for origin %s", hook, origin)
+            return
 
     @classmethod
     def register_module_hook(cls, module: str, hook: ModuleHookType) -> None:
@@ -636,7 +640,8 @@ def unregister_module_hook(cls, module: str, hook: ModuleHookType) -> None:
 
         instance = t.cast(ModuleWatchdog, cls._instance)
         if module not in instance._hook_map:
-            raise ValueError("No hooks registered for module %s" % module)
+            log.warning("No hooks registered for module %s", module)
+            return
 
         try:
             if module in instance._hook_map:
@@ -645,7 +650,8 @@ def unregister_module_hook(cls, module: str, hook: ModuleHookType) -> None:
                 if not hooks:
                     del instance._hook_map[module]
         except ValueError:
-            raise ValueError("Hook %r not registered for module %r" % (hook, module))
+            log.warning("Hook %r not registered for module %r", hook, module)
+            return
 
     @classmethod
     def after_module_imported(cls, module: str) -> t.Callable[[ModuleHookType], None]:

tests/internal/test_module.py

@@ -97,14 +97,6 @@ def test_after_module_imported_decorator(module_watchdog):
     hook.assert_called_once_with(module)
 
 
-def test_register_hook_without_install():
-    with pytest.raises(RuntimeError):
-        ModuleWatchdog.register_origin_hook(__file__, mock.Mock())
-
-    with pytest.raises(RuntimeError):
-        ModuleWatchdog.register_module_hook(__name__, mock.Mock())
-
-
 @pytest.mark.subprocess(env=dict(MODULE_ORIGIN=str(origin(tests.test_module))))
 def test_import_origin_hook_for_module_not_yet_imported():
     import os
@@ -238,8 +230,7 @@ def test_module_unregister_origin_hook(module_watchdog):
 
     assert module_watchdog._instance._hook_map[str(path)] == []
 
-    with pytest.raises(ValueError):
-        module_watchdog.unregister_origin_hook(path, hook)
+    module_watchdog.unregister_origin_hook(path, hook)
 
 
 def test_module_unregister_module_hook(module_watchdog):
@@ -257,21 +248,18 @@ def test_module_unregister_module_hook(module_watchdog):
     module_watchdog.unregister_module_hook(module, hook)
     assert module_watchdog._instance._hook_map[module] == []
 
-    with pytest.raises(ValueError):
-        module_watchdog.unregister_module_hook(module, hook)
+    module_watchdog.unregister_module_hook(module, hook)
 
 
 def test_module_watchdog_multiple_install():
     ModuleWatchdog.install()
-    with pytest.raises(RuntimeError):
-        ModuleWatchdog.install()
-
+    assert ModuleWatchdog.is_installed()
+    ModuleWatchdog.install()
     assert ModuleWatchdog.is_installed()
 
     ModuleWatchdog.uninstall()
-    with pytest.raises(RuntimeError):
-        ModuleWatchdog.uninstall()
-
+    assert not ModuleWatchdog.is_installed()
+    ModuleWatchdog.uninstall()
     assert not ModuleWatchdog.is_installed()