From 11e1f9aa1f80e71766f10739876db992ef1eb70d Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 10 Apr 2024 01:38:52 +1000 Subject: [PATCH] feat(integrations): Add django signals_denylist to filter signals that are attached to by signals_span (#2758) --- sentry_sdk/integrations/django/__init__.py | 5 ++- .../integrations/django/signals_handlers.py | 6 ++- tests/integrations/django/myapp/signals.py | 15 +++++++ tests/integrations/django/myapp/urls.py | 5 +++ tests/integrations/django/myapp/views.py | 12 ++++++ tests/integrations/django/test_basic.py | 42 +++++++++++++++++++ 6 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 tests/integrations/django/myapp/signals.py diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 98834a4693..a38674f09d 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -114,6 +114,7 @@ class DjangoIntegration(Integration): middleware_spans = None signals_spans = None cache_spans = None + signals_denylist = [] # type: list[signals.Signal] def __init__( self, @@ -121,8 +122,9 @@ def __init__( middleware_spans=True, signals_spans=True, cache_spans=False, + signals_denylist=None, ): - # type: (str, bool, bool, bool) -> None + # type: (str, bool, bool, bool, Optional[list[signals.Signal]]) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( "Invalid value for transaction_style: %s (must be in %s)" @@ -132,6 +134,7 @@ def __init__( self.middleware_spans = middleware_spans self.signals_spans = signals_spans self.cache_spans = cache_spans + self.signals_denylist = signals_denylist or [] @staticmethod def setup_once(): diff --git a/sentry_sdk/integrations/django/signals_handlers.py b/sentry_sdk/integrations/django/signals_handlers.py index 097a56c8aa..3d1aadab1f 100644 --- a/sentry_sdk/integrations/django/signals_handlers.py +++ b/sentry_sdk/integrations/django/signals_handlers.py @@ -78,7 +78,11 @@ def wrapper(*args, **kwargs): return wrapper integration = hub.get_integration(DjangoIntegration) - if integration and integration.signals_spans: + if ( + integration + and integration.signals_spans + and self not in integration.signals_denylist + ): for idx, receiver in enumerate(sync_receivers): sync_receivers[idx] = sentry_sync_receiver_wrapper(receiver) diff --git a/tests/integrations/django/myapp/signals.py b/tests/integrations/django/myapp/signals.py new file mode 100644 index 0000000000..3dab92b8d9 --- /dev/null +++ b/tests/integrations/django/myapp/signals.py @@ -0,0 +1,15 @@ +from django.core import signals +from django.dispatch import receiver + +myapp_custom_signal = signals.Signal() +myapp_custom_signal_silenced = signals.Signal() + + +@receiver(myapp_custom_signal) +def signal_handler(sender, **kwargs): + assert sender == "hello" + + +@receiver(myapp_custom_signal_silenced) +def signal_handler_silenced(sender, **kwargs): + assert sender == "hello" diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 92621b07a2..672a9b15ae 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -76,6 +76,11 @@ def path(path, *args, **kwargs): name="csrf_hello_not_exempt", ), path("sync/thread_ids", views.thread_ids_sync, name="thread_ids_sync"), + path( + "send-myapp-custom-signal", + views.send_myapp_custom_signal, + name="send_myapp_custom_signal", + ), ] # async views diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 193147003b..294895430b 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -14,6 +14,11 @@ from django.views.decorators.csrf import csrf_exempt from django.views.generic import ListView +from tests.integrations.django.myapp.signals import ( + myapp_custom_signal, + myapp_custom_signal_silenced, +) + try: from rest_framework.decorators import api_view from rest_framework.response import Response @@ -253,3 +258,10 @@ def thread_ids_sync(*args, **kwargs): my_async_view = None thread_ids_async = None post_echo_async = None + + +@csrf_exempt +def send_myapp_custom_signal(request): + myapp_custom_signal.send(sender="hello") + myapp_custom_signal_silenced.send(sender="hello") + return HttpResponse("ok") diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 8c01c71830..1efe4be278 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -29,6 +29,7 @@ from sentry_sdk.tracing import Span from tests.conftest import ApproxDict, unpack_werkzeug_response from tests.integrations.django.myapp.wsgi import application +from tests.integrations.django.myapp.signals import myapp_custom_signal_silenced from tests.integrations.django.utils import pytest_mark_django_db_decorator DJANGO_VERSION = DJANGO_VERSION[:2] @@ -1035,6 +1036,47 @@ def test_signals_spans_disabled(sentry_init, client, capture_events): assert not transaction["spans"] +EXPECTED_SIGNALS_SPANS_FILTERED = """\ +- op="http.server": description=null + - op="event.django": description="django.db.reset_queries" + - op="event.django": description="django.db.close_old_connections" + - op="event.django": description="tests.integrations.django.myapp.signals.signal_handler"\ +""" + + +def test_signals_spans_filtering(sentry_init, client, capture_events, render_span_tree): + sentry_init( + integrations=[ + DjangoIntegration( + middleware_spans=False, + signals_denylist=[ + myapp_custom_signal_silenced, + ], + ), + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("send_myapp_custom_signal")) + + (transaction,) = events + + assert render_span_tree(transaction) == EXPECTED_SIGNALS_SPANS_FILTERED + + assert transaction["spans"][0]["op"] == "event.django" + assert transaction["spans"][0]["description"] == "django.db.reset_queries" + + assert transaction["spans"][1]["op"] == "event.django" + assert transaction["spans"][1]["description"] == "django.db.close_old_connections" + + assert transaction["spans"][2]["op"] == "event.django" + assert ( + transaction["spans"][2]["description"] + == "tests.integrations.django.myapp.signals.signal_handler" + ) + + def test_csrf(sentry_init, client): """ Assert that CSRF view decorator works even with the view wrapped in our own