Skip to content

Commit

Permalink
feat(sdk-crash-detection): Call code in post processing (#49170)
Browse files Browse the repository at this point in the history
Call SDK crash detection code in post-processing. I had to adapt the
code a bit, cause I tested it with a dictionary instead of a stored
event. I adapted the tests to use the same approach as the
post-processing tests. The logic of the tests didn't change. I
refactored all of them to work with stored events.

Fixes GH-46175

Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
  • Loading branch information
philipphofmann and jjbayer authored May 30, 2023
1 parent 087e2c1 commit ed415e4
Show file tree
Hide file tree
Showing 7 changed files with 523 additions and 375 deletions.
14 changes: 14 additions & 0 deletions src/sentry/tasks/post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,19 @@ def fire_error_processed(job: PostProcessJob):
)


def sdk_crash_monitoring(job: PostProcessJob):
from sentry.utils.sdk_crashes.sdk_crash_detection import sdk_crash_detection

if job["is_reprocessed"]:
return

event = job["event"]

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)


def plugin_post_process_group(plugin_slug, event, **kwargs):
"""
Fires post processing hooks for a group.
Expand Down Expand Up @@ -1044,6 +1057,7 @@ def plugin_post_process_group(plugin_slug, event, **kwargs):
process_similarity,
update_existing_attachments,
fire_error_processed,
sdk_crash_monitoring,
],
GroupCategory.PERFORMANCE: [
process_snoozes,
Expand Down
139 changes: 68 additions & 71 deletions src/sentry/utils/sdk_crashes/event_stripper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,75 +4,72 @@
from sentry.utils.safe import get_path
from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector

ALLOWED_EVENT_KEYS = {
"type",
"datetime",
"timestamp",
"platform",
"sdk",
"level",
"logger",
"exception",
"debug_meta",
"contexts",
}

class EventStripper:
def __init__(
self,
sdk_crash_detector: SDKCrashDetector,
):
self
self.sdk_crash_detector = sdk_crash_detector

ALLOWED_EVENT_KEYS = {
"type",
"datetime",
"timestamp",
"platform",
"sdk",
"level",
"logger",
"exception",
"debug_meta",
"contexts",
}

def strip_event_data(self, event: Event) -> Event:
new_event = dict(filter(self._filter_event, event.items()))
new_event["contexts"] = dict(filter(self._filter_contexts, new_event["contexts"].items()))

stripped_frames = []
frames = get_path(event, "exception", "values", -1, "stacktrace", "frames")

if frames is not None:
stripped_frames = self._strip_frames(frames)
new_event["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames

debug_meta_images = get_path(event, "debug_meta", "images")
if debug_meta_images is not None:
stripped_debug_meta_images = self._strip_debug_meta(debug_meta_images, stripped_frames)
new_event["debug_meta"]["images"] = stripped_debug_meta_images

return new_event

def _filter_event(self, pair):
key, _ = pair
if key in self.ALLOWED_EVENT_KEYS:
return True

return False

def _filter_contexts(self, pair):
key, _ = pair
if key in {"os", "device"}:
return True
return False

def _strip_debug_meta(
self, debug_meta_images: Sequence[Mapping[str, Any]], frames: Sequence[Mapping[str, Any]]
) -> Sequence[Mapping[str, Any]]:

frame_image_addresses = {frame["image_addr"] for frame in frames}

return [
image for image in debug_meta_images if image["image_addr"] in frame_image_addresses
]

def _strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]:
"""
Only keep SDK frames or non in app frames.
"""
return [
frame
for frame in frames
if self.sdk_crash_detector.is_sdk_frame(frame) or frame.get("in_app", True) is False
]

def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Event:
new_event_data = dict(filter(_filter_event, event.data.items()))
new_event_data["contexts"] = dict(filter(_filter_contexts, new_event_data["contexts"].items()))

stripped_frames = []
frames = get_path(new_event_data, "exception", "values", -1, "stacktrace", "frames")

if frames is not None:
stripped_frames = _strip_frames(frames, sdk_crash_detector)
new_event_data["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames

debug_meta_images = get_path(new_event_data, "debug_meta", "images")
if debug_meta_images is not None:
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


def _filter_event(pair):
key, _ = pair
if key in ALLOWED_EVENT_KEYS:
return True

return False


def _filter_contexts(pair):
key, _ = pair
if key in {"os", "device"}:
return True
return False


def _strip_debug_meta(
debug_meta_images: Sequence[Mapping[str, Any]], frames: Sequence[Mapping[str, Any]]
) -> Sequence[Mapping[str, Any]]:

frame_image_addresses = {frame["image_addr"] for frame in frames}

return [image for image in debug_meta_images if image["image_addr"] in frame_image_addresses]


def _strip_frames(
frames: Sequence[Mapping[str, Any]], sdk_crash_detector: SDKCrashDetector
) -> Sequence[Mapping[str, Any]]:
"""
Only keep SDK frames or non in app frames.
"""
return [
frame
for frame in frames
if sdk_crash_detector.is_sdk_frame(frame) or frame.get("in_app", None) is False
]
36 changes: 26 additions & 10 deletions src/sentry/utils/sdk_crashes/sdk_crash_detection.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from __future__ import annotations

from sentry.eventstore.models import Event
from sentry.issues.grouptype import GroupCategory
from sentry.utils.safe import get_path, set_path
from sentry.utils.sdk_crashes.event_stripper import EventStripper
from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector
from sentry.utils.sdk_crashes.event_stripper import strip_event_data
from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector


Expand All @@ -11,39 +13,53 @@ def __init__(self):
self

def report(self, event: Event) -> None:
pass
return


class SDKCrashDetection:
def __init__(
self,
sdk_crash_reporter: SDKCrashReporter,
sdk_crash_detector: SDKCrashDetector,
event_stripper: EventStripper,
):
self
self.sdk_crash_reporter = sdk_crash_reporter
self.cocoa_sdk_crash_detector = sdk_crash_detector
self.event_stripper = event_stripper

def detect_sdk_crash(self, event: Event) -> None:
if event.get("type", None) != "error" or event.get("platform") != "cocoa":

should_detect_sdk_crash = (
event.group
and event.group.issue_category == GroupCategory.ERROR
and event.group.platform == "cocoa"
)
if not should_detect_sdk_crash:
return

context = get_path(event, "contexts", "sdk_crash_detection")
context = get_path(event.data, "contexts", "sdk_crash_detection")
if context is not None and context.get("detected", False):
return

is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False
is_unhandled = (
get_path(event.data, "exception", "values", -1, "mechanism", "data", "handled") is False
)
if is_unhandled is False:
return

frames = get_path(event, "exception", "values", -1, "stacktrace", "frames")
frames = get_path(event.data, "exception", "values", -1, "stacktrace", "frames")
if not frames:
return

if self.cocoa_sdk_crash_detector.is_sdk_crash(frames):
sdk_crash_event = self.event_stripper.strip_event_data(event)
sdk_crash_event = strip_event_data(event, self.cocoa_sdk_crash_detector)

set_path(sdk_crash_event, "contexts", "sdk_crash_detection", value={"detected": True})
set_path(
sdk_crash_event.data, "contexts", "sdk_crash_detection", value={"detected": True}
)
self.sdk_crash_reporter.report(sdk_crash_event)


_crash_reporter = SDKCrashReporter()
_cocoa_sdk_crash_detector = CocoaSDKCrashDetector()

sdk_crash_detection = SDKCrashDetection(_crash_reporter, _cocoa_sdk_crash_detector)
20 changes: 20 additions & 0 deletions tests/sentry/tasks/test_post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from sentry.types.group import GroupSubStatus
from sentry.utils.cache import cache
from tests.sentry.issues.test_utils import OccurrenceTestMixin
from tests.sentry.utils.sdk_crashes.test_fixture import get_crash_event


class EventMatcher:
Expand Down Expand Up @@ -1481,6 +1482,24 @@ def test_forecast_in_activity(self, mock_is_escalating):
).exists()


@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter")
class SDKCrashMonitoringTestMixin(BasePostProgressGroupMixin):
def test_sdk_crash_monitoring_is_called_with_event(self, mock_sdk_crash_reporter):
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_reporter.report.assert_called_once()


@region_silo_test
class PostProcessGroupErrorTest(
TestCase,
Expand All @@ -1493,6 +1512,7 @@ class PostProcessGroupErrorTest(
RuleProcessorTestMixin,
ServiceHooksTestMixin,
SnoozeTestMixin,
SDKCrashMonitoringTestMixin,
):
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)
Expand Down
Loading

0 comments on commit ed415e4

Please sign in to comment.