Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion sentry_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,13 @@
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)

Check warning on line 202 in sentry_sdk/client.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/client.py#L201-L202

Added lines #L201 - L202 were not covered by tests

if function_type in (staticmethod, classmethod):
traced_function = staticmethod(traced_function)

Check warning on line 205 in sentry_sdk/client.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/client.py#L205

Added line #L205 was not covered by tests

setattr(class_obj, function_name, traced_function)

Check warning on line 207 in sentry_sdk/client.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/client.py#L207

Added line #L207 was not covered by tests
setattr(module_obj, class_name, class_obj)
logger.debug("Enabled tracing for %s", function_qualname)

Expand Down
6 changes: 3 additions & 3 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
transaction.set_http_status(response.status)
return response

Application._handle = sentry_app_handle # type: ignore[method-assign]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy was complaining about the type: ignore comments in this file, so I had to make changes here, even though the changes are unrelated to the rest of the PR

Application._handle = sentry_app_handle

Check warning on line 144 in sentry_sdk/integrations/aiohttp.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/aiohttp.py#L144

Added line #L144 was not covered by tests

old_urldispatcher_resolve = UrlDispatcher.resolve

Expand Down Expand Up @@ -173,7 +173,7 @@

return rv

UrlDispatcher.resolve = sentry_urldispatcher_resolve # type: ignore[method-assign]
UrlDispatcher.resolve = sentry_urldispatcher_resolve

Check warning on line 176 in sentry_sdk/integrations/aiohttp.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/aiohttp.py#L176

Added line #L176 was not covered by tests

old_client_session_init = ClientSession.__init__

Expand All @@ -190,7 +190,7 @@
kwargs["trace_configs"] = client_trace_configs
return old_client_session_init(*args, **kwargs)

ClientSession.__init__ = init # type: ignore[method-assign]
ClientSession.__init__ = init

Check warning on line 193 in sentry_sdk/integrations/aiohttp.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/aiohttp.py#L193

Added line #L193 was not covered by tests


def create_trace_config():
Expand Down
34 changes: 33 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
import socket
from threading import Thread
from contextlib import contextmanager

import pytest
import jsonschema
Expand All @@ -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
Expand All @@ -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"

Expand Down Expand Up @@ -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
60 changes: 59 additions & 1 deletion tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import pytest

from tests.conftest import patch_start_tracing_child

from sentry_sdk import (
Client,
push_scope,
Expand All @@ -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,
Expand Down Expand Up @@ -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
49 changes: 49 additions & 0 deletions tests/tracing/test_decorator_async_py3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
from unittest import mock
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file moved from tests/tracing/test_decorator_py3.py

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"
103 changes: 0 additions & 103 deletions tests/tracing/test_decorator_py3.py

This file was deleted.