From 9f9f01246c357f76e4b9ceb6d3806b373d4d190b Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 21 Oct 2024 15:40:33 +0200 Subject: [PATCH] opentelemetry-instrumentation: add unwrapping from dotted paths strings Make it possible to express object to unwrap as dotted module paths strings. This helps in avoiding side effects or race conditions with other instrumentations if we do importing too early. While at it add tests also for current functionality. --- .../opentelemetry/instrumentation/utils.py | 20 ++++- .../tests/test_utils.py | 80 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 73c000ee9c..5e31109cf0 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -14,6 +14,7 @@ import urllib.parse from contextlib import contextmanager +from importlib import import_module from re import escape, sub from typing import Dict, Iterable, Sequence @@ -83,10 +84,27 @@ def http_status_to_status_code( def unwrap(obj, 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..f54a9ed87a 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,72 @@ def test_suppress_http_instrumentation_key(self): self.assertTrue(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)) self.assertIsNone(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)) + + @staticmethod + def _wrap_method(): + return wrap_function_wrapper( + WrappedClass, "method", WrappedClass.wrapper_method + ) + + def test_unwrap_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_unwrap_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_unwrap_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_unwrap_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_unwrap_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)) + + def test_does_nothing_if_cannot_find_attribute(self): + instance = WrappedClass() + unwrap(instance, "method_not_found") + + def test_unwrap_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))