From 7cbd3bdaeaea2f11ea36849f1e55d9c195ae4b27 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 24 Oct 2024 22:19:29 +0200 Subject: [PATCH] opentelemetry-instrumentation: add unwrapping from dotted paths strings (#2919) --- CHANGELOG.md | 2 + .../opentelemetry/instrumentation/utils.py | 24 +++++- .../tests/test_utils.py | 83 +++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0079547454..1a345d939a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 73c000ee9c..a0d9ae18f9 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/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__) diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index d3807a1bdb..5ddd45d692 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/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))