fix: don't import module in unwrap if not already imported (#4321)

* fix: don't import module in unwrap if not already

* Changelog

* Make test clearer

* Update CHANGELOG.md

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

---------

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

CHANGELOG.md

@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Fix intermittent `Core Contrib Test` CI failures caused by GitHub git CDN SHA propagation lag by installing core packages from the already-checked-out local copy instead of a second git clone
   ([#4305](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4305))
+- Don't import module in unwrap if not already imported
+  ([#4321](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4321))
 
 ### Breaking changes
 

opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py

@@ -14,6 +14,7 @@
 
 from __future__ import annotations
 
+import sys
 import urllib.parse
 from contextlib import contextmanager
 from importlib import import_module
@@ -103,6 +104,9 @@ def unwrap(obj: object, attr: str):
             raise ImportError(
                 f"Cannot parse '{obj}' as dotted import path"
             ) from exc
+        if module_path not in sys.modules:
+            # Was never imported, meaning it could never have been wrapped
+            return
         module = import_module(module_path)
         try:
             obj = getattr(module, class_name)

opentelemetry-instrumentation/tests/test_utils.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import sys
 import unittest
 from http import HTTPStatus
 
@@ -295,13 +296,21 @@ class UnwrapTestCase(unittest.TestCase):
         unwrap(WrappedClass, "method")
         self.assertFalse(isinstance(instance.method, BaseObjectProxy))
 
-    def test_raises_import_error_if_cannot_find_module(self):
+    def test_noop_if_module_not_imported(self):
+        # A module that exists but hasn't been imported, treated
+        # as no-op.
+        self.assertNotIn("email.generator", sys.modules)
+        unwrap("email.generator.BytesGenerator", "flatten")
+        self.assertNotIn("email.generator", sys.modules)
+
+    def test_noop_if_cannot_find_module(self):
         self._wrap_method()
         instance = WrappedClass()
         self.assertTrue(isinstance(instance.method, BaseObjectProxy))
 
-        with self.assertRaisesRegex(ImportError, "No module named 'does'"):
-            unwrap("does.not.exist.WrappedClass", "method")
+        # Treated same as an existing module that hasn't been imported,
+        # as a no-op.
+        unwrap("does.not.exist.WrappedClass", "method")
 
         unwrap(WrappedClass, "method")
         self.assertFalse(isinstance(instance.method, BaseObjectProxy))