From de6fd149ae37aa315cd03026502b5e11b9a94063 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 2 Oct 2024 14:41:08 +0100 Subject: [PATCH 1/5] feat(django): Add SpotlightMiddleware when Spotlight is enabled This patch replaces Django's debug error page with Spotlight when it is enabled and is running. It bails when `DEBUG` is `False` or when it cannot connect to the Spotlight web server. --- sentry_sdk/client.py | 4 +++- sentry_sdk/spotlight.py | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 0dd216ab21..8201a0e500 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -61,6 +61,7 @@ from sentry_sdk.metrics import MetricsAggregator from sentry_sdk.scope import Scope from sentry_sdk.session import Session + from sentry_sdk.spotlight import SpotlightClient from sentry_sdk.transport import Transport I = TypeVar("I", bound=Integration) # noqa: E741 @@ -153,6 +154,8 @@ class BaseClient: The basic definition of a client that is used for sending data to Sentry. """ + spotlight = None # type: Optional[SpotlightClient] + def __init__(self, options=None): # type: (Optional[Dict[str, Any]]) -> None self.options = ( @@ -385,7 +388,6 @@ def _capture_envelope(envelope): disabled_integrations=self.options["disabled_integrations"], ) - self.spotlight = None spotlight_config = self.options.get("spotlight") if spotlight_config is None and "SENTRY_SPOTLIGHT" in os.environ: spotlight_env_value = os.environ["SENTRY_SPOTLIGHT"] diff --git a/sentry_sdk/spotlight.py b/sentry_sdk/spotlight.py index 3a5a713077..292c57613c 100644 --- a/sentry_sdk/spotlight.py +++ b/sentry_sdk/spotlight.py @@ -1,10 +1,16 @@ import io +import urllib.parse +import urllib.request +import urllib.error import urllib3 +from django.http import HttpResponseServerError +from django.conf import settings from typing import TYPE_CHECKING if TYPE_CHECKING: from typing import Any + from typing import Callable from typing import Dict from typing import Optional @@ -46,6 +52,40 @@ def capture_envelope(self, envelope): logger.warning(str(e)) +class SpotlightMiddleware: + def __init__(self, get_response): + # type: (Any, Callable[..., Any]) -> None + self.get_response = get_response + + def __call__(self, request): + # type: (Any, Any) -> Any + return self.get_response(request) + + def process_exception(self, _request, exception): + # type: (Any, Any, Exception) -> Optional[HttpResponseServerError] + if not settings.DEBUG: + return None + + import sentry_sdk.api + + spotlight_client = sentry_sdk.api.get_client().spotlight + if spotlight_client is None: + return None + + # Spotlight URL has a trailing `/stream` part at the end so split it off + spotlight_url = spotlight_client.url.rsplit("/", 1)[0] + + try: + spotlight = ( + urllib.request.urlopen(spotlight_url).read().decode("utf-8") + ).replace("", f'') + except urllib.error.URLError: + return None + else: + sentry_sdk.api.capture_exception(exception) + return HttpResponseServerError(spotlight) + + def setup_spotlight(options): # type: (Dict[str, Any]) -> Optional[SpotlightClient] @@ -58,4 +98,6 @@ def setup_spotlight(options): else: return None + settings.MIDDLEWARE.append("sentry_sdk.spotlight.SpotlightMiddleware") + return SpotlightClient(url) From 3ea64513968ecdbc3b5cee96a9ea4e2fff786f2d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 2 Oct 2024 20:01:09 +0100 Subject: [PATCH 2/5] don't assume django everywhere :facepalm: --- sentry_sdk/spotlight.py | 64 ++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/sentry_sdk/spotlight.py b/sentry_sdk/spotlight.py index 292c57613c..d87c4a7b61 100644 --- a/sentry_sdk/spotlight.py +++ b/sentry_sdk/spotlight.py @@ -3,8 +3,6 @@ import urllib.request import urllib.error import urllib3 -from django.http import HttpResponseServerError -from django.conf import settings from typing import TYPE_CHECKING @@ -52,38 +50,45 @@ def capture_envelope(self, envelope): logger.warning(str(e)) -class SpotlightMiddleware: - def __init__(self, get_response): - # type: (Any, Callable[..., Any]) -> None - self.get_response = get_response +try: + from django.http import HttpResponseServerError + from django.conf import settings - def __call__(self, request): - # type: (Any, Any) -> Any - return self.get_response(request) + class SpotlightMiddleware: + def __init__(self, get_response): + # type: (Any, Callable[..., Any]) -> None + self.get_response = get_response - def process_exception(self, _request, exception): - # type: (Any, Any, Exception) -> Optional[HttpResponseServerError] - if not settings.DEBUG: - return None + def __call__(self, request): + # type: (Any, Any) -> Any + return self.get_response(request) - import sentry_sdk.api + def process_exception(self, _request, exception): + # type: (Any, Any, Exception) -> Optional[HttpResponseServerError] + if not settings.DEBUG: + return None - spotlight_client = sentry_sdk.api.get_client().spotlight - if spotlight_client is None: - return None + import sentry_sdk.api - # Spotlight URL has a trailing `/stream` part at the end so split it off - spotlight_url = spotlight_client.url.rsplit("/", 1)[0] + spotlight_client = sentry_sdk.api.get_client().spotlight + if spotlight_client is None: + return None - try: - spotlight = ( - urllib.request.urlopen(spotlight_url).read().decode("utf-8") - ).replace("", f'') - except urllib.error.URLError: - return None - else: - sentry_sdk.api.capture_exception(exception) - return HttpResponseServerError(spotlight) + # Spotlight URL has a trailing `/stream` part at the end so split it off + spotlight_url = spotlight_client.url.rsplit("/", 1)[0] + + try: + spotlight = ( + urllib.request.urlopen(spotlight_url).read().decode("utf-8") + ).replace("", f'') + except urllib.error.URLError: + return None + else: + sentry_sdk.api.capture_exception(exception) + return HttpResponseServerError(spotlight) + +except ImportError: + settings = None def setup_spotlight(options): @@ -98,6 +103,7 @@ def setup_spotlight(options): else: return None - settings.MIDDLEWARE.append("sentry_sdk.spotlight.SpotlightMiddleware") + if settings is not None: + settings.MIDDLEWARE.append("sentry_sdk.spotlight.SpotlightMiddleware") return SpotlightClient(url) From ad338876fccb8977deed3addb7994d0f17717338 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 3 Oct 2024 13:33:03 +0100 Subject: [PATCH 3/5] add opt-out via $SENTRY_SPOTLIGHT_ON_ERROR --- sentry_sdk/spotlight.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/spotlight.py b/sentry_sdk/spotlight.py index d87c4a7b61..3e8072b5d8 100644 --- a/sentry_sdk/spotlight.py +++ b/sentry_sdk/spotlight.py @@ -1,4 +1,5 @@ import io +import os import urllib.parse import urllib.request import urllib.error @@ -12,7 +13,7 @@ from typing import Dict from typing import Optional -from sentry_sdk.utils import logger +from sentry_sdk.utils import logger, env_to_bool from sentry_sdk.envelope import Envelope @@ -103,7 +104,9 @@ def setup_spotlight(options): else: return None - if settings is not None: + if settings is not None and env_to_bool( + os.environ.get("SENTRY_SPOTLIGHT_ON_ERROR", "1") + ): settings.MIDDLEWARE.append("sentry_sdk.spotlight.SpotlightMiddleware") return SpotlightClient(url) From 709a8d8bf09b7cd035d20a47f96103ec6a4c4587 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 4 Oct 2024 13:59:01 +0100 Subject: [PATCH 4/5] add cursory tests --- tests/integrations/django/test_basic.py | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 2089f1e936..2889b5b11c 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -1240,3 +1240,35 @@ def test_transaction_http_method_custom(sentry_init, client, capture_events): (event1, event2) = events assert event1["request"]["method"] == "OPTIONS" assert event2["request"]["method"] == "HEAD" + + +def test_ensures_spotlight_middleware_when_spotlight_is_enabled(sentry_init, settings): + """ + Test that ensures if Spotlight is enabled, relevant SpotlightMiddleware + is added to middleware list in settings. + """ + original_middleware = frozenset(settings.MIDDLEWARE) + + sentry_init(integrations=[DjangoIntegration()], spotlight=True) + + added = frozenset(settings.MIDDLEWARE) ^ original_middleware + + assert "sentry_sdk.spotlight.SpotlightMiddleware" in added + + +def test_ensures_no_spotlight_middleware_when_env_killswitch_is_false( + monkeypatch, sentry_init, settings +): + """ + Test that ensures if Spotlight is enabled, but is set to a falsy value + the relevant SpotlightMiddleware is NOT added to middleware list in settings. + """ + monkeypatch.setenv("SENTRY_SPOTLIGHT_ON_ERROR", "no") + + original_middleware = frozenset(settings.MIDDLEWARE) + + sentry_init(integrations=[DjangoIntegration()], spotlight=True) + + added = frozenset(settings.MIDDLEWARE) ^ original_middleware + + assert "sentry_sdk.spotlight.SpotlightMiddleware" not in added From 344af850295b141f5e3e6a2f70c541b0a92799ed Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 4 Oct 2024 14:08:44 +0100 Subject: [PATCH 5/5] one more test --- tests/integrations/django/test_basic.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 2889b5b11c..a8cc02fda5 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -1272,3 +1272,22 @@ def test_ensures_no_spotlight_middleware_when_env_killswitch_is_false( added = frozenset(settings.MIDDLEWARE) ^ original_middleware assert "sentry_sdk.spotlight.SpotlightMiddleware" not in added + + +def test_ensures_no_spotlight_middleware_when_no_spotlight( + monkeypatch, sentry_init, settings +): + """ + Test that ensures if Spotlight is not enabled + the relevant SpotlightMiddleware is NOT added to middleware list in settings. + """ + # We should NOT have the middleware even if the env var is truthy if Spotlight is off + monkeypatch.setenv("SENTRY_SPOTLIGHT_ON_ERROR", "1") + + original_middleware = frozenset(settings.MIDDLEWARE) + + sentry_init(integrations=[DjangoIntegration()], spotlight=False) + + added = frozenset(settings.MIDDLEWARE) ^ original_middleware + + assert "sentry_sdk.spotlight.SpotlightMiddleware" not in added