Merge branch 'main' into anshul-fix-1781-alt

Author: lzchen

CHANGELOG.md

@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2082](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2082))
 - `opentelemetry-instrumentation-redis` Add additional attributes for methods create_index and search, rename those spans
   ([#2635](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2635))
+- `opentelemetry-instrumentation` Add support for string based dotted module paths in unwrap
+  ([#2919](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2919))
 
 ### Fixed
 
@@ -80,6 +82,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2753](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2753))
 - `opentelemetry-instrumentation-grpc` Fix grpc supported version
   ([#2845](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2845))
+- `opentelemetry-instrumentation-asyncio` fix `AttributeError` in
+  `AsyncioInstrumentor.trace_to_thread` when `func` is a `functools.partial` instance
+  ([#2911](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2911))
 
 ## Version 1.26.0/0.47b0 (2024-07-23)
 

instrumentation/opentelemetry-instrumentation-asyncio/src/opentelemetry/instrumentation/asyncio/__init__.py

@@ -78,6 +78,7 @@ def func():
 """
 
 import asyncio
+import functools
 import sys
 from asyncio import futures
 from timeit import default_timer
@@ -231,14 +232,15 @@ def wrap_taskgroup_create_task(method, instance, args, kwargs) -> None:
     def trace_to_thread(self, func: callable):
         """Trace a function."""
         start = default_timer()
+        func_name = getattr(func, "__name__", None)
+        if func_name is None and isinstance(func, functools.partial):
+            func_name = func.func.__name__
         span = (
-            self._tracer.start_span(
-                f"{ASYNCIO_PREFIX} to_thread-" + func.__name__
-            )
-            if func.__name__ in self._to_thread_name_to_trace
+            self._tracer.start_span(f"{ASYNCIO_PREFIX} to_thread-" + func_name)
+            if func_name in self._to_thread_name_to_trace
             else None
         )
-        attr = {"type": "to_thread", "name": func.__name__}
+        attr = {"type": "to_thread", "name": func_name}
         exception = None
         try:
             attr["state"] = "finished"

instrumentation/opentelemetry-instrumentation-asyncio/tests/test_asyncio_to_thread.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import asyncio
+import functools
 import sys
 from unittest import skipIf
 from unittest.mock import patch
@@ -72,3 +73,37 @@ async def to_thread():
                 for point in metric.data.data_points:
                     self.assertEqual(point.attributes["type"], "to_thread")
                     self.assertEqual(point.attributes["name"], "multiply")
+
+    @skipIf(
+        sys.version_info < (3, 9), "to_thread is only available in Python 3.9+"
+    )
+    def test_to_thread_partial_func(self):
+        def multiply(x, y):
+            return x * y
+
+        double = functools.partial(multiply, 2)
+
+        async def to_thread():
+            result = await asyncio.to_thread(double, 3)
+            assert result == 6
+
+        with self._tracer.start_as_current_span("root"):
+            asyncio.run(to_thread())
+        spans = self.memory_exporter.get_finished_spans()
+
+        self.assertEqual(len(spans), 2)
+        assert spans[0].name == "asyncio to_thread-multiply"
+        for metric in (
+            self.memory_metrics_reader.get_metrics_data()
+            .resource_metrics[0]
+            .scope_metrics[0]
+            .metrics
+        ):
+            if metric.name == "asyncio.process.duration":
+                for point in metric.data.data_points:
+                    self.assertEqual(point.attributes["type"], "to_thread")
+                    self.assertEqual(point.attributes["name"], "multiply")
+            if metric.name == "asyncio.process.created":
+                for point in metric.data.data_points:
+                    self.assertEqual(point.attributes["type"], "to_thread")
+                    self.assertEqual(point.attributes["name"], "multiply")

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

@@ -14,8 +14,9 @@
 
 import urllib.parse
 from contextlib import contextmanager
+from importlib import import_module
 from re import escape, sub
-from typing import Dict, Iterable, Sequence
+from typing import Dict, Iterable, Sequence, Union
 
 from wrapt import ObjectProxy
 
@@ -80,13 +81,30 @@ def http_status_to_status_code(
     return StatusCode.ERROR
 
 
-def unwrap(obj, attr: str):
+def unwrap(obj: Union[object, str], attr: str):
     """Given a function that was wrapped by wrapt.wrap_function_wrapper, unwrap it
 
+    The object containing the function to unwrap may be passed as dotted module path string.
+
     Args:
-        obj: Object that holds a reference to the wrapped function
+        obj: Object that holds a reference to the wrapped function or dotted import path as string
         attr (str): Name of the wrapped function
     """
+    if isinstance(obj, str):
+        try:
+            module_path, class_name = obj.rsplit(".", 1)
+        except ValueError as exc:
+            raise ImportError(
+                f"Cannot parse '{obj}' as dotted import path"
+            ) from exc
+        module = import_module(module_path)
+        try:
+            obj = getattr(module, class_name)
+        except AttributeError as exc:
+            raise ImportError(
+                f"Cannot import '{class_name}' from '{module}'"
+            ) from exc
+
     func = getattr(obj, attr, None)
     if func and isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"):
         setattr(obj, attr, func.__wrapped__)

opentelemetry-instrumentation/tests/test_utils.py

@@ -15,6 +15,8 @@
 import unittest
 from http import HTTPStatus
 
+from wrapt import ObjectProxy, wrap_function_wrapper
+
 from opentelemetry.context import (
     _SUPPRESS_HTTP_INSTRUMENTATION_KEY,
     _SUPPRESS_INSTRUMENTATION_KEY,
@@ -29,10 +31,19 @@
     is_instrumentation_enabled,
     suppress_http_instrumentation,
     suppress_instrumentation,
+    unwrap,
 )
 from opentelemetry.trace import StatusCode
 
 
+class WrappedClass:
+    def method(self):
+        pass
+
+    def wrapper_method(self):
+        pass
+
+
 class TestUtils(unittest.TestCase):
     # See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status
     def test_http_status_to_status_code(self):
@@ -240,3 +251,75 @@ def test_suppress_http_instrumentation_key(self):
             self.assertTrue(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY))
 
         self.assertIsNone(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY))
+
+
+class UnwrapTestCase(unittest.TestCase):
+    @staticmethod
+    def _wrap_method():
+        return wrap_function_wrapper(
+            WrappedClass, "method", WrappedClass.wrapper_method
+        )
+
+    def test_can_unwrap_object_attribute(self):
+        self._wrap_method()
+        instance = WrappedClass()
+        self.assertTrue(isinstance(instance.method, ObjectProxy))
+
+        unwrap(WrappedClass, "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+
+    def test_can_unwrap_object_attribute_as_string(self):
+        self._wrap_method()
+        instance = WrappedClass()
+        self.assertTrue(isinstance(instance.method, ObjectProxy))
+
+        unwrap("tests.test_utils.WrappedClass", "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+
+    def test_raises_import_error_if_path_not_well_formed(self):
+        self._wrap_method()
+        instance = WrappedClass()
+        self.assertTrue(isinstance(instance.method, ObjectProxy))
+
+        with self.assertRaisesRegex(
+            ImportError, "Cannot parse '' as dotted import path"
+        ):
+            unwrap("", "method")
+
+        unwrap(WrappedClass, "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+
+    def test_raises_import_error_if_cannot_find_module(self):
+        self._wrap_method()
+        instance = WrappedClass()
+        self.assertTrue(isinstance(instance.method, ObjectProxy))
+
+        with self.assertRaisesRegex(ImportError, "No module named 'does'"):
+            unwrap("does.not.exist.WrappedClass", "method")
+
+        unwrap(WrappedClass, "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+
+    def test_raises_import_error_if_cannot_find_object(self):
+        self._wrap_method()
+        instance = WrappedClass()
+        self.assertTrue(isinstance(instance.method, ObjectProxy))
+
+        with self.assertRaisesRegex(
+            ImportError, "Cannot import 'NotWrappedClass' from"
+        ):
+            unwrap("tests.test_utils.NotWrappedClass", "method")
+
+        unwrap(WrappedClass, "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+
+    # pylint: disable=no-self-use
+    def test_does_nothing_if_cannot_find_attribute(self):
+        instance = WrappedClass()
+        unwrap(instance, "method_not_found")
+
+    def test_does_nothing_if_attribute_is_not_from_wrapt(self):
+        instance = WrappedClass()
+        self.assertFalse(isinstance(instance.method, ObjectProxy))
+        unwrap(WrappedClass, "method")
+        self.assertFalse(isinstance(instance.method, ObjectProxy))