ref(sdk-crashes): Add some code comments (#52608)

Add some comments to the classes and move should_detect_sdk_crash to the
base class SDKCrashDetector.

src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py

@@ -1,49 +1,21 @@
 from typing import Any, Mapping, Sequence
 
-from packaging.version import InvalidVersion, Version
-
-from sentry.db.models import NodeData
 from sentry.utils.glob import glob_match
-from sentry.utils.safe import get_path
-from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector
+from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector, SDKCrashDetectorConfig
 
 
 class CocoaSDKCrashDetector(SDKCrashDetector):
-    @property
-    def min_sdk_version(self) -> str:
-        """
-        Since changing the debug image type to macho (https://github.com/getsentry/sentry-cocoa/pull/2701)
-        released in sentry-cocoa 8.2.0 (https://github.com/getsentry/sentry-cocoa/blob/main/CHANGELOG.md#820),
-        the frames contain the full paths required for detecting system frames in is_system_library_frame.
-        Therefore, we require at least sentry-cocoa 8.2.0.
-        """
-        return "8.2.0"
-
-    def should_detect_sdk_crash(self, event_data: NodeData) -> bool:
-        sdk_name = get_path(event_data, "sdk", "name")
-        if sdk_name and sdk_name != "sentry.cocoa":
-            return False
-
-        sdk_version = get_path(event_data, "sdk", "version")
-        if not sdk_version:
-            return False
-
-        try:
-            minimum_cocoa_sdk_version = Version(self.min_sdk_version)
-            cocoa_sdk_version = Version(sdk_version)
-
-            if cocoa_sdk_version < minimum_cocoa_sdk_version:
-                return False
-        except InvalidVersion:
-            return False
-
-        is_unhandled = (
-            get_path(event_data, "exception", "values", -1, "mechanism", "handled") is False
+    def __init__(self):
+
+        config = SDKCrashDetectorConfig(
+            sdk_name="sentry.cocoa",
+            # Since changing the debug image type to macho (https://github.com/getsentry/sentry-cocoa/pull/2701)
+            # released in sentry-cocoa 8.2.0 (https://github.com/getsentry/sentry-cocoa/blob/main/CHANGELOG.md#820),
+            # the frames contain the full paths required for detecting system frames in is_system_library_frame.
+            # Therefore, we require at least sentry-cocoa 8.2.0.
+            min_sdk_version="8.2.0",
         )
-        if not is_unhandled:
-            return False
-
-        return True
+        super().__init__(config)
 
     def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
         if not frames:

src/sentry/utils/sdk_crashes/event_stripper.py

@@ -96,6 +96,10 @@ EVENT_DATA_ALLOWLIST = {
 def strip_event_data(
     event_data: NodeData, sdk_crash_detector: SDKCrashDetector
 ) -> Mapping[str, Any]:
+    """
+    This method keeps only properties based on the ALLOW_LIST. For frames, both the allow list applies,
+    and the method only keeps SDK frames and system library frames.
+    """
 
     frames = get_path(event_data, "exception", "values", -1, "stacktrace", "frames")
     if not frames:

src/sentry/utils/sdk_crashes/sdk_crash_detection.py

@@ -21,6 +21,16 @@ class SDKCrashReporter:
 
 
 class SDKCrashDetection:
+    """
+    This class checks events for SDK crashes, a crash caused by a bug in one of our SDKs.
+    When it detects such an event, it only keeps essential data and stores the event in a
+    dedicated Sentry project only Sentry employees can access.
+
+    This class doesn't seek to detect severe bugs, such as the transport layer breaking or
+    the SDK continuously crashing. CI or other quality mechanisms should find such severe
+    bugs. Furthermore, the solution only targets SDKs maintained by us, Sentry.
+    """
+
     def __init__(
         self,
         sdk_crash_reporter: SDKCrashReporter,
@@ -32,6 +42,14 @@ class SDKCrashDetection:
     def detect_sdk_crash(
         self, event: Event, event_project_id: int, sample_rate: float
     ) -> Optional[Event]:
+        """
+        Checks if the passed-in event is an SDK crash and stores the stripped event to a Sentry
+        project specified with event_project_id.
+
+        :param event: The event to check for an SDK crash.
+        :param event_project_id: The project ID to store the SDK crash event to if one is detected.
+        :param sample_rate: Sampling gets applied after an event is considered an SDK crash.
+        """
 
         should_detect_sdk_crash = (
             event.group

src/sentry/utils/sdk_crashes/sdk_crash_detector.py

@@ -1,17 +1,63 @@
 from abc import ABC, abstractmethod
+from dataclasses import dataclass
 from typing import Any, Mapping, Sequence, Set
 
+from packaging.version import InvalidVersion, Version
+
 from sentry.db.models import NodeData
+from sentry.utils.safe import get_path
+
+
+@dataclass
+class SDKCrashDetectorConfig:
+    sdk_name: str
+
+    min_sdk_version: str
 
 
 class SDKCrashDetector(ABC):
+    """
+    This class is still a work in progress. The plan is that every SDK has to define a subclass of
+    this base class to get SDK crash detection up and running. We most likely will have to pull
+    out some logic of the CocoaSDKCrashDetector into this class when adding the SDK crash detection
+    for another SDK.
+    """
+
+    def __init__(
+        self,
+        config: SDKCrashDetectorConfig,
+    ):
+        self.config = config
+
     @property
     def fields_containing_paths(self) -> Set[str]:
         return {"package", "module", "abs_path"}
 
-    @abstractmethod
     def should_detect_sdk_crash(self, event_data: NodeData) -> bool:
-        raise NotImplementedError
+        sdk_name = get_path(event_data, "sdk", "name")
+        if sdk_name and sdk_name != self.config.sdk_name:
+            return False
+
+        sdk_version = get_path(event_data, "sdk", "version")
+        if not sdk_version:
+            return False
+
+        try:
+            minimum_sdk_version = Version(self.config.min_sdk_version)
+            actual_sdk_version = Version(sdk_version)
+
+            if actual_sdk_version < minimum_sdk_version:
+                return False
+        except InvalidVersion:
+            return False
+
+        is_unhandled = (
+            get_path(event_data, "exception", "values", -1, "mechanism", "handled") is False
+        )
+        if not is_unhandled:
+            return False
+
+        return True
 
     @abstractmethod
     def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
@@ -24,6 +70,11 @@ class SDKCrashDetector(ABC):
 
     @abstractmethod
     def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool:
+        """
+        Returns true if frame is an SDK frame.
+
+        :param frame: The frame of a stacktrace.
+        """
         raise NotImplementedError
 
     @abstractmethod