Skip to content

Commit

Permalink
fix(profiling): Use type() instead when extracting frames (#3716)
Browse files Browse the repository at this point in the history
When extract frame names, we should avoid accessing the `__class__` attribute as
it can be overwritten in the class implementation. In this particular instance,
the `SimpleLazyObject` class in django wraps `__class__` so when it is accessed,
it can cause the underlying lazy object to be evaluation unexpectedly. To avoid
this, use the `type()` builtin function which does cannot be overwritten and
will return the correct class.

Note that this does not work with old style classes but since dropping python 2
support, we only need to consider new style classes.
  • Loading branch information
Zylphrex authored Oct 29, 2024
1 parent bf40090 commit 02d0934
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
2 changes: 1 addition & 1 deletion sentry_sdk/profiler/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def get_frame_name(frame):
and co_varnames[0] == "self"
and "self" in frame.f_locals
):
for cls in frame.f_locals["self"].__class__.__mro__:
for cls in type(frame.f_locals["self"]).__mro__:
if name in cls.__dict__:
return "{}.{}".format(cls.__name__, name)
except (AttributeError, ValueError):
Expand Down
48 changes: 48 additions & 0 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import inspect
import json
import os
import re
import sys
import pytest
from functools import partial
from unittest.mock import patch
Expand All @@ -12,6 +14,7 @@
from django.core.management import execute_from_command_line
from django.db.utils import OperationalError, ProgrammingError, DataError
from django.http.request import RawPostDataException
from django.utils.functional import SimpleLazyObject

try:
from django.urls import reverse
Expand All @@ -29,6 +32,7 @@
)
from sentry_sdk.integrations.django.signals_handlers import _get_receiver_name
from sentry_sdk.integrations.executing import ExecutingIntegration
from sentry_sdk.profiler.utils import get_frame_name
from sentry_sdk.tracing import Span
from tests.conftest import unpack_werkzeug_response
from tests.integrations.django.myapp.wsgi import application
Expand Down Expand Up @@ -1295,3 +1299,47 @@ def test_ensures_no_spotlight_middleware_when_no_spotlight(
added = frozenset(settings.MIDDLEWARE) ^ original_middleware

assert "sentry_sdk.spotlight.SpotlightMiddleware" not in added


def test_get_frame_name_when_in_lazy_object():
allowed_to_init = False

class SimpleLazyObjectWrapper(SimpleLazyObject):
def unproxied_method(self):
"""
For testing purposes. We inject a method on the SimpleLazyObject
class so if python is executing this method, we should get
this class instead of the wrapped class and avoid evaluating
the wrapped object too early.
"""
return inspect.currentframe()

class GetFrame:
def __init__(self):
assert allowed_to_init, "GetFrame not permitted to initialize yet"

def proxied_method(self):
"""
For testing purposes. We add an proxied method on the instance
class so if python is executing this method, we should get
this class instead of the wrapper class.
"""
return inspect.currentframe()

instance = SimpleLazyObjectWrapper(lambda: GetFrame())

assert get_frame_name(instance.unproxied_method()) == (
"SimpleLazyObjectWrapper.unproxied_method"
if sys.version_info < (3, 11)
else "test_get_frame_name_when_in_lazy_object.<locals>.SimpleLazyObjectWrapper.unproxied_method"
)

# Now that we're about to access an instance method on the wrapped class,
# we should permit initializing it
allowed_to_init = True

assert get_frame_name(instance.proxied_method()) == (
"GetFrame.proxied_method"
if sys.version_info < (3, 11)
else "test_get_frame_name_when_in_lazy_object.<locals>.GetFrame.proxied_method"
)

0 comments on commit 02d0934

Please sign in to comment.