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

feat(integrations): add django signals_denylist to filter signals that are attached to by signals_span #2758

Merged
merged 4 commits into from
Apr 9, 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
5 changes: 4 additions & 1 deletion sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,17 @@ class DjangoIntegration(Integration):
middleware_spans = None
signals_spans = None
cache_spans = None
signals_denylist = [] # type: list[signals.Signal]

def __init__(
self,
transaction_style="url",
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)"
Expand All @@ -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():
Expand Down
6 changes: 5 additions & 1 deletion sentry_sdk/integrations/django/signals_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
15 changes: 15 additions & 0 deletions tests/integrations/django/myapp/signals.py
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 5 additions & 0 deletions tests/integrations/django/myapp/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/integrations/django/myapp/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
42 changes: 42 additions & 0 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down