Skip to content

Commit

Permalink
feat(sdk-crash-detection): Keep system lib frames (#52082)
Browse files Browse the repository at this point in the history
Only keep SDK and system library frames detected based on the library
path.

Closes #50916
  • Loading branch information
philipphofmann authored Jul 3, 2023
1 parent 0465d2b commit a1c2d96
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 24 deletions.
2 changes: 1 addition & 1 deletion fixtures/sdk_crash_detection/crash_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]:
return {
"function": function,
"package": "Sentry",
"package": "/private/var/containers/Bundle/Application/59E988EF-46DB-4C75-8E08-10C27DC3E90E/iOS-Swift.app/Frameworks/Sentry.framework/Sentry",
"in_app": in_app,
"image_addr": "0x100304000",
}
Expand Down
17 changes: 15 additions & 2 deletions src/sentry/utils/sdk_crashes/event_stripper.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,23 @@ 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.
Only keep SDK frames or Apple system libraries.
We need to adapt this logic once we support other platforms.
"""

def is_system_library(frame: Mapping[str, Any]) -> bool:
fields_containing_paths = {"package", "module", "abs_path"}
system_library_paths = {"/System/Library/", "/usr/lib/system/"}

for field in fields_containing_paths:
for path in system_library_paths:
if frame.get(field, "").startswith(path):
return True

return False

return [
frame
for frame in frames
if sdk_crash_detector.is_sdk_frame(frame) or frame.get("in_app", None) is False
if sdk_crash_detector.is_sdk_frame(frame) or is_system_library(frame)
]
1 change: 0 additions & 1 deletion src/sentry/utils/sdk_crashes/sdk_crash_detection.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ def detect_sdk_crash(self, event: Event, event_project_id: int) -> Optional[Even
return None

if self.cocoa_sdk_crash_detector.is_sdk_crash(frames):
# We still need to strip event data for to avoid collecting PII. We will do this in a separate PR.
sdk_crash_event_data = strip_event_data(event.data, self.cocoa_sdk_crash_detector)

set_path(
Expand Down
49 changes: 29 additions & 20 deletions tests/sentry/utils/sdk_crashes/test_event_stripper.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import abc

from fixtures.sdk_crash_detection.crash_event import (
IN_APP_FRAME,
get_crash_event,
get_crash_event_with_frames,
get_frames,
Expand Down Expand Up @@ -190,16 +189,28 @@ def test_strip_event_data_keeps_exception_stacktrace(self):
"platform": "platform",
}

def test_strip_frames_sentry_in_app_frame_kept(self):
frames = get_frames("SentryCrashMonitor_CPPException.cpp", sentry_frame_in_app=True)
self._execute_strip_frames_test(frames)

def test_strip_frames_sentry_non_in_app_frame_kept(self):
def test_strip_frames(self):
frames = get_frames("SentryCrashMonitor_CPPException.cpp", sentry_frame_in_app=False)
self._execute_strip_frames_test(frames)

def _execute_strip_frames_test(self, frames):
event_data = get_crash_event_with_frames(frames)
frames_kept = [
{
"abs_path": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore",
},
{
"module": "/usr/lib/system/libsystem_c.dylib",
},
]

frames_stripped = [
{
"abs_path": "/System/Librry/PrivateFrameworks/UIKitCore.framework/UIKitCore",
},
{
"module": "a/usr/lib/system/libsystem_c.dylib",
},
]

event_data = get_crash_event_with_frames(frames_kept + frames_stripped + list(frames))

event = self.create_event(
data=event_data,
Expand All @@ -212,17 +223,15 @@ def _execute_strip_frames_test(self, frames):
stripped_event_data, "exception", "values", -1, "stacktrace", "frames"
)

assert len(stripped_frames) == 7
assert (
len(
[
frame
for frame in stripped_frames
if frame["function"] == IN_APP_FRAME["function"]
]
)
== 0
), "in_app frame should be removed"
assert len(stripped_frames) == 9

cocoa_sdk_frame = stripped_frames[-1]
assert cocoa_sdk_frame == {
"function": "SentryCrashMonitor_CPPException.cpp",
"package": "/private/var/containers/Bundle/Application/59E988EF-46DB-4C75-8E08-10C27DC3E90E/iOS-Swift.app/Frameworks/Sentry.framework/Sentry",
"in_app": False,
"image_addr": "0x100304000",
}


@region_silo_test
Expand Down

0 comments on commit a1c2d96

Please sign in to comment.