diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 45ed04f81fcc4..bfc9e34614d3a 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -3434,4 +3434,7 @@ def build_cdc_postgres_init_db_volume(settings): # tests) SENTRY_METRICS_INDEXER_RAISE_VALIDATION_ERRORS = False +# The project ID for SDK Crash Monitoring to save the detected SDK crashed to. +SDK_CRASH_DETECTION_PROJECT_ID = None + SENTRY_FILE_COPY_ROLLOUT_RATE = 0.01 diff --git a/src/sentry/tasks/post_process.py b/src/sentry/tasks/post_process.py index a85f49e1d92f2..34108a8d2c49f 100644 --- a/src/sentry/tasks/post_process.py +++ b/src/sentry/tasks/post_process.py @@ -1053,6 +1053,12 @@ def sdk_crash_monitoring(job: PostProcessJob): if not features.has("organizations:sdk-crash-detection", event.project.organization): return + if settings.SDK_CRASH_DETECTION_PROJECT_ID is None: + logger.warning( + "SDK crash detection is enabled but SDK_CRASH_DETECTION_PROJECT_ID is not set." + ) + return None + with metrics.timer("post_process.sdk_crash_monitoring.duration"): with sentry_sdk.start_span(op="tasks.post_process_group.sdk_crash_monitoring"): sdk_crash_detection.detect_sdk_crash(event=event) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 997415548f540..1233e2f11c20a 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -34,8 +34,7 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Even stripped_debug_meta_images = _strip_debug_meta(debug_meta_images, stripped_frames) new_event_data["debug_meta"]["images"] = stripped_debug_meta_images - event.data = new_event_data - return event + return new_event_data def _filter_contexts(pair): diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index ad7da9de59225..a7da84972e067 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -1,5 +1,9 @@ from __future__ import annotations +from typing import Any, Dict, Optional + +from django.conf import settings + from sentry.eventstore.models import Event from sentry.issues.grouptype import GroupCategory from sentry.utils.safe import get_path, set_path @@ -12,8 +16,12 @@ class SDKCrashReporter: def __init__(self): self - def report(self, event: Event) -> None: - return + def report(self, event_data: Dict[str, Any], event_project_id: int) -> Event: + from sentry.event_manager import EventManager + + manager = EventManager(event_data) + manager.normalize() + return manager.save(project_id=event_project_id) class SDKCrashDetection: @@ -26,8 +34,7 @@ def __init__( self.sdk_crash_reporter = sdk_crash_reporter self.cocoa_sdk_crash_detector = sdk_crash_detector - def detect_sdk_crash(self, event: Event) -> None: - + def detect_sdk_crash(self, event: Event) -> Optional[Event]: should_detect_sdk_crash = ( event.group and event.group.issue_category == GroupCategory.ERROR @@ -38,25 +45,28 @@ def detect_sdk_crash(self, event: Event) -> None: context = get_path(event.data, "contexts", "sdk_crash_detection") if context is not None and context.get("detected", False): - return + return None is_unhandled = ( get_path(event.data, "exception", "values", -1, "mechanism", "data", "handled") is False ) if is_unhandled is False: - return + return None frames = get_path(event.data, "exception", "values", -1, "stacktrace", "frames") if not frames: - return + return None if self.cocoa_sdk_crash_detector.is_sdk_crash(frames): - sdk_crash_event = strip_event_data(event, self.cocoa_sdk_crash_detector) + sdk_crash_event_data = strip_event_data(event, self.cocoa_sdk_crash_detector) set_path( - sdk_crash_event.data, "contexts", "sdk_crash_detection", value={"detected": True} + sdk_crash_event_data, "contexts", "sdk_crash_detection", value={"detected": True} + ) + + return self.sdk_crash_reporter.report( + sdk_crash_event_data, settings.SDK_CRASH_DETECTION_PROJECT_ID ) - self.sdk_crash_reporter.report(sdk_crash_event) _crash_reporter = SDKCrashReporter() diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index d2ce954ba087c..d50786494d391 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -1486,6 +1486,7 @@ def test_forecast_in_activity(self, mock_is_escalating): class SDKCrashMonitoringTestMixin(BasePostProgressGroupMixin): @with_feature("organizations:sdk-crash-detection") @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") + @override_settings(SDK_CRASH_DETECTION_PROJECT_ID=1234) def test_sdk_crash_monitoring_is_called_with_event(self, mock_sdk_crash_reporter): event = self.create_event( data=get_crash_event(), @@ -1519,6 +1520,23 @@ def test_sdk_crash_monitoring_is_not_called_with_disabled_feature( mock_sdk_crash_detection.detect_sdk_crash.assert_not_called() + @with_feature("organizations:sdk-crash-detection") + @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection") + def test_sdk_crash_monitoring_is_not_called_without_project_id(self, mock_sdk_crash_detection): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) + + self.call_post_process_group( + is_new=True, + is_regression=False, + is_new_group_environment=True, + event=event, + ) + + mock_sdk_crash_detection.detect_sdk_crash.assert_not_called() + @region_silo_test class PostProcessGroupErrorTest( diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index 4eadc5077ae0c..614066625ab9b 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -20,7 +20,7 @@ class BaseEventStripperMixin(BaseTestCase, metaclass=abc.ABCMeta): def create_event(self, data, project_id, assert_no_errors=True): pass - def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): + def execute_test(self): pass @@ -31,11 +31,11 @@ def test_strip_event_data_keeps_allowed_keys(self): project_id=self.project.id, ) - stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) keys_removed = {"tags", "user", "threads", "breadcrumbs", "environment"} for key in keys_removed: - assert stripped_event.data.get(key) is None, f"key {key} should be removed" + assert stripped_event_data.get(key) is None, f"key {key} should be removed" keys_kept = { "type", @@ -50,19 +50,7 @@ def test_strip_event_data_keeps_allowed_keys(self): } for key in keys_kept: - assert stripped_event.data.get(key) is not None, f"key {key} should be kept" - - def test_strip_event_data_removes_ip_address(self): - event = self.create_event( - data=get_crash_event(), - project_id=self.project.id, - ) - - assert event.ip_address is not None - - stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) - - assert stripped_event.ip_address is None + assert stripped_event_data.get(key) is not None, f"key {key} should be kept" def test_strip_event_data_without_debug_meta(self): event_data = get_crash_event() @@ -73,9 +61,9 @@ def test_strip_event_data_without_debug_meta(self): project_id=self.project.id, ) - stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) - debug_meta_images = get_path(stripped_event.data, "debug_meta", "images") + debug_meta_images = get_path(stripped_event_data, "debug_meta", "images") assert debug_meta_images is None def test_strip_event_data_strips_context(self): @@ -84,9 +72,9 @@ def test_strip_event_data_strips_context(self): project_id=self.project.id, ) - stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) - contexts = stripped_event.data.get("contexts") + contexts = stripped_event_data.get("contexts") assert contexts is not None assert contexts.get("app") is None assert contexts.get("os") is not None @@ -98,9 +86,9 @@ def test_strip_event_data_strips_non_referenced_dsyms(self): project_id=self.project.id, ) - stripped_event = strip_event_data(event, Mock()) + stripped_event_data = strip_event_data(event, Mock()) - debug_meta_images = get_path(stripped_event.data, "debug_meta", "images") + debug_meta_images = get_path(stripped_event_data, "debug_meta", "images") image_addresses = set(map(lambda image: image["image_addr"], debug_meta_images)) expected_image_addresses = {"0x1a4e8f000", "0x100304000", "0x100260000"} @@ -122,10 +110,10 @@ def _execute_strip_frames_test(self, frames): project_id=self.project.id, ) - stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) stripped_frames = get_path( - stripped_event.data, "exception", "values", -1, "stacktrace", "frames" + stripped_event_data, "exception", "values", -1, "stacktrace", "frames" ) assert len(stripped_frames) == 6 diff --git a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py index 3da71d5bbbe26..621adf7c213cd 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -2,9 +2,13 @@ from typing import Any, Mapping, Sequence from unittest.mock import patch +import pytest +from django.test.utils import override_settings + +from sentry.eventstore.snuba.backend import SnubaEventStorage from sentry.issues.grouptype import PerformanceNPlusOneGroupType from sentry.testutils import TestCase -from sentry.testutils.cases import BaseTestCase +from sentry.testutils.cases import BaseTestCase, SnubaTestCase from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin from sentry.testutils.silo import region_silo_test from sentry.utils.sdk_crashes.sdk_crash_detection import sdk_crash_detection @@ -33,8 +37,8 @@ def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): if should_be_reported: mock_sdk_crash_reporter.report.assert_called_once() - reported_event = mock_sdk_crash_reporter.report.call_args.args[0] - assert reported_event.data["contexts"]["sdk_crash_detection"]["detected"] is True + reported_event_data = mock_sdk_crash_reporter.report.call_args.args[0] + assert reported_event_data["contexts"]["sdk_crash_detection"]["detected"] is True else: mock_sdk_crash_reporter.report.assert_not_called() @@ -287,6 +291,36 @@ def test_frames_only_in_app_after_sentry_frame_not_reported(self, mock_sdk_crash ) +class SDKCrashReportTestMixin(BaseSDKCrashDetectionMixin, SnubaTestCase): + @pytest.mark.django_db + def test_sdk_crash_event_stored_to_sdk_crash_project(self): + + cocoa_sdk_crashes_project = self.create_project( + name="Cocoa SDK Crashes", + slug="cocoa-sdk-crashes", + teams=[self.team], + fire_project_created=True, + ) + + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) + + with override_settings(SDK_CRASH_DETECTION_PROJECT_ID=cocoa_sdk_crashes_project.id): + sdk_crash_event = sdk_crash_detection.detect_sdk_crash(event=event) + + assert sdk_crash_event is not None + + event_store = SnubaEventStorage() + fetched_sdk_crash_event = event_store.get_event_by_id( + cocoa_sdk_crashes_project.id, sdk_crash_event.event_id + ) + + assert cocoa_sdk_crashes_project.id == fetched_sdk_crash_event.project_id + assert sdk_crash_event.event_id == fetched_sdk_crash_event.event_id + + @region_silo_test class SDKCrashDetectionTest( TestCase, @@ -295,6 +329,7 @@ class SDKCrashDetectionTest( CococaSDKFilenameTestMixin, CococaSDKFramesTestMixin, PerformanceEventTestMixin, + SDKCrashReportTestMixin, ): def create_event(self, data, project_id, assert_no_errors=True): return self.store_event(data=data, project_id=project_id, assert_no_errors=assert_no_errors)