fix(api): Fix tracing TypeError for static and class methods (getsentry#2559)

szokeasaurusrex · sentrivana · web-flow · commit 8bd2f4617895 · 2024-01-05T11:03:55.000+01:00
Fixes TypeError that occurred when static or class methods, which were passed in the `functions_to_trace` argument when initializing the SDK, were called on an instance. Fixes getsentryGH-2525 --------- Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -198,7 +198,13 @@ def _setup_instrumentation(self, functions_to_trace):
                     module_obj = import_module(module_name)
                     class_obj = getattr(module_obj, class_name)
                     function_obj = getattr(class_obj, function_name)
-                    setattr(class_obj, function_name, trace(function_obj))
+                    function_type = type(class_obj.__dict__[function_name])
+                    traced_function = trace(function_obj)
+
+                    if function_type in (staticmethod, classmethod):
+                        traced_function = staticmethod(traced_function)
+
+                    setattr(class_obj, function_name, traced_function)
                     setattr(module_obj, class_name, class_obj)
                     logger.debug("Enabled tracing for %s", function_qualname)
 
diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py
@@ -141,7 +141,7 @@ async def sentry_app_handle(self, request, *args, **kwargs):
                         transaction.set_http_status(response.status)
                         return response
 
-        Application._handle = sentry_app_handle  # type: ignore[method-assign]
+        Application._handle = sentry_app_handle
 
         old_urldispatcher_resolve = UrlDispatcher.resolve
 
@@ -173,7 +173,7 @@ async def sentry_urldispatcher_resolve(self, request):
 
             return rv
 
-        UrlDispatcher.resolve = sentry_urldispatcher_resolve  # type: ignore[method-assign]
+        UrlDispatcher.resolve = sentry_urldispatcher_resolve
 
         old_client_session_init = ClientSession.__init__
 
@@ -190,7 +190,7 @@ def init(*args, **kwargs):
             kwargs["trace_configs"] = client_trace_configs
             return old_client_session_init(*args, **kwargs)
 
-        ClientSession.__init__ = init  # type: ignore[method-assign]
+        ClientSession.__init__ = init
 
 
 def create_trace_config():
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -2,6 +2,7 @@
 import os
 import socket
 from threading import Thread
+from contextlib import contextmanager
 
 import pytest
 import jsonschema
@@ -27,8 +28,13 @@
     from http.server import BaseHTTPRequestHandler, HTTPServer
 
 
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
 import sentry_sdk
-from sentry_sdk._compat import iteritems, reraise, string_types
+from sentry_sdk._compat import iteritems, reraise, string_types, PY2
 from sentry_sdk.envelope import Envelope
 from sentry_sdk.integrations import _processed_integrations  # noqa: F401
 from sentry_sdk.profiler import teardown_profiler
@@ -37,6 +43,12 @@
 
 from tests import _warning_recorder, _warning_recorder_mgr
 
+from sentry_sdk._types import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from typing import Optional
+    from collections.abc import Iterator
+
 
 SENTRY_EVENT_SCHEMA = "./checkouts/data-schemas/relay/event.schema.json"
 
@@ -620,3 +632,23 @@ def werkzeug_set_cookie(client, servername, key, value):
         client.set_cookie(servername, key, value)
     except TypeError:
         client.set_cookie(key, value)
+
+
+@contextmanager
+def patch_start_tracing_child(fake_transaction_is_none=False):
+    # type: (bool) -> Iterator[Optional[mock.MagicMock]]
+    if not fake_transaction_is_none:
+        fake_transaction = mock.MagicMock()
+        fake_start_child = mock.MagicMock()
+        fake_transaction.start_child = fake_start_child
+    else:
+        fake_transaction = None
+        fake_start_child = None
+
+    version = "2" if PY2 else "3"
+
+    with mock.patch(
+        "sentry_sdk.tracing_utils_py%s.get_current_span" % version,
+        return_value=fake_transaction,
+    ):
+        yield fake_start_child
diff --git a/tests/test_basics.py b/tests/test_basics.py
@@ -5,6 +5,8 @@
 
 import pytest
 
+from tests.conftest import patch_start_tracing_child
+
 from sentry_sdk import (
     Client,
     push_scope,
@@ -17,7 +19,7 @@
     last_event_id,
     Hub,
 )
-from sentry_sdk._compat import reraise
+from sentry_sdk._compat import reraise, PY2
 from sentry_sdk.integrations import (
     _AUTO_ENABLING_INTEGRATIONS,
     Integration,
@@ -736,3 +738,59 @@ def test_multiple_setup_integrations_calls():
 
     second_call_return = setup_integrations([NoOpIntegration()], with_defaults=False)
     assert second_call_return == {NoOpIntegration.identifier: NoOpIntegration()}
+
+
+class TracingTestClass:
+    @staticmethod
+    def static(arg):
+        return arg
+
+    @classmethod
+    def class_(cls, arg):
+        return cls, arg
+
+
+def test_staticmethod_tracing(sentry_init):
+    test_staticmethod_name = "tests.test_basics.TracingTestClass.static"
+    if not PY2:
+        # Skip this check on Python 2 since __qualname__ is available in Python 3 only. Skipping is okay,
+        # since the assertion would be expected to fail in Python 3 if there is any problem.
+        assert (
+            ".".join(
+                [
+                    TracingTestClass.static.__module__,
+                    TracingTestClass.static.__qualname__,
+                ]
+            )
+            == test_staticmethod_name
+        ), "The test static method was moved or renamed. Please update the name accordingly"
+
+    sentry_init(functions_to_trace=[{"qualified_name": test_staticmethod_name}])
+
+    for instance_or_class in (TracingTestClass, TracingTestClass()):
+        with patch_start_tracing_child() as fake_start_child:
+            assert instance_or_class.static(1) == 1
+            assert fake_start_child.call_count == 1
+
+
+def test_classmethod_tracing(sentry_init):
+    test_classmethod_name = "tests.test_basics.TracingTestClass.class_"
+    if not PY2:
+        # Skip this check on Python 2 since __qualname__ is available in Python 3 only. Skipping is okay,
+        # since the assertion would be expected to fail in Python 3 if there is any problem.
+        assert (
+            ".".join(
+                [
+                    TracingTestClass.class_.__module__,
+                    TracingTestClass.class_.__qualname__,
+                ]
+            )
+            == test_classmethod_name
+        ), "The test class method was moved or renamed. Please update the name accordingly"
+
+    sentry_init(functions_to_trace=[{"qualified_name": test_classmethod_name}])
+
+    for instance_or_class in (TracingTestClass, TracingTestClass()):
+        with patch_start_tracing_child() as fake_start_child:
+            assert instance_or_class.class_(1) == (TracingTestClass, 1)
+            assert fake_start_child.call_count == 1
diff --git a/tests/tracing/test_decorator_async_py3.py b/tests/tracing/test_decorator_async_py3.py
@@ -0,0 +1,49 @@
+from unittest import mock
+import pytest
+import sys
+
+from tests.conftest import patch_start_tracing_child
+
+from sentry_sdk.tracing_utils_py3 import (
+    start_child_span_decorator as start_child_span_decorator_py3,
+)
+from sentry_sdk.utils import logger
+
+if sys.version_info < (3, 6):
+    pytest.skip("Async decorator only works on Python 3.6+", allow_module_level=True)
+
+
+async def my_async_example_function():
+    return "return_of_async_function"
+
+
+@pytest.mark.asyncio
+async def test_trace_decorator_async_py3():
+    with patch_start_tracing_child() as fake_start_child:
+        result = await my_async_example_function()
+        fake_start_child.assert_not_called()
+        assert result == "return_of_async_function"
+
+        result2 = await start_child_span_decorator_py3(my_async_example_function)()
+        fake_start_child.assert_called_once_with(
+            op="function",
+            description="test_decorator_async_py3.my_async_example_function",
+        )
+        assert result2 == "return_of_async_function"
+
+
+@pytest.mark.asyncio
+async def test_trace_decorator_async_py3_no_trx():
+    with patch_start_tracing_child(fake_transaction_is_none=True):
+        with mock.patch.object(logger, "warning", mock.Mock()) as fake_warning:
+            result = await my_async_example_function()
+            fake_warning.assert_not_called()
+            assert result == "return_of_async_function"
+
+            result2 = await start_child_span_decorator_py3(my_async_example_function)()
+            fake_warning.assert_called_once_with(
+                "Can not create a child span for %s. "
+                "Please start a Sentry transaction before calling this function.",
+                "test_decorator_async_py3.my_async_example_function",
+            )
+            assert result2 == "return_of_async_function"
diff --git a/tests/tracing/test_decorator_py3.py b/tests/tracing/test_decorator_py3.py
diff --git a/tests/tracing/test_decorator_sync.py b/tests/tracing/test_decorator_sync.py
