From 8f39673aae2a700bcdb0d100de7322067f156288 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Fri, 3 Feb 2023 09:57:32 +0100 Subject: [PATCH 01/37] test: Fix test discovery by adding missing init.py Running test_discover.py with the test explorer in VSCode failed with an import file mismatch. This is fixed now by adding a missing __init__.py file to tests/sentry/data_export/processors. --- tests/sentry/data_export/processors/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/sentry/data_export/processors/__init__.py diff --git a/tests/sentry/data_export/processors/__init__.py b/tests/sentry/data_export/processors/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 From 48cd13a6aafe91d79f01253400f7c542615db51f Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 6 Feb 2023 11:59:45 +0100 Subject: [PATCH 02/37] backup --- src/sentry/event_manager.py | 7 + .../utils/sdk_crashes/sdk_crash_detection.py | 27 ++++ .../sdk_crashes/test_sdk_crash_detection.py | 126 ++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 src/sentry/utils/sdk_crashes/sdk_crash_detection.py create mode 100644 tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 3a46eb0f3a6cb8..3ab6ea90d12e5c 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -140,6 +140,7 @@ detect_performance_problems, ) from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim +from sentry.utils.sdk_crashes.sdk_crash_detection import detect_sdk_crash if TYPE_CHECKING: from sentry.eventstore.models import Event @@ -2209,6 +2210,11 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping) ) +def _detect_sdk_crashes(jobs: Sequence[Job]): + for job in jobs: + detect_sdk_crash(job["data"]) + + class PerformanceJob(TypedDict, total=False): performance_problems: Sequence[PerformanceProblem] event: Event @@ -2473,6 +2479,7 @@ def save_transaction_events(jobs: Sequence[Job], projects: ProjectsMapping) -> S _nodestore_save_many(jobs) _eventstream_insert_many(jobs) _track_outcome_accepted_many(jobs) + _detect_sdk_crashes(jobs) return jobs diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py new file mode 100644 index 00000000000000..ec59852886f9b9 --- /dev/null +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +from typing import Any, Mapping, Sequence + +from sentry.eventstore.models import Event +from sentry.utils.glob import glob_match + + +def detect_sdk_crash(data: Event): + event_id = data.get("event_id", None) + if event_id is None: + return + return + + +def is_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: + if not frames: + return False + + last_frame = frames[-1] + if last_frame is None: + return False + + if glob_match(last_frame["function"], "?[[]Sentry*", ignorecase=True): + return True + + return False diff --git a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py new file mode 100644 index 00000000000000..2f0131297e4ed2 --- /dev/null +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -0,0 +1,126 @@ +from typing import Any, Mapping + +import pytest + +from sentry.utils.sdk_crashes.sdk_crash_detection import is_sdk_crash + + +@pytest.mark.parametrize( + "function,expected", + [ + ("-[SentryHub getScope]", True), + ("-[sentryisgreat]", True), + ("-[SentryCrash crash]", True), + ("-[SenryHub getScope]", False), + ("-SentryHub getScope]", False), + ("-[SomeSentryHub getScope]", False), + ], +) +def test_sdk_crash_detection(function, expected): + frames = [ + { + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1b6568000", + "instruction_addr": "0x1b676a1a8", + "symbol_addr": "0x1b676a0dc", + }, + { + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "package": "UIKitCore", + "in_app": False, + "data": {"symbolicator_status": "symbolicated"}, + "image_addr": "0x1b6568000", + "instruction_addr": "0x1b66b8238", + "symbol_addr": "0x1b66b7d90", + }, + { + "function": "-[UIViewController _endAppearanceTransition:]", + "symbol": "-[UIViewController _endAppearanceTransition:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1b6568000", + "instruction_addr": "0x1b678d344", + "symbol_addr": "0x1b678d270", + }, + { + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1b6568000", + "instruction_addr": "0x1b678d4c8", + "symbol_addr": "0x1b678d428", + }, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1b6568000", + "instruction_addr": "0x1b6574ef0", + "symbol_addr": "0x1b6574a78", + }, + { + "function": "LoginViewController.viewDidAppear", + "raw_function": "@objc LoginViewController.viewDidAppear(Bool)", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbFTo", + "package": "SentryApp", + "filename": "", + "abs_path": "", + "in_app": True, + "image_addr": "0x1025e8000", + "instruction_addr": "0x102b16798", + "symbol_addr": "0x1025e8000", + }, + { + "function": "LoginViewController.viewDidAppear", + "raw_function": "LoginViewController.viewDidAppear(Bool)", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", + "lineno": 196, + "in_app": True, + "image_addr": "0x1025e8000", + "instruction_addr": "0x102b16630", + "symbol_addr": "0x1025e8000", + }, + { + "function": "__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", + "symbol": "__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", + "package": "Sentry", + "filename": "SentryBreadcrumbTracker.m", + "abs_path": "/Users/vagrant/git/iOS/Carthage/Checkouts/sentry-cocoa/Sources/Sentry/SentryBreadcrumbTracker.m", + "lineno": 145, + "in_app": True, + "image_addr": "0x1055dc000", + "instruction_addr": "0x1056054d0", + "symbol_addr": "0x1056052e8", + }, + create_sentry_frame(function), + ] + + assert is_sdk_crash(frames) is expected + + +def test_is_sdk_crash_no_frames(): + assert is_sdk_crash([]) is False + + +def test_is_sdk_crash_single_frame(): + assert is_sdk_crash([create_sentry_frame("-[Sentry]")]) is True + + +def create_sentry_frame(function) -> Mapping[str, Any]: + return { + "function": function, + "package": "Sentry", + "filename": "SentryHub.m", + "instruction_addr": 4295098388, + "abs_path": "/Users/sentry/git/iOS/Carthage/Checkouts/sentry-cocoa/Sources/Sentry/SentryHub.m", + "in_app:": False, + } From beb3c770284bd9cc9a54d37944f8009270809aa0 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 7 Feb 2023 12:25:14 +0100 Subject: [PATCH 03/37] backup --- .../utils/sdk_crashes/sdk_crash_detection.py | 30 +++- .../sdk_crashes/test_sdk_crash_detection.py | 162 +++++++++++++----- 2 files changed, 139 insertions(+), 53 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index ec59852886f9b9..839bb6f09df7c8 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -17,11 +17,29 @@ def is_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: if not frames: return False - last_frame = frames[-1] - if last_frame is None: - return False - - if glob_match(last_frame["function"], "?[[]Sentry*", ignorecase=True): - return True + for frame in reversed(frames): + + function = frame.get("function") + + if function is not None: + # [SentrySDK crash] is a testing function causing a crash. + # Therefore, we don't want to mark it a as a SDK crash. + if "SentrySDK crash" in function: + return False + + functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] + for matcher in functionsMatchers: + if glob_match(frame.get("function"), matcher, ignorecase=True): + return True + + filename = frame.get("filename") + if filename is not None: + filenameMatchers = ["Sentry**"] + for matcher in filenameMatchers: + if glob_match(filename, matcher, ignorecase=True): + return True + + if frame.get("in_app") is True: + return False return False 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 2f0131297e4ed2..7092146c489b13 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -4,16 +4,33 @@ from sentry.utils.sdk_crashes.sdk_crash_detection import is_sdk_crash +in_app_frame = { + "function": "LoginViewController.viewDidAppear", + "raw_function": "LoginViewController.viewDidAppear(Bool)", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", + "lineno": 196, + "in_app": True, + "image_addr": "0x1025e8000", + "instruction_addr": "0x102b16630", + "symbol_addr": "0x1025e8000", +} + @pytest.mark.parametrize( "function,expected", [ ("-[SentryHub getScope]", True), + ("sentrycrashdl_getBinaryImage", True), ("-[sentryisgreat]", True), + ("__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", True), ("-[SentryCrash crash]", True), ("-[SenryHub getScope]", False), ("-SentryHub getScope]", False), ("-[SomeSentryHub getScope]", False), + ("+[SentrySDK crash]", False), ], ) def test_sdk_crash_detection(function, expected): @@ -23,85 +40,135 @@ def test_sdk_crash_detection(function, expected): "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "package": "UIKitCore", "in_app": False, - "image_addr": "0x1b6568000", - "instruction_addr": "0x1b676a1a8", - "symbol_addr": "0x1b676a0dc", }, { "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "package": "UIKitCore", "in_app": False, - "data": {"symbolicator_status": "symbolicated"}, - "image_addr": "0x1b6568000", - "instruction_addr": "0x1b66b8238", - "symbol_addr": "0x1b66b7d90", }, { "function": "-[UIViewController _endAppearanceTransition:]", "symbol": "-[UIViewController _endAppearanceTransition:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x1b6568000", - "instruction_addr": "0x1b678d344", - "symbol_addr": "0x1b678d270", }, { "function": "-[UIViewController __viewDidAppear:]", "symbol": "-[UIViewController __viewDidAppear:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x1b6568000", - "instruction_addr": "0x1b678d4c8", - "symbol_addr": "0x1b678d428", }, { "function": "-[UIViewController _setViewAppearState:isAnimating:]", "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x1b6568000", - "instruction_addr": "0x1b6574ef0", - "symbol_addr": "0x1b6574a78", - }, - { - "function": "LoginViewController.viewDidAppear", - "raw_function": "@objc LoginViewController.viewDidAppear(Bool)", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbFTo", - "package": "SentryApp", - "filename": "", - "abs_path": "", - "in_app": True, - "image_addr": "0x1025e8000", - "instruction_addr": "0x102b16798", - "symbol_addr": "0x1025e8000", }, + in_app_frame, { "function": "LoginViewController.viewDidAppear", - "raw_function": "LoginViewController.viewDidAppear(Bool)", "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", "package": "SentryApp", "filename": "LoginViewController.swift", - "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", "lineno": 196, "in_app": True, - "image_addr": "0x1025e8000", - "instruction_addr": "0x102b16630", - "symbol_addr": "0x1025e8000", }, + create_sentry_frame(function), + ] + + assert is_sdk_crash(frames) is expected + + +def test_is_sdk_crash_only_non_inapp_after_sentry_frame(): + frames = [ { - "function": "__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", - "symbol": "__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", - "package": "Sentry", - "filename": "SentryBreadcrumbTracker.m", - "abs_path": "/Users/vagrant/git/iOS/Carthage/Checkouts/sentry-cocoa/Sources/Sentry/SentryBreadcrumbTracker.m", - "lineno": 145, - "in_app": True, - "image_addr": "0x1055dc000", - "instruction_addr": "0x1056054d0", - "symbol_addr": "0x1056052e8", + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + ] + + assert is_sdk_crash(frames) is True + + +def test_is_sdk_crash_only_inapp_after_sentry_frame(): + frames = [ + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + in_app_frame, + ] + + assert is_sdk_crash(frames) is False + + +@pytest.mark.parametrize( + "filename,expected", + [ + ("SentryCrashMonitor_CPPException.cpp", True), + ("SentryMonitor_CPPException.cpp", True), + ("SentrMonitor_CPPException.cpp", False), + ], +) +def test_is_sdk_crash_filename(filename, expected): + frames = [ + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + { + "function": "CPPExceptionTerminate", + "raw_function": "CPPExceptionTerminate()", + "filename": filename, + "symbol": "_ZL21CPPExceptionTerminatev", + "package": "MainApp", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, }, - create_sentry_frame(function), ] assert is_sdk_crash(frames) is expected @@ -111,6 +178,10 @@ def test_is_sdk_crash_no_frames(): assert is_sdk_crash([]) is False +def test_is_sdk_crash_empty_frames(): + assert is_sdk_crash([{"empty": "frame"}]) is False + + def test_is_sdk_crash_single_frame(): assert is_sdk_crash([create_sentry_frame("-[Sentry]")]) is True @@ -119,8 +190,5 @@ def create_sentry_frame(function) -> Mapping[str, Any]: return { "function": function, "package": "Sentry", - "filename": "SentryHub.m", - "instruction_addr": 4295098388, - "abs_path": "/Users/sentry/git/iOS/Carthage/Checkouts/sentry-cocoa/Sources/Sentry/SentryHub.m", "in_app:": False, } From ef314727f927447b4715d8f791439fb0c14502d7 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 8 Feb 2023 10:06:46 +0100 Subject: [PATCH 04/37] Change frame order from newest to oldest --- .../utils/sdk_crashes/sdk_crash_detection.py | 7 +- .../sdk_crashes/test_sdk_crash_detection.py | 90 +++++++++---------- 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 839bb6f09df7c8..8b54f0a244c38b 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -14,10 +14,15 @@ def detect_sdk_crash(data: Event): def is_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: + """ + Returns true if the stacktrace is a SDK crash. + + :param frames: The stacktrace frames ordered from newest to oldest. + """ if not frames: return False - for frame in reversed(frames): + for frame in frames: function = frame.get("function") 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 7092146c489b13..82b2b020f23e87 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -35,15 +35,25 @@ ) def test_sdk_crash_detection(function, expected): frames = [ + create_sentry_frame(function), { - "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "function": "LoginViewController.viewDidAppear", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + "lineno": 196, + "in_app": True, + }, + in_app_frame, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", "package": "UIKitCore", "in_app": False, }, { - "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", "package": "UIKitCore", "in_app": False, }, @@ -54,27 +64,17 @@ def test_sdk_crash_detection(function, expected): "in_app": False, }, { - "function": "-[UIViewController __viewDidAppear:]", - "symbol": "-[UIViewController __viewDidAppear:]", + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "package": "UIKitCore", "in_app": False, }, { - "function": "-[UIViewController _setViewAppearState:isAnimating:]", - "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "package": "UIKitCore", "in_app": False, }, - in_app_frame, - { - "function": "LoginViewController.viewDidAppear", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - "lineno": 196, - "in_app": True, - }, - create_sentry_frame(function), ] assert is_sdk_crash(frames) is expected @@ -83,22 +83,22 @@ def test_sdk_crash_detection(function, expected): def test_is_sdk_crash_only_non_inapp_after_sentry_frame(): frames = [ { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", "in_app": False, }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "_objc_terminate", "symbol": "_ZL15_objc_terminatev", "package": "libobjc.A.dylib", "in_app": False, }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", "in_app": False, }, ] @@ -108,26 +108,26 @@ def test_is_sdk_crash_only_non_inapp_after_sentry_frame(): def test_is_sdk_crash_only_inapp_after_sentry_frame(): frames = [ + in_app_frame, { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", "in_app": False, }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "_objc_terminate", "symbol": "_ZL15_objc_terminatev", "package": "libobjc.A.dylib", "in_app": False, }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", "in_app": False, }, - in_app_frame, ] assert is_sdk_crash(frames) is False @@ -144,9 +144,15 @@ def test_is_sdk_crash_only_inapp_after_sentry_frame(): def test_is_sdk_crash_filename(filename, expected): frames = [ { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", "in_app": False, }, { @@ -158,15 +164,9 @@ def test_is_sdk_crash_filename(filename, expected): "in_app": False, }, { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", "in_app": False, }, ] From 3b6bf5cb277ba883c43a0911ea0a7507b5c62b5d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 8 Feb 2023 10:11:27 +0100 Subject: [PATCH 05/37] Rename to Cocoa --- .../utils/sdk_crashes/sdk_crash_detection.py | 4 +-- .../sdk_crashes/test_sdk_crash_detection.py | 30 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 8b54f0a244c38b..a8f8671bb9b65d 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -13,9 +13,9 @@ def detect_sdk_crash(data: Event): return -def is_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: +def is_cocoa_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: """ - Returns true if the stacktrace is a SDK crash. + Returns true if the stacktrace is a Cocoa SDK crash. :param frames: The stacktrace frames ordered from newest to oldest. """ 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 82b2b020f23e87..180163d3701aad 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -2,7 +2,7 @@ import pytest -from sentry.utils.sdk_crashes.sdk_crash_detection import is_sdk_crash +from sentry.utils.sdk_crashes.sdk_crash_detection import is_cocoa_sdk_crash in_app_frame = { "function": "LoginViewController.viewDidAppear", @@ -33,7 +33,7 @@ ("+[SentrySDK crash]", False), ], ) -def test_sdk_crash_detection(function, expected): +def test_cocoa_sdk_crash_detection(function, expected): frames = [ create_sentry_frame(function), { @@ -77,10 +77,10 @@ def test_sdk_crash_detection(function, expected): }, ] - assert is_sdk_crash(frames) is expected + assert is_cocoa_sdk_crash(frames) is expected -def test_is_sdk_crash_only_non_inapp_after_sentry_frame(): +def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): frames = [ { "function": "__handleUncaughtException", @@ -103,10 +103,10 @@ def test_is_sdk_crash_only_non_inapp_after_sentry_frame(): }, ] - assert is_sdk_crash(frames) is True + assert is_cocoa_sdk_crash(frames) is True -def test_is_sdk_crash_only_inapp_after_sentry_frame(): +def test_is_cocoa_sdk_crash_only_inapp_after_sentry_frame(): frames = [ in_app_frame, { @@ -130,7 +130,7 @@ def test_is_sdk_crash_only_inapp_after_sentry_frame(): }, ] - assert is_sdk_crash(frames) is False + assert is_cocoa_sdk_crash(frames) is False @pytest.mark.parametrize( @@ -141,7 +141,7 @@ def test_is_sdk_crash_only_inapp_after_sentry_frame(): ("SentrMonitor_CPPException.cpp", False), ], ) -def test_is_sdk_crash_filename(filename, expected): +def test_is_cocoa_sdk_crash_filename(filename, expected): frames = [ { "function": "__handleUncaughtException", @@ -171,19 +171,19 @@ def test_is_sdk_crash_filename(filename, expected): }, ] - assert is_sdk_crash(frames) is expected + assert is_cocoa_sdk_crash(frames) is expected -def test_is_sdk_crash_no_frames(): - assert is_sdk_crash([]) is False +def test_is_cocoa_sdk_crash_no_frames(): + assert is_cocoa_sdk_crash([]) is False -def test_is_sdk_crash_empty_frames(): - assert is_sdk_crash([{"empty": "frame"}]) is False +def test_is_cocoa_sdk_crash_empty_frames(): + assert is_cocoa_sdk_crash([{"empty": "frame"}]) is False -def test_is_sdk_crash_single_frame(): - assert is_sdk_crash([create_sentry_frame("-[Sentry]")]) is True +def test_is_cocoa_sdk_crash_single_frame(): + assert is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True def create_sentry_frame(function) -> Mapping[str, Any]: From 9cc03e63e1c737c7bb6233238e5bef86421471a2 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 8 Feb 2023 12:05:22 +0100 Subject: [PATCH 06/37] Add events --- src/sentry/event_manager.py | 2 +- .../utils/sdk_crashes/sdk_crash_detection.py | 23 +++++++-- .../sdk_crashes/test_sdk_crash_detection.py | 49 ++++++++++++++++++- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 3ab6ea90d12e5c..c98a5cd633e5be 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -660,6 +660,7 @@ def save( ) _track_outcome_accepted_many(jobs) + _detect_sdk_crashes(jobs) self._data = job["event"].data.data @@ -2479,7 +2480,6 @@ def save_transaction_events(jobs: Sequence[Job], projects: ProjectsMapping) -> S _nodestore_save_many(jobs) _eventstream_insert_many(jobs) _track_outcome_accepted_many(jobs) - _detect_sdk_crashes(jobs) return jobs diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index a8f8671bb9b65d..c9cc23ee0cba6c 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -4,13 +4,26 @@ from sentry.eventstore.models import Event from sentry.utils.glob import glob_match +from sentry.utils.safe import get_path -def detect_sdk_crash(data: Event): - event_id = data.get("event_id", None) - if event_id is None: - return - return +def detect_sdk_crash(data: Event) -> bool: + if data.get("type") != "error" or data.get("platform") != "cocoa": + return False + + is_unhandled = get_path(data, "exception", "values", -1, "mechanism", "handled") is False + + if is_unhandled is False: + return False + + frames = get_path(data, "exception", "values", -1, "stacktrace", "frames") + if not frames: + return False + + if is_cocoa_sdk_crash(frames): + return True + + return False def is_cocoa_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: 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 180163d3701aad..43133a3311073e 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -2,7 +2,7 @@ import pytest -from sentry.utils.sdk_crashes.sdk_crash_detection import is_cocoa_sdk_crash +from sentry.utils.sdk_crashes.sdk_crash_detection import detect_sdk_crash, is_cocoa_sdk_crash in_app_frame = { "function": "LoginViewController.viewDidAppear", @@ -19,6 +19,53 @@ } +def make_crash_event(handled=False, function="-[Sentry]", **kwargs): + result = { + "type": "error", + "platform": "cocoa", + "exception": { + "values": [ + { + "stacktrace": { + "frames": [ + { + "function": function, + "package": "Sentry", + "in_app:": False, + } + ], + }, + "type": "SIGABRT", + "mechanism": {"handled": handled}, + } + ] + }, + } + result.update(kwargs) + return result + + +@pytest.mark.parametrize( + "data,expected", + [ + ( + make_crash_event(), + True, + ), + ( + make_crash_event(handled=True), + False, + ), + (make_crash_event(function="Senry"), False), + (make_crash_event(platform="coco"), False), + (make_crash_event(type="erro"), False), + (make_crash_event(exception=[]), False), + ], +) +def test_process(data, expected): + assert detect_sdk_crash(data) is expected + + @pytest.mark.parametrize( "function,expected", [ From fe89218916648a44473a9fed6dab4e7e59340bc4 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 9 Feb 2023 13:09:53 +0100 Subject: [PATCH 07/37] strip frames --- .../utils/sdk_crashes/sdk_crash_detection.py | 52 ++++--- .../sdk_crashes/test_sdk_crash_detection.py | 131 ++++++++++++------ 2 files changed, 121 insertions(+), 62 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index c9cc23ee0cba6c..7f663fb6ec4704 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -26,6 +26,18 @@ def detect_sdk_crash(data: Event) -> bool: return False +def strip_event_data(data: Event) -> Event: + return data + + +def strip_frames(frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: + return [ + frame + for frame in frames + if _is_cocoa_sdk_frame(frame) or frame.get("in_app", True) is False + ] + + def is_cocoa_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: """ Returns true if the stacktrace is a Cocoa SDK crash. @@ -36,28 +48,34 @@ def is_cocoa_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: return False for frame in frames: + if _is_cocoa_sdk_frame(frame): + return True - function = frame.get("function") + if frame.get("in_app") is True: + return False - if function is not None: - # [SentrySDK crash] is a testing function causing a crash. - # Therefore, we don't want to mark it a as a SDK crash. - if "SentrySDK crash" in function: - return False + return False - functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] - for matcher in functionsMatchers: - if glob_match(frame.get("function"), matcher, ignorecase=True): - return True - filename = frame.get("filename") - if filename is not None: - filenameMatchers = ["Sentry**"] - for matcher in filenameMatchers: - if glob_match(filename, matcher, ignorecase=True): - return True +def _is_cocoa_sdk_frame(frame: Mapping[str, Any]) -> bool: + function = frame.get("function") - if frame.get("in_app") is True: + if function is not None: + # [SentrySDK crash] is a testing function causing a crash. + # Therefore, we don't want to mark it a as a SDK crash. + if "SentrySDK crash" in function: return False + functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] + for matcher in functionsMatchers: + if glob_match(frame.get("function"), matcher, ignorecase=True): + return True + + filename = frame.get("filename") + if filename is not None: + filenameMatchers = ["Sentry**"] + for matcher in filenameMatchers: + if glob_match(filename, matcher, ignorecase=True): + return True + return False 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 43133a3311073e..192756dfcb3e65 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -2,7 +2,11 @@ import pytest -from sentry.utils.sdk_crashes.sdk_crash_detection import detect_sdk_crash, is_cocoa_sdk_crash +from sentry.utils.sdk_crashes.sdk_crash_detection import ( + detect_sdk_crash, + is_cocoa_sdk_crash, + strip_frames, +) in_app_frame = { "function": "LoginViewController.viewDidAppear", @@ -81,48 +85,7 @@ def test_process(data, expected): ], ) def test_cocoa_sdk_crash_detection(function, expected): - frames = [ - create_sentry_frame(function), - { - "function": "LoginViewController.viewDidAppear", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - "lineno": 196, - "in_app": True, - }, - in_app_frame, - { - "function": "-[UIViewController _setViewAppearState:isAnimating:]", - "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController __viewDidAppear:]", - "symbol": "-[UIViewController __viewDidAppear:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController _endAppearanceTransition:]", - "symbol": "-[UIViewController _endAppearanceTransition:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "package": "UIKitCore", - "in_app": False, - }, - ] + frames = get_frames(function) assert is_cocoa_sdk_crash(frames) is expected @@ -233,9 +196,87 @@ def test_is_cocoa_sdk_crash_single_frame(): assert is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True -def create_sentry_frame(function) -> Mapping[str, Any]: +def test_is_cocoa_sdk_crash_single_in_app_frame(): + assert is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]", in_app=True)]) is True + + +def test_strip_frames_removes_in_app(): + frames = get_frames("sentrycrashdl_getBinaryImage") + + stripped_frames = strip_frames(frames) + assert len(stripped_frames) == 6 + assert ( + len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) + == 0 + ), "in_app frame should be removed" + + +@pytest.mark.parametrize( + "function,in_app", + [ + ("SentryCrashMonitor_CPPException.cpp", True), + ("SentryCrashMonitor_CPPException.cpp", False), + ("sentrycrashdl_getBinaryImage", True), + ], +) +def test_strip_frames_keeps_sentry(function, in_app): + frames = get_frames(function, sentry_frame_in_app=in_app) + + stripped_frames = strip_frames(frames) + + assert len(stripped_frames) == 6 + assert ( + len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) + == 0 + ), "in_app frame should be removed" + + +def create_sentry_frame(function, in_app: bool = False) -> Mapping[str, Any]: return { "function": function, "package": "Sentry", - "in_app:": False, + "in_app": in_app, } + + +def get_frames(function: str, sentry_frame_in_app: bool = False): + return [ + create_sentry_frame(function, sentry_frame_in_app), + { + "function": "LoginViewController.viewDidAppear", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + }, + in_app_frame, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController _endAppearanceTransition:]", + "symbol": "-[UIViewController _endAppearanceTransition:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "package": "UIKitCore", + "in_app": False, + }, + ] From db8f089b4c35cf86d3b0059681cf77272d302ca5 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 9 Feb 2023 13:23:37 +0100 Subject: [PATCH 08/37] clarify test method names --- .../sdk_crashes/test_sdk_crash_detection.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) 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 192756dfcb3e65..67aaae3b4fb8e2 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -27,6 +27,7 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): result = { "type": "error", "platform": "cocoa", + "tags": {"level": "fatal"}, "exception": { "values": [ { @@ -65,8 +66,16 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): (make_crash_event(type="erro"), False), (make_crash_event(exception=[]), False), ], + ids=[ + "unhandled_is_detected", + "handled_not_detected", + "wrong_function_not_detected", + "wrong_platform_not_detected", + "wrong_type_not_detected", + "no_exception_not_detected", + ], ) -def test_process(data, expected): +def test_detect_sdk_crash(data, expected): assert detect_sdk_crash(data) is expected @@ -216,10 +225,11 @@ def test_strip_frames_removes_in_app(): [ ("SentryCrashMonitor_CPPException.cpp", True), ("SentryCrashMonitor_CPPException.cpp", False), - ("sentrycrashdl_getBinaryImage", True), + ("sentr", False), ], + ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept", "non_sentry_non_in_app_kept"], ) -def test_strip_frames_keeps_sentry(function, in_app): +def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) stripped_frames = strip_frames(frames) From 0592d1c2c7804aa697e192edd4c09eab6e83fe4d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 9 Feb 2023 13:36:02 +0100 Subject: [PATCH 09/37] add more fields to event --- .../sdk_crashes/test_sdk_crash_detection.py | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) 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 67aaae3b4fb8e2..006362dbb4ce51 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -25,9 +25,15 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): result = { + "event_id": "80e3496eff734ab0ac993167aaa0d1cd", + "project": 5218188, + "release": "5.222.5", "type": "error", + "level": "fatal", "platform": "cocoa", "tags": {"level": "fatal"}, + "datetime": "2023-02-08T23:51:35.000000Z", + "timestamp": 1675936223.0, "exception": { "values": [ { @@ -45,6 +51,138 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): } ] }, + "breadcrumbs": { + "values": [ + { + "timestamp": 1675900265.0, + "type": "debug", + "category": "started", + "level": "info", + "message": "Breadcrumb Tracking", + }, + ] + }, + "contexts": { + "app": { + "app_start_time": "2023-02-08T23:51:05Z", + "device_app_hash": "8854fe9e3d4e4a66493baee798bfae0228efabf1", + "build_type": "app store", + "app_identifier": "com.some.company.io", + "app_name": "SomeCompany", + "app_version": "5.222.5", + "app_build": "21036", + "app_id": "397D4F75-6C01-32D1-BF46-62098979E470", + "type": "app", + }, + "device": { + "family": "iOS", + "model": "iPhone14,8", + "model_id": "D28AP", + "arch": "arm64e", + "memory_size": 5944508416, + "free_memory": 102154240, + "usable_memory": 4125687808, + "storage_size": 127854202880, + "boot_time": "2023-02-01T05:21:23Z", + "timezone": "PST", + "type": "device", + }, + "os": { + "name": "iOS", + "version": "16.3", + "build": "20D47", + "kernel_version": "Darwin Kernel Version 22.3.0: Wed Jan 4 21:25:19 PST 2023; root:xnu-8792.82.2~1/RELEASE_ARM64_T8110", + "rooted": False, + "type": "os", + }, + }, + # Todo add referenced in stacktrace + "debug_meta": { + "images": [ + { + "name": "CrashProbeiOS", + "image_vmaddr": "0x0000000100000000", + "image_addr": "0x0000000100088000", + "type": "apple", + "image_size": 65536, + "uuid": "2C656702-AA16-3E5F-94D9-D4430DA53398", + }, + ] + }, + "environment": "test-app", + "sdk": { + "name": "sentry.cocoa", + "version": "8.1.0", + "integrations": [ + "Crash", + "PerformanceTracking", + "MetricKit", + "WatchdogTerminationTracking", + "ViewHierarchy", + "NetworkTracking", + "ANRTracking", + "AutoBreadcrumbTracking", + "FramesTracking", + "AppStartTracking", + "Screenshot", + "FileIOTracking", + "UIEventTracking", + "AutoSessionTracking", + "CoreDataTracking", + "PreWarmedAppStartTracing", + ], + }, + "threads": { + "values": [ + { + "id": 0, + "stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "raw_stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "crashed": True, + } + ] + }, + "user": { + "id": "803F5C87-0F8B-41C7-8499-27BD71A92738", + "ip_address": "192.168.0.1", + "geo": {"country_code": "US", "region": "United States"}, + }, } result.update(kwargs) return result From 8eda48059209d44a8bf95e0ee1d8b575354814dc Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 13 Feb 2023 10:44:38 +0100 Subject: [PATCH 10/37] use mock --- src/sentry/event_manager.py | 5 +- .../utils/sdk_crashes/sdk_crash_detection.py | 111 ++++++++++-------- .../sdk_crashes/test_sdk_crash_detection.py | 59 +++++++--- 3 files changed, 105 insertions(+), 70 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index c98a5cd633e5be..c99a145eca09a6 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -140,7 +140,7 @@ detect_performance_problems, ) from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim -from sentry.utils.sdk_crashes.sdk_crash_detection import detect_sdk_crash +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter if TYPE_CHECKING: from sentry.eventstore.models import Event @@ -2213,7 +2213,8 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping) def _detect_sdk_crashes(jobs: Sequence[Job]): for job in jobs: - detect_sdk_crash(job["data"]) + sdk_crash_detector = SDKCrashDetection(sdk_crash_reporter=SDKCrashReporter()) + sdk_crash_detector.detect_sdk_crash(job["data"]) class PerformanceJob(TypedDict, total=False): diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 7f663fb6ec4704..8064ac4f0aa0d5 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -7,75 +7,82 @@ from sentry.utils.safe import get_path -def detect_sdk_crash(data: Event) -> bool: - if data.get("type") != "error" or data.get("platform") != "cocoa": - return False +class SDKCrashReporter: + def __init__(self): + self - is_unhandled = get_path(data, "exception", "values", -1, "mechanism", "handled") is False + def report(self, event: Event) -> None: + pass - if is_unhandled is False: - return False - frames = get_path(data, "exception", "values", -1, "stacktrace", "frames") - if not frames: - return False +class SDKCrashDetection: + def __init__(self, sdk_crash_reporter: SDKCrashReporter): + self + self.sdk_crash_reporter = sdk_crash_reporter - if is_cocoa_sdk_crash(frames): - return True + def detect_sdk_crash(self, data: Event) -> None: + if data.get("type") != "error" or data.get("platform") != "cocoa": + return - return False + is_unhandled = get_path(data, "exception", "values", -1, "mechanism", "handled") is False + if is_unhandled is False: + return -def strip_event_data(data: Event) -> Event: - return data + frames = get_path(data, "exception", "values", -1, "stacktrace", "frames") + if not frames: + return + if self.is_cocoa_sdk_crash(frames): + self.sdk_crash_reporter.report(data) -def strip_frames(frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: - return [ - frame - for frame in frames - if _is_cocoa_sdk_frame(frame) or frame.get("in_app", True) is False - ] + def strip_event_data(self, data: Event) -> Event: + return data + def strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: + return [ + frame + for frame in frames + if self._is_cocoa_sdk_frame(frame) or frame.get("in_app", True) is False + ] -def is_cocoa_sdk_crash(frames: Sequence[Mapping[str, Any]]) -> bool: - """ - Returns true if the stacktrace is a Cocoa SDK crash. + def is_cocoa_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: + """ + Returns true if the stacktrace is a Cocoa SDK crash. - :param frames: The stacktrace frames ordered from newest to oldest. - """ - if not frames: - return False - - for frame in frames: - if _is_cocoa_sdk_frame(frame): - return True - - if frame.get("in_app") is True: + :param frames: The stacktrace frames ordered from newest to oldest. + """ + if not frames: return False - return False + for frame in frames: + if self._is_cocoa_sdk_frame(frame): + return True + if frame.get("in_app") is True: + return False -def _is_cocoa_sdk_frame(frame: Mapping[str, Any]) -> bool: - function = frame.get("function") + return False - if function is not None: - # [SentrySDK crash] is a testing function causing a crash. - # Therefore, we don't want to mark it a as a SDK crash. - if "SentrySDK crash" in function: - return False + def _is_cocoa_sdk_frame(self, frame: Mapping[str, Any]) -> bool: + function = frame.get("function") - functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] - for matcher in functionsMatchers: - if glob_match(frame.get("function"), matcher, ignorecase=True): - return True + if function is not None: + # [SentrySDK crash] is a testing function causing a crash. + # Therefore, we don't want to mark it a as a SDK crash. + if "SentrySDK crash" in function: + return False - filename = frame.get("filename") - if filename is not None: - filenameMatchers = ["Sentry**"] - for matcher in filenameMatchers: - if glob_match(filename, matcher, ignorecase=True): - return True + functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] + for matcher in functionsMatchers: + if glob_match(frame.get("function"), matcher, ignorecase=True): + return True + + filename = frame.get("filename") + if filename is not None: + filenameMatchers = ["Sentry**"] + for matcher in filenameMatchers: + if glob_match(filename, matcher, ignorecase=True): + return True - return False + return False 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 006362dbb4ce51..dc077afd93ddca 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,12 +1,9 @@ from typing import Any, Mapping +from unittest.mock import Mock import pytest -from sentry.utils.sdk_crashes.sdk_crash_detection import ( - detect_sdk_crash, - is_cocoa_sdk_crash, - strip_frames, -) +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection in_app_frame = { "function": "LoginViewController.viewDidAppear", @@ -214,7 +211,15 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): ], ) def test_detect_sdk_crash(data, expected): - assert detect_sdk_crash(data) is expected + crash_reporter = Mock() + crash_detector = SDKCrashDetection(crash_reporter) + + crash_detector.detect_sdk_crash(data) + + if expected: + crash_reporter.report.assert_called_once_with(data) + else: + crash_reporter.report.assert_not_called() @pytest.mark.parametrize( @@ -234,7 +239,9 @@ def test_detect_sdk_crash(data, expected): def test_cocoa_sdk_crash_detection(function, expected): frames = get_frames(function) - assert is_cocoa_sdk_crash(frames) is expected + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash(frames) is expected def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): @@ -260,7 +267,9 @@ def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): }, ] - assert is_cocoa_sdk_crash(frames) is True + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash(frames) is True def test_is_cocoa_sdk_crash_only_inapp_after_sentry_frame(): @@ -287,7 +296,9 @@ def test_is_cocoa_sdk_crash_only_inapp_after_sentry_frame(): }, ] - assert is_cocoa_sdk_crash(frames) is False + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash(frames) is False @pytest.mark.parametrize( @@ -328,29 +339,43 @@ def test_is_cocoa_sdk_crash_filename(filename, expected): }, ] - assert is_cocoa_sdk_crash(frames) is expected + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash(frames) is expected def test_is_cocoa_sdk_crash_no_frames(): - assert is_cocoa_sdk_crash([]) is False + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash([]) is False def test_is_cocoa_sdk_crash_empty_frames(): - assert is_cocoa_sdk_crash([{"empty": "frame"}]) is False + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash([{"empty": "frame"}]) is False def test_is_cocoa_sdk_crash_single_frame(): - assert is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True + crash_detector = SDKCrashDetection(Mock()) + + assert crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True def test_is_cocoa_sdk_crash_single_in_app_frame(): - assert is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]", in_app=True)]) is True + crash_detector = SDKCrashDetection(Mock()) + + assert ( + crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]", in_app=True)]) is True + ) def test_strip_frames_removes_in_app(): frames = get_frames("sentrycrashdl_getBinaryImage") - stripped_frames = strip_frames(frames) + crash_detector = SDKCrashDetection(Mock()) + + stripped_frames = crash_detector.strip_frames(frames) assert len(stripped_frames) == 6 assert ( len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) @@ -370,7 +395,9 @@ def test_strip_frames_removes_in_app(): def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) - stripped_frames = strip_frames(frames) + crash_detector = SDKCrashDetection(Mock()) + + stripped_frames = crash_detector.strip_frames(frames) assert len(stripped_frames) == 6 assert ( From 27482bdd55ce059d28aa52d788b22dc8eb28c40e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 13 Feb 2023 11:13:45 +0100 Subject: [PATCH 11/37] Use mocks --- .../utils/sdk_crashes/sdk_crash_detection.py | 2 +- .../sdk_crashes/test_sdk_crash_detection.py | 122 +++++++++--------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 8064ac4f0aa0d5..556e626ed44b42 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -36,7 +36,7 @@ def detect_sdk_crash(self, data: Event) -> None: if self.is_cocoa_sdk_crash(frames): self.sdk_crash_reporter.report(data) - def strip_event_data(self, data: Event) -> Event: + def _strip_event_data(self, data: Event) -> Event: return data def strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: 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 dc077afd93ddca..d91ff1bff916aa 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -20,6 +20,57 @@ } +def create_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: + return { + "function": function, + "package": "Sentry", + "in_app": in_app, + } + + +def get_frames(function: str, sentry_frame_in_app: bool = False): + return [ + create_sentry_frame(function, sentry_frame_in_app), + { + "function": "LoginViewController.viewDidAppear", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + }, + in_app_frame, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController _endAppearanceTransition:]", + "symbol": "-[UIViewController _endAppearanceTransition:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "package": "UIKitCore", + "in_app": False, + }, + ] + + def make_crash_event(handled=False, function="-[Sentry]", **kwargs): result = { "event_id": "80e3496eff734ab0ac993167aaa0d1cd", @@ -35,13 +86,7 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): "values": [ { "stacktrace": { - "frames": [ - { - "function": function, - "package": "Sentry", - "in_app:": False, - } - ], + "frames": get_frames(function), }, "type": "SIGABRT", "mechanism": {"handled": handled}, @@ -237,11 +282,17 @@ def test_detect_sdk_crash(data, expected): ], ) def test_cocoa_sdk_crash_detection(function, expected): - frames = get_frames(function) + data = make_crash_event(function=function) - crash_detector = SDKCrashDetection(Mock()) + crash_reporter = Mock() + crash_detector = SDKCrashDetection(crash_reporter) - assert crash_detector.is_cocoa_sdk_crash(frames) is expected + crash_detector.detect_sdk_crash(data) + + if expected: + crash_reporter.report.assert_called_once_with(data) + else: + crash_reporter.report.assert_not_called() def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): @@ -404,54 +455,3 @@ def test_strip_frames(function, in_app): len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) == 0 ), "in_app frame should be removed" - - -def create_sentry_frame(function, in_app: bool = False) -> Mapping[str, Any]: - return { - "function": function, - "package": "Sentry", - "in_app": in_app, - } - - -def get_frames(function: str, sentry_frame_in_app: bool = False): - return [ - create_sentry_frame(function, sentry_frame_in_app), - { - "function": "LoginViewController.viewDidAppear", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - }, - in_app_frame, - { - "function": "-[UIViewController _setViewAppearState:isAnimating:]", - "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController __viewDidAppear:]", - "symbol": "-[UIViewController __viewDidAppear:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController _endAppearanceTransition:]", - "symbol": "-[UIViewController _endAppearanceTransition:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "package": "UIKitCore", - "in_app": False, - }, - ] From ece3894d7d79862ccf42b396804a660cf85b7876 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 14 Feb 2023 09:22:43 +0100 Subject: [PATCH 12/37] givenCrashDetector --- src/sentry/event_manager.py | 4 +-- .../utils/sdk_crashes/sdk_crash_detection.py | 2 +- .../sdk_crashes/test_sdk_crash_detection.py | 34 +++++++++++-------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index b0ca1a3da87d19..a27a9edaadeeda 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -140,7 +140,7 @@ detect_performance_problems, ) from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter if TYPE_CHECKING: from sentry.eventstore.models import BaseEvent, Event @@ -2243,7 +2243,7 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping) def _detect_sdk_crashes(jobs: Sequence[Job]): for job in jobs: - sdk_crash_detector = SDKCrashDetection(sdk_crash_reporter=SDKCrashReporter()) + sdk_crash_detector = SDKCrashDetector(sdk_crash_reporter=SDKCrashReporter()) sdk_crash_detector.detect_sdk_crash(job["data"]) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 556e626ed44b42..40476fec61fd8d 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -15,7 +15,7 @@ def report(self, event: Event) -> None: pass -class SDKCrashDetection: +class SDKCrashDetector: def __init__(self, sdk_crash_reporter: SDKCrashReporter): self self.sdk_crash_reporter = sdk_crash_reporter 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 d91ff1bff916aa..4fc5809435a1e4 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,9 +1,9 @@ -from typing import Any, Mapping +from typing import Any, Mapping, Tuple from unittest.mock import Mock import pytest -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter in_app_frame = { "function": "LoginViewController.viewDidAppear", @@ -230,6 +230,12 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): return result +def givenCrashDetector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: + crash_reporter = Mock() + crash_detection = SDKCrashDetector(crash_reporter) + return crash_detection, crash_reporter + + @pytest.mark.parametrize( "data,expected", [ @@ -256,8 +262,7 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): ], ) def test_detect_sdk_crash(data, expected): - crash_reporter = Mock() - crash_detector = SDKCrashDetection(crash_reporter) + crash_detector, crash_reporter = givenCrashDetector() crash_detector.detect_sdk_crash(data) @@ -284,8 +289,7 @@ def test_detect_sdk_crash(data, expected): def test_cocoa_sdk_crash_detection(function, expected): data = make_crash_event(function=function) - crash_reporter = Mock() - crash_detector = SDKCrashDetection(crash_reporter) + crash_detector, crash_reporter = givenCrashDetector() crash_detector.detect_sdk_crash(data) @@ -318,7 +322,7 @@ def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): }, ] - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash(frames) is True @@ -347,7 +351,7 @@ def test_is_cocoa_sdk_crash_only_inapp_after_sentry_frame(): }, ] - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash(frames) is False @@ -390,31 +394,31 @@ def test_is_cocoa_sdk_crash_filename(filename, expected): }, ] - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash(frames) is expected def test_is_cocoa_sdk_crash_no_frames(): - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash([]) is False def test_is_cocoa_sdk_crash_empty_frames(): - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash([{"empty": "frame"}]) is False def test_is_cocoa_sdk_crash_single_frame(): - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True def test_is_cocoa_sdk_crash_single_in_app_frame(): - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() assert ( crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]", in_app=True)]) is True @@ -424,7 +428,7 @@ def test_is_cocoa_sdk_crash_single_in_app_frame(): def test_strip_frames_removes_in_app(): frames = get_frames("sentrycrashdl_getBinaryImage") - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() stripped_frames = crash_detector.strip_frames(frames) assert len(stripped_frames) == 6 @@ -446,7 +450,7 @@ def test_strip_frames_removes_in_app(): def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) - crash_detector = SDKCrashDetection(Mock()) + crash_detector, _ = givenCrashDetector() stripped_frames = crash_detector.strip_frames(frames) From 53a48011863e03e2850aa8b17d80f78f6fa1b32a Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 14 Feb 2023 09:23:43 +0100 Subject: [PATCH 13/37] rename to event --- src/sentry/utils/sdk_crashes/sdk_crash_detection.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 40476fec61fd8d..4e17191d53bc21 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -20,21 +20,21 @@ def __init__(self, sdk_crash_reporter: SDKCrashReporter): self self.sdk_crash_reporter = sdk_crash_reporter - def detect_sdk_crash(self, data: Event) -> None: - if data.get("type") != "error" or data.get("platform") != "cocoa": + def detect_sdk_crash(self, event: Event) -> None: + if event.get("type") != "error" or event.get("platform") != "cocoa": return - is_unhandled = get_path(data, "exception", "values", -1, "mechanism", "handled") is False + is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False if is_unhandled is False: return - frames = get_path(data, "exception", "values", -1, "stacktrace", "frames") + frames = get_path(event, "exception", "values", -1, "stacktrace", "frames") if not frames: return if self.is_cocoa_sdk_crash(frames): - self.sdk_crash_reporter.report(data) + self.sdk_crash_reporter.report(event) def _strip_event_data(self, data: Event) -> Event: return data From 7f859e3aacba2991d0167d4ab6f8b66284b2122d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 14 Feb 2023 10:03:10 +0100 Subject: [PATCH 14/37] Make _is_cocoa_sdk_crash private --- .../utils/sdk_crashes/sdk_crash_detection.py | 6 +- .../sdk_crashes/test_sdk_crash_detection.py | 304 +++++++++--------- 2 files changed, 155 insertions(+), 155 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 4e17191d53bc21..96436ebb8cda63 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -21,7 +21,7 @@ def __init__(self, sdk_crash_reporter: SDKCrashReporter): self.sdk_crash_reporter = sdk_crash_reporter def detect_sdk_crash(self, event: Event) -> None: - if event.get("type") != "error" or event.get("platform") != "cocoa": + if event.get("type", None) != "error" or event.get("platform") != "cocoa": return is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False @@ -33,7 +33,7 @@ def detect_sdk_crash(self, event: Event) -> None: if not frames: return - if self.is_cocoa_sdk_crash(frames): + if self._is_cocoa_sdk_crash(frames): self.sdk_crash_reporter.report(event) def _strip_event_data(self, data: Event) -> Event: @@ -46,7 +46,7 @@ def strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[ if self._is_cocoa_sdk_frame(frame) or frame.get("in_app", True) is False ] - def is_cocoa_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: + def _is_cocoa_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: """ Returns true if the stacktrace is a Cocoa SDK crash. 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 4fc5809435a1e4..c94380f63f7b40 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,4 +1,4 @@ -from typing import Any, Mapping, Tuple +from typing import Any, Mapping, Sequence, Tuple from unittest.mock import Mock import pytest @@ -28,7 +28,7 @@ def create_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any } -def get_frames(function: str, sentry_frame_in_app: bool = False): +def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: return [ create_sentry_frame(function, sentry_frame_in_app), { @@ -71,7 +71,13 @@ def get_frames(function: str, sentry_frame_in_app: bool = False): ] -def make_crash_event(handled=False, function="-[Sentry]", **kwargs): +def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: + return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) + + +def get_crash_event_with_frames( + frames: Sequence[Mapping[str, Any]], handled=False, **kwargs +) -> Sequence[Mapping[str, Any]]: result = { "event_id": "80e3496eff734ab0ac993167aaa0d1cd", "project": 5218188, @@ -86,7 +92,7 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): "values": [ { "stacktrace": { - "frames": get_frames(function), + "frames": frames, }, "type": "SIGABRT", "mechanism": {"handled": handled}, @@ -230,27 +236,21 @@ def make_crash_event(handled=False, function="-[Sentry]", **kwargs): return result -def givenCrashDetector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: - crash_reporter = Mock() - crash_detection = SDKCrashDetector(crash_reporter) - return crash_detection, crash_reporter - - @pytest.mark.parametrize( - "data,expected", + "event,should_be_reported", [ ( - make_crash_event(), + get_crash_event(), True, ), ( - make_crash_event(handled=True), + get_crash_event(handled=True), False, ), - (make_crash_event(function="Senry"), False), - (make_crash_event(platform="coco"), False), - (make_crash_event(type="erro"), False), - (make_crash_event(exception=[]), False), + (get_crash_event(function="Senry"), False), + (get_crash_event(platform="coco"), False), + (get_crash_event(type="erro"), False), + (get_crash_event(exception=[]), False), ], ids=[ "unhandled_is_detected", @@ -261,19 +261,12 @@ def givenCrashDetector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: "no_exception_not_detected", ], ) -def test_detect_sdk_crash(data, expected): - crash_detector, crash_reporter = givenCrashDetector() - - crash_detector.detect_sdk_crash(data) - - if expected: - crash_reporter.report.assert_called_once_with(data) - else: - crash_reporter.report.assert_not_called() +def test_detect_sdk_crash(event, should_be_reported): + _run_report_test_with_event(event, should_be_reported) @pytest.mark.parametrize( - "function,expected", + "function,should_be_reported", [ ("-[SentryHub getScope]", True), ("sentrycrashdl_getBinaryImage", True), @@ -286,149 +279,131 @@ def test_detect_sdk_crash(data, expected): ("+[SentrySDK crash]", False), ], ) -def test_cocoa_sdk_crash_detection(function, expected): - data = make_crash_event(function=function) - - crash_detector, crash_reporter = givenCrashDetector() - - crash_detector.detect_sdk_crash(data) - - if expected: - crash_reporter.report.assert_called_once_with(data) - else: - crash_reporter.report.assert_not_called() - - -def test_is_cocoa_sdk_crash_only_non_inapp_after_sentry_frame(): - frames = [ - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", - "in_app": False, - }, - { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), - { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", - "in_app": False, - }, - ] - - crash_detector, _ = givenCrashDetector() - - assert crash_detector.is_cocoa_sdk_crash(frames) is True +def test_cocoa_sdk_crash_detection(function, should_be_reported): + event = get_crash_event(function=function) - -def test_is_cocoa_sdk_crash_only_inapp_after_sentry_frame(): - frames = [ - in_app_frame, - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", - "in_app": False, - }, - { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), - { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", - "in_app": False, - }, - ] - - crash_detector, _ = givenCrashDetector() - - assert crash_detector.is_cocoa_sdk_crash(frames) is False + _run_report_test_with_event(event, should_be_reported) @pytest.mark.parametrize( - "filename,expected", + "filename,should_be_reported", [ ("SentryCrashMonitor_CPPException.cpp", True), ("SentryMonitor_CPPException.cpp", True), ("SentrMonitor_CPPException.cpp", False), ], ) -def test_is_cocoa_sdk_crash_filename(filename, expected): - frames = [ - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", - "in_app": False, - }, - { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - { - "function": "CPPExceptionTerminate", - "raw_function": "CPPExceptionTerminate()", - "filename": filename, - "symbol": "_ZL21CPPExceptionTerminatev", - "package": "MainApp", - "in_app": False, - }, - { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", - "in_app": False, - }, - ] - - crash_detector, _ = givenCrashDetector() - - assert crash_detector.is_cocoa_sdk_crash(frames) is expected - - -def test_is_cocoa_sdk_crash_no_frames(): - crash_detector, _ = givenCrashDetector() - - assert crash_detector.is_cocoa_sdk_crash([]) is False - - -def test_is_cocoa_sdk_crash_empty_frames(): - crash_detector, _ = givenCrashDetector() - - assert crash_detector.is_cocoa_sdk_crash([{"empty": "frame"}]) is False - - -def test_is_cocoa_sdk_crash_single_frame(): - crash_detector, _ = givenCrashDetector() +def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): + event = get_crash_event_with_frames( + frames=[ + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + { + "function": "CPPExceptionTerminate", + "raw_function": "CPPExceptionTerminate()", + "filename": filename, + "symbol": "_ZL21CPPExceptionTerminatev", + "package": "MainApp", + "in_app": False, + }, + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + ] + ) - assert crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]")]) is True + _run_report_test_with_event(event, should_be_reported) -def test_is_cocoa_sdk_crash_single_in_app_frame(): - crash_detector, _ = givenCrashDetector() +@pytest.mark.parametrize( + "frames,should_be_reported", + [ + ([], False), + ([{"empty": "frame"}], False), + ([create_sentry_frame("-[Sentry]")], True), + ([create_sentry_frame("-[Sentry]", in_app=True)], True), + ( + [ + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + ], + True, + ), + ( + [ + in_app_frame, + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + create_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + ], + False, + ), + ], + ids=[ + "no_frames_not_detected", + "empty_frame_not_detected", + "single_frame_is_detected", + "single_in_app_frame_is_detected", + "only_non_inapp_after_sentry_frame_is_detected", + "only_inapp_after_sentry_frame_not_detected", + ], +) +def test_report_cocoa_sdk_crash_frames(frames, should_be_reported): + event = get_crash_event_with_frames(frames) - assert ( - crash_detector.is_cocoa_sdk_crash([create_sentry_frame("-[Sentry]", in_app=True)]) is True - ) + _run_report_test_with_event(event, should_be_reported) def test_strip_frames_removes_in_app(): frames = get_frames("sentrycrashdl_getBinaryImage") - crash_detector, _ = givenCrashDetector() + crash_detector, _ = given_crash_detector() stripped_frames = crash_detector.strip_frames(frames) assert len(stripped_frames) == 6 @@ -450,7 +425,7 @@ def test_strip_frames_removes_in_app(): def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) - crash_detector, _ = givenCrashDetector() + crash_detector, _ = given_crash_detector() stripped_frames = crash_detector.strip_frames(frames) @@ -459,3 +434,28 @@ def test_strip_frames(function, in_app): len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) == 0 ), "in_app frame should be removed" + + +def given_crash_detector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: + crash_reporter = Mock() + crash_detection = SDKCrashDetector(crash_reporter) + return crash_detection, crash_reporter + + +def assert_sdk_crash_reported(crash_reporter: SDKCrashReporter, expected_data: dict): + crash_reporter.report.assert_called_once_with(expected_data) + + +def _run_report_test_with_event(event, should_be_reported): + crash_detector, crash_reporter = given_crash_detector() + + crash_detector.detect_sdk_crash(event) + + if should_be_reported: + assert_sdk_crash_reported(crash_reporter, event) + else: + assert_no_sdk_crash_reported(crash_reporter) + + +def assert_no_sdk_crash_reported(crash_reporter: SDKCrashReporter): + crash_reporter.report.assert_not_called() From 83a578d666e0a74659a83390b77b4eaa59fe868f Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 14 Feb 2023 11:39:29 +0100 Subject: [PATCH 15/37] add event stripper --- .../utils/sdk_crashes/event_stripper.py | 40 +++ .../utils/sdk_crashes/sdk_crash_detection.py | 10 +- .../utils/sdk_crashes/test_event_stripper.py | 40 +++ .../sentry/utils/sdk_crashes/test_fixture.py | 232 ++++++++++++++++ .../sdk_crashes/test_sdk_crash_detection.py | 261 ++---------------- 5 files changed, 338 insertions(+), 245 deletions(-) create mode 100644 src/sentry/utils/sdk_crashes/event_stripper.py create mode 100644 tests/sentry/utils/sdk_crashes/test_event_stripper.py create mode 100644 tests/sentry/utils/sdk_crashes/test_fixture.py diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py new file mode 100644 index 00000000000000..8b841758e3dc0c --- /dev/null +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -0,0 +1,40 @@ +from sentry.eventstore.models import Event + + +class EventStripper: + def __init__(self): + self + + 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())) + + 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 diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 96436ebb8cda63..0756751ca5ea93 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -5,6 +5,7 @@ from sentry.eventstore.models import Event from sentry.utils.glob import glob_match from sentry.utils.safe import get_path +from sentry.utils.sdk_crashes.event_stripper import EventStripper class SDKCrashReporter: @@ -16,9 +17,10 @@ def report(self, event: Event) -> None: class SDKCrashDetector: - def __init__(self, sdk_crash_reporter: SDKCrashReporter): + def __init__(self, sdk_crash_reporter: SDKCrashReporter, event_stripper: EventStripper): self self.sdk_crash_reporter = sdk_crash_reporter + self.event_stripper = event_stripper def detect_sdk_crash(self, event: Event) -> None: if event.get("type", None) != "error" or event.get("platform") != "cocoa": @@ -34,10 +36,8 @@ def detect_sdk_crash(self, event: Event) -> None: return if self._is_cocoa_sdk_crash(frames): - self.sdk_crash_reporter.report(event) - - def _strip_event_data(self, data: Event) -> Event: - return data + sdk_crash_event = self.event_stripper.strip_event_data(event) + self.sdk_crash_reporter.report(sdk_crash_event) def strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: return [ diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py new file mode 100644 index 00000000000000..7f4f3194092ae9 --- /dev/null +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -0,0 +1,40 @@ +from sentry.utils.sdk_crashes.event_stripper import EventStripper +from tests.sentry.utils.sdk_crashes.test_fixture import get_crash_event + + +def test_strip_event_data_keeps_allowed_keys(): + event_stripper = EventStripper() + + stripped_event = event_stripper.strip_event_data(get_crash_event()) + + keys_removed = {"tags", "user", "threads", "breadcrumbs", "environment"} + for key in keys_removed: + assert stripped_event.get(key) is None + + keys_kept = { + "type", + "datetime", + "timestamp", + "platform", + "sdk", + "level", + "logger", + "exception", + "debug_meta", + "contexts", + } + + for key in keys_kept: + assert stripped_event.get(key) is not None + + +def test_strip_event_data_strips_context(): + event_stripper = EventStripper() + + stripped_event = event_stripper.strip_event_data(get_crash_event()) + + contexts = stripped_event.get("contexts") + assert contexts is not None + assert contexts.get("app") is None + assert contexts.get("os") is not None + assert contexts.get("device") is not None diff --git a/tests/sentry/utils/sdk_crashes/test_fixture.py b/tests/sentry/utils/sdk_crashes/test_fixture.py new file mode 100644 index 00000000000000..e8c43c77e25a3b --- /dev/null +++ b/tests/sentry/utils/sdk_crashes/test_fixture.py @@ -0,0 +1,232 @@ +from typing import Any, Mapping, Sequence + +IN_APP_FRAME = { + "function": "LoginViewController.viewDidAppear", + "raw_function": "LoginViewController.viewDidAppear(Bool)", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", + "lineno": 196, + "in_app": True, + "image_addr": "0x1025e8000", + "instruction_addr": "0x102b16630", + "symbol_addr": "0x1025e8000", +} + + +def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: + return { + "function": function, + "package": "Sentry", + "in_app": in_app, + } + + +def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: + return [ + get_sentry_frame(function, sentry_frame_in_app), + { + "function": "LoginViewController.viewDidAppear", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + }, + IN_APP_FRAME, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UIViewController _endAppearanceTransition:]", + "symbol": "-[UIViewController _endAppearanceTransition:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "package": "UIKitCore", + "in_app": False, + }, + { + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "package": "UIKitCore", + "in_app": False, + }, + ] + + +def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: + return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) + + +def get_crash_event_with_frames( + frames: Sequence[Mapping[str, Any]], handled=False, **kwargs +) -> Sequence[Mapping[str, Any]]: + result = { + "event_id": "80e3496eff734ab0ac993167aaa0d1cd", + "project": 5218188, + "release": "5.222.5", + "type": "error", + "level": "fatal", + "platform": "cocoa", + "tags": {"level": "fatal"}, + "datetime": "2023-02-08T23:51:35.000000Z", + "timestamp": 1675936223.0, + "exception": { + "values": [ + { + "stacktrace": { + "frames": frames, + }, + "type": "SIGABRT", + "mechanism": {"handled": handled}, + } + ] + }, + "breadcrumbs": { + "values": [ + { + "timestamp": 1675900265.0, + "type": "debug", + "category": "started", + "level": "info", + "message": "Breadcrumb Tracking", + }, + ] + }, + "contexts": { + "app": { + "app_start_time": "2023-02-08T23:51:05Z", + "device_app_hash": "8854fe9e3d4e4a66493baee798bfae0228efabf1", + "build_type": "app store", + "app_identifier": "com.some.company.io", + "app_name": "SomeCompany", + "app_version": "5.222.5", + "app_build": "21036", + "app_id": "397D4F75-6C01-32D1-BF46-62098979E470", + "type": "app", + }, + "device": { + "family": "iOS", + "model": "iPhone14,8", + "model_id": "D28AP", + "arch": "arm64e", + "memory_size": 5944508416, + "free_memory": 102154240, + "usable_memory": 4125687808, + "storage_size": 127854202880, + "boot_time": "2023-02-01T05:21:23Z", + "timezone": "PST", + "type": "device", + }, + "os": { + "name": "iOS", + "version": "16.3", + "build": "20D47", + "kernel_version": "Darwin Kernel Version 22.3.0: Wed Jan 4 21:25:19 PST 2023; root:xnu-8792.82.2~1/RELEASE_ARM64_T8110", + "rooted": False, + "type": "os", + }, + }, + # Todo add referenced in stacktrace + "debug_meta": { + "images": [ + { + "name": "CrashProbeiOS", + "image_vmaddr": "0x0000000100000000", + "image_addr": "0x0000000100088000", + "type": "apple", + "image_size": 65536, + "uuid": "2C656702-AA16-3E5F-94D9-D4430DA53398", + }, + ] + }, + "environment": "test-app", + "sdk": { + "name": "sentry.cocoa", + "version": "8.1.0", + "integrations": [ + "Crash", + "PerformanceTracking", + "MetricKit", + "WatchdogTerminationTracking", + "ViewHierarchy", + "NetworkTracking", + "ANRTracking", + "AutoBreadcrumbTracking", + "FramesTracking", + "AppStartTracking", + "Screenshot", + "FileIOTracking", + "UIEventTracking", + "AutoSessionTracking", + "CoreDataTracking", + "PreWarmedAppStartTracing", + ], + }, + "threads": { + "values": [ + { + "id": 0, + "stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "raw_stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "crashed": True, + } + ] + }, + "user": { + "id": "803F5C87-0F8B-41C7-8499-27BD71A92738", + "ip_address": "192.168.0.1", + "geo": {"country_code": "US", "region": "United States"}, + }, + "logger": "my.logger.name", + } + result.update(kwargs) + return result 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 c94380f63f7b40..30332d11268e3c 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,239 +1,16 @@ -from typing import Any, Mapping, Sequence, Tuple -from unittest.mock import Mock +from typing import Tuple +from unittest.mock import MagicMock, Mock import pytest from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter - -in_app_frame = { - "function": "LoginViewController.viewDidAppear", - "raw_function": "LoginViewController.viewDidAppear(Bool)", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", - "lineno": 196, - "in_app": True, - "image_addr": "0x1025e8000", - "instruction_addr": "0x102b16630", - "symbol_addr": "0x1025e8000", -} - - -def create_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: - return { - "function": function, - "package": "Sentry", - "in_app": in_app, - } - - -def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: - return [ - create_sentry_frame(function, sentry_frame_in_app), - { - "function": "LoginViewController.viewDidAppear", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - }, - in_app_frame, - { - "function": "-[UIViewController _setViewAppearState:isAnimating:]", - "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController __viewDidAppear:]", - "symbol": "-[UIViewController __viewDidAppear:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UIViewController _endAppearanceTransition:]", - "symbol": "-[UIViewController _endAppearanceTransition:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "package": "UIKitCore", - "in_app": False, - }, - { - "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "package": "UIKitCore", - "in_app": False, - }, - ] - - -def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: - return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) - - -def get_crash_event_with_frames( - frames: Sequence[Mapping[str, Any]], handled=False, **kwargs -) -> Sequence[Mapping[str, Any]]: - result = { - "event_id": "80e3496eff734ab0ac993167aaa0d1cd", - "project": 5218188, - "release": "5.222.5", - "type": "error", - "level": "fatal", - "platform": "cocoa", - "tags": {"level": "fatal"}, - "datetime": "2023-02-08T23:51:35.000000Z", - "timestamp": 1675936223.0, - "exception": { - "values": [ - { - "stacktrace": { - "frames": frames, - }, - "type": "SIGABRT", - "mechanism": {"handled": handled}, - } - ] - }, - "breadcrumbs": { - "values": [ - { - "timestamp": 1675900265.0, - "type": "debug", - "category": "started", - "level": "info", - "message": "Breadcrumb Tracking", - }, - ] - }, - "contexts": { - "app": { - "app_start_time": "2023-02-08T23:51:05Z", - "device_app_hash": "8854fe9e3d4e4a66493baee798bfae0228efabf1", - "build_type": "app store", - "app_identifier": "com.some.company.io", - "app_name": "SomeCompany", - "app_version": "5.222.5", - "app_build": "21036", - "app_id": "397D4F75-6C01-32D1-BF46-62098979E470", - "type": "app", - }, - "device": { - "family": "iOS", - "model": "iPhone14,8", - "model_id": "D28AP", - "arch": "arm64e", - "memory_size": 5944508416, - "free_memory": 102154240, - "usable_memory": 4125687808, - "storage_size": 127854202880, - "boot_time": "2023-02-01T05:21:23Z", - "timezone": "PST", - "type": "device", - }, - "os": { - "name": "iOS", - "version": "16.3", - "build": "20D47", - "kernel_version": "Darwin Kernel Version 22.3.0: Wed Jan 4 21:25:19 PST 2023; root:xnu-8792.82.2~1/RELEASE_ARM64_T8110", - "rooted": False, - "type": "os", - }, - }, - # Todo add referenced in stacktrace - "debug_meta": { - "images": [ - { - "name": "CrashProbeiOS", - "image_vmaddr": "0x0000000100000000", - "image_addr": "0x0000000100088000", - "type": "apple", - "image_size": 65536, - "uuid": "2C656702-AA16-3E5F-94D9-D4430DA53398", - }, - ] - }, - "environment": "test-app", - "sdk": { - "name": "sentry.cocoa", - "version": "8.1.0", - "integrations": [ - "Crash", - "PerformanceTracking", - "MetricKit", - "WatchdogTerminationTracking", - "ViewHierarchy", - "NetworkTracking", - "ANRTracking", - "AutoBreadcrumbTracking", - "FramesTracking", - "AppStartTracking", - "Screenshot", - "FileIOTracking", - "UIEventTracking", - "AutoSessionTracking", - "CoreDataTracking", - "PreWarmedAppStartTracing", - ], - }, - "threads": { - "values": [ - { - "id": 0, - "stacktrace": { - "frames": [ - { - "function": "", - "in_app": False, - "data": {"symbolicator_status": "unknown_image"}, - "image_addr": "0x0", - "instruction_addr": "0x1129be52e", - "symbol_addr": "0x0", - }, - { - "function": "", - "in_app": False, - "data": {"symbolicator_status": "unknown_image"}, - "image_addr": "0x0", - "instruction_addr": "0x104405f21", - "symbol_addr": "0x0", - }, - ], - }, - "raw_stacktrace": { - "frames": [ - { - "function": "", - "in_app": False, - "image_addr": "0x0", - "instruction_addr": "0x1129be52e", - "symbol_addr": "0x0", - }, - { - "function": "", - "in_app": False, - "image_addr": "0x0", - "instruction_addr": "0x104405f21", - "symbol_addr": "0x0", - }, - ], - }, - "crashed": True, - } - ] - }, - "user": { - "id": "803F5C87-0F8B-41C7-8499-27BD71A92738", - "ip_address": "192.168.0.1", - "geo": {"country_code": "US", "region": "United States"}, - }, - } - result.update(kwargs) - return result +from tests.sentry.utils.sdk_crashes.test_fixture import ( + IN_APP_FRAME, + get_crash_event, + get_crash_event_with_frames, + get_frames, + get_sentry_frame, +) @pytest.mark.parametrize( @@ -333,8 +110,8 @@ def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): [ ([], False), ([{"empty": "frame"}], False), - ([create_sentry_frame("-[Sentry]")], True), - ([create_sentry_frame("-[Sentry]", in_app=True)], True), + ([get_sentry_frame("-[Sentry]")], True), + ([get_sentry_frame("-[Sentry]", in_app=True)], True), ( [ { @@ -349,7 +126,7 @@ def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): "package": "libobjc.A.dylib", "in_app": False, }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), + get_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "std::__terminate", "symbol": "_ZSt11__terminatePFvvE", @@ -361,7 +138,7 @@ def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): ), ( [ - in_app_frame, + IN_APP_FRAME, { "function": "__handleUncaughtException", "symbol": "__handleUncaughtException", @@ -374,7 +151,7 @@ def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): "package": "libobjc.A.dylib", "in_app": False, }, - create_sentry_frame("sentrycrashdl_getBinaryImage"), + get_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "std::__terminate", "symbol": "_ZSt11__terminatePFvvE", @@ -408,7 +185,7 @@ def test_strip_frames_removes_in_app(): stripped_frames = crash_detector.strip_frames(frames) assert len(stripped_frames) == 6 assert ( - len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) + len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) == 0 ), "in_app frame should be removed" @@ -431,14 +208,18 @@ def test_strip_frames(function, in_app): assert len(stripped_frames) == 6 assert ( - len([frame for frame in stripped_frames if frame["function"] == in_app_frame["function"]]) + len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) == 0 ), "in_app frame should be removed" def given_crash_detector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: crash_reporter = Mock() - crash_detection = SDKCrashDetector(crash_reporter) + event_stripper = Mock() + event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) + + crash_detection = SDKCrashDetector(crash_reporter, event_stripper) + return crash_detection, crash_reporter From be86e3710f5e40c95c6289292d921ab2c04ded1a Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 22 Mar 2023 11:19:26 +0100 Subject: [PATCH 16/37] strip frames before calling report --- .../utils/sdk_crashes/sdk_crash_detection.py | 5 +-- .../sdk_crashes/test_sdk_crash_detection.py | 33 +++++++------------ 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 0756751ca5ea93..5d7380e374be04 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -27,7 +27,6 @@ def detect_sdk_crash(self, event: Event) -> None: return is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False - if is_unhandled is False: return @@ -37,9 +36,11 @@ def detect_sdk_crash(self, event: Event) -> None: if self._is_cocoa_sdk_crash(frames): sdk_crash_event = self.event_stripper.strip_event_data(event) + stripped_frames = self._strip_frames(frames) + sdk_crash_event["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames self.sdk_crash_reporter.report(sdk_crash_event) - def strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: + def _strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: return [ frame for frame in frames 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 30332d11268e3c..df38b440eb4060 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -3,6 +3,7 @@ import pytest +from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, @@ -177,34 +178,24 @@ def test_report_cocoa_sdk_crash_frames(frames, should_be_reported): _run_report_test_with_event(event, should_be_reported) -def test_strip_frames_removes_in_app(): - frames = get_frames("sentrycrashdl_getBinaryImage") - - crash_detector, _ = given_crash_detector() - - stripped_frames = crash_detector.strip_frames(frames) - assert len(stripped_frames) == 6 - assert ( - len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) - == 0 - ), "in_app frame should be removed" - - @pytest.mark.parametrize( "function,in_app", [ ("SentryCrashMonitor_CPPException.cpp", True), ("SentryCrashMonitor_CPPException.cpp", False), - ("sentr", False), ], - ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept", "non_sentry_non_in_app_kept"], + ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept"], ) def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) + event = get_crash_event_with_frames(frames) - crash_detector, _ = given_crash_detector() + crash_detector, crash_reporter = given_crash_detector() + crash_detector.detect_sdk_crash(event) - stripped_frames = crash_detector.strip_frames(frames) + crash_reporter.report.assert_called_once() + reported_event = crash_reporter.report.call_args.args[0] + stripped_frames = get_path(reported_event, "exception", "values", -1, "stacktrace", "frames") assert len(stripped_frames) == 6 assert ( @@ -223,10 +214,6 @@ def given_crash_detector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: return crash_detection, crash_reporter -def assert_sdk_crash_reported(crash_reporter: SDKCrashReporter, expected_data: dict): - crash_reporter.report.assert_called_once_with(expected_data) - - def _run_report_test_with_event(event, should_be_reported): crash_detector, crash_reporter = given_crash_detector() @@ -238,5 +225,9 @@ def _run_report_test_with_event(event, should_be_reported): assert_no_sdk_crash_reported(crash_reporter) +def assert_sdk_crash_reported(crash_reporter: SDKCrashReporter, expected_data: dict): + crash_reporter.report.assert_called_once_with(expected_data) + + def assert_no_sdk_crash_reported(crash_reporter: SDKCrashReporter): crash_reporter.report.assert_not_called() From 4216b94d00e6b36bfed92dc703ad753cd3b23d9f Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 29 Mar 2023 09:34:07 +0200 Subject: [PATCH 17/37] add base class --- src/sentry/event_manager.py | 4 +- .../utils/sdk_crashes/event_stripper.py | 27 +++++- .../utils/sdk_crashes/sdk_crash_detection.py | 83 +++++++++++-------- .../sdk_crashes/test_sdk_crash_detection.py | 12 ++- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 341c4f5afb9a3f..a4676be1cde2d3 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -144,7 +144,7 @@ detect_performance_problems, ) from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter if TYPE_CHECKING: from sentry.eventstore.models import BaseEvent, Event @@ -2271,7 +2271,7 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping) def _detect_sdk_crashes(jobs: Sequence[Job]): for job in jobs: - sdk_crash_detector = SDKCrashDetector(sdk_crash_reporter=SDKCrashReporter()) + sdk_crash_detector = SDKCrashDetection(sdk_crash_reporter=SDKCrashReporter()) sdk_crash_detector.detect_sdk_crash(job["data"]) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 8b841758e3dc0c..83e6a9646b6285 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -1,9 +1,17 @@ +from typing import Any, Mapping, Sequence + from sentry.eventstore.models import Event +from sentry.utils.safe import get_path +from sentry.utils.sdk_crashes.sdk_crash_detection import CocoaSDKCrashDetector class EventStripper: - def __init__(self): + def __init__( + self, + cocoa_sdk_crash_detector: CocoaSDKCrashDetector, + ): self + self.cocoa_sdk_crash_detector = cocoa_sdk_crash_detector ALLOWED_EVENT_KEYS = { "type", @@ -19,11 +27,13 @@ def __init__(self): } 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())) + frames = get_path(event, "exception", "values", -1, "stacktrace", "frames") + stripped_frames = self._strip_frames(frames) + new_event["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames + return new_event def _filter_event(self, pair): @@ -38,3 +48,14 @@ def _filter_contexts(self, pair): if key in {"os", "device"}: return True return False + + 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.cocoa_sdk_crash_detector.is_sdk_frame(frame) + or frame.get("in_app", True) is False + ] diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 5d7380e374be04..c1dab855b3494a 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -1,5 +1,6 @@ from __future__ import annotations +from abc import ABC, abstractmethod from typing import Any, Mapping, Sequence from sentry.eventstore.models import Event @@ -16,48 +17,35 @@ def report(self, event: Event) -> None: pass -class SDKCrashDetector: - def __init__(self, sdk_crash_reporter: SDKCrashReporter, event_stripper: EventStripper): - self - self.sdk_crash_reporter = sdk_crash_reporter - self.event_stripper = event_stripper +class SDKCrashDetector(ABC): + @abstractmethod + def init(self): + raise NotImplementedError - def detect_sdk_crash(self, event: Event) -> None: - if event.get("type", None) != "error" or event.get("platform") != "cocoa": - return - - is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False - if is_unhandled is False: - return + @abstractmethod + def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: + """ + Returns true if the stacktrace stems from an SDK crash. - frames = get_path(event, "exception", "values", -1, "stacktrace", "frames") - if not frames: - return + :param frames: The stacktrace frames ordered from newest to oldest. + """ + raise NotImplementedError - if self._is_cocoa_sdk_crash(frames): - sdk_crash_event = self.event_stripper.strip_event_data(event) - stripped_frames = self._strip_frames(frames) - sdk_crash_event["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames - self.sdk_crash_reporter.report(sdk_crash_event) + @abstractmethod + def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: + raise NotImplementedError - def _strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping[str, Any]]: - return [ - frame - for frame in frames - if self._is_cocoa_sdk_frame(frame) or frame.get("in_app", True) is False - ] - def _is_cocoa_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: - """ - Returns true if the stacktrace is a Cocoa SDK crash. +class CocoaSDKCrashDetector(SDKCrashDetector): + def __init__(self): + self - :param frames: The stacktrace frames ordered from newest to oldest. - """ + def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: if not frames: return False for frame in frames: - if self._is_cocoa_sdk_frame(frame): + if self.is_sdk_frame(frame): return True if frame.get("in_app") is True: @@ -65,7 +53,7 @@ def _is_cocoa_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: return False - def _is_cocoa_sdk_frame(self, frame: Mapping[str, Any]) -> bool: + def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: function = frame.get("function") if function is not None: @@ -87,3 +75,32 @@ def _is_cocoa_sdk_frame(self, frame: Mapping[str, Any]) -> bool: return True return False + + +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": + return + + is_unhandled = get_path(event, "exception", "values", -1, "mechanism", "handled") is False + if is_unhandled is False: + return + + frames = get_path(event, "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) + self.sdk_crash_reporter.report(sdk_crash_event) 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 df38b440eb4060..7d6049c3afbacc 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -4,7 +4,11 @@ import pytest from sentry.utils.safe import get_path -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetector, SDKCrashReporter +from sentry.utils.sdk_crashes.sdk_crash_detection import ( + CocoaSDKCrashDetector, + SDKCrashDetection, + SDKCrashReporter, +) from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, get_crash_event, @@ -204,12 +208,14 @@ def test_strip_frames(function, in_app): ), "in_app frame should be removed" -def given_crash_detector() -> Tuple[SDKCrashDetector, SDKCrashReporter]: +def given_crash_detector() -> Tuple[SDKCrashDetection, SDKCrashReporter]: crash_reporter = Mock() event_stripper = Mock() + cocoa_sdk_crash_detector = CocoaSDKCrashDetector() + event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) - crash_detection = SDKCrashDetector(crash_reporter, event_stripper) + crash_detection = SDKCrashDetection(crash_reporter, cocoa_sdk_crash_detector, event_stripper) return crash_detection, crash_reporter From b9e95eb26856746f9b1ab17e7a64dc6b83339a1c Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 29 Mar 2023 09:43:43 +0200 Subject: [PATCH 18/37] split up into files --- .../sdk_crashes/cocoa_sdk_crash_detector.py | 45 +++++++++++++ .../utils/sdk_crashes/event_stripper.py | 9 ++- .../utils/sdk_crashes/sdk_crash_detection.py | 65 +------------------ .../utils/sdk_crashes/sdk_crash_detector.py | 21 ++++++ .../sdk_crashes/test_sdk_crash_detection.py | 7 +- 5 files changed, 73 insertions(+), 74 deletions(-) create mode 100644 src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py create mode 100644 src/sentry/utils/sdk_crashes/sdk_crash_detector.py diff --git a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py new file mode 100644 index 00000000000000..86370ce1ab74f8 --- /dev/null +++ b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py @@ -0,0 +1,45 @@ +from typing import Any, Mapping, Sequence + +from sentry.utils.glob import glob_match +from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector + + +class CocoaSDKCrashDetector(SDKCrashDetector): + def __init__(self): + self + + def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: + if not frames: + return False + + for frame in frames: + if self.is_sdk_frame(frame): + return True + + if frame.get("in_app") is True: + return False + + return False + + def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: + function = frame.get("function") + + if function is not None: + # [SentrySDK crash] is a testing function causing a crash. + # Therefore, we don't want to mark it a as a SDK crash. + if "SentrySDK crash" in function: + return False + + functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] + for matcher in functionsMatchers: + if glob_match(frame.get("function"), matcher, ignorecase=True): + return True + + filename = frame.get("filename") + if filename is not None: + filenameMatchers = ["Sentry**"] + for matcher in filenameMatchers: + if glob_match(filename, matcher, ignorecase=True): + return True + + return False diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 83e6a9646b6285..010e5305da74b7 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -2,16 +2,16 @@ from sentry.eventstore.models import Event from sentry.utils.safe import get_path -from sentry.utils.sdk_crashes.sdk_crash_detection import CocoaSDKCrashDetector +from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector class EventStripper: def __init__( self, - cocoa_sdk_crash_detector: CocoaSDKCrashDetector, + sdk_crash_detector: SDKCrashDetector, ): self - self.cocoa_sdk_crash_detector = cocoa_sdk_crash_detector + self.sdk_crash_detector = sdk_crash_detector ALLOWED_EVENT_KEYS = { "type", @@ -56,6 +56,5 @@ def _strip_frames(self, frames: Sequence[Mapping[str, Any]]) -> Sequence[Mapping return [ frame for frame in frames - if self.cocoa_sdk_crash_detector.is_sdk_frame(frame) - or frame.get("in_app", True) is False + if self.sdk_crash_detector.is_sdk_frame(frame) or frame.get("in_app", True) is False ] diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index c1dab855b3494a..9ba37b450edbcf 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -1,12 +1,9 @@ from __future__ import annotations -from abc import ABC, abstractmethod -from typing import Any, Mapping, Sequence - from sentry.eventstore.models import Event -from sentry.utils.glob import glob_match from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.event_stripper import EventStripper +from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector class SDKCrashReporter: @@ -17,66 +14,6 @@ def report(self, event: Event) -> None: pass -class SDKCrashDetector(ABC): - @abstractmethod - def init(self): - raise NotImplementedError - - @abstractmethod - def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: - """ - Returns true if the stacktrace stems from an SDK crash. - - :param frames: The stacktrace frames ordered from newest to oldest. - """ - raise NotImplementedError - - @abstractmethod - def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: - raise NotImplementedError - - -class CocoaSDKCrashDetector(SDKCrashDetector): - def __init__(self): - self - - def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: - if not frames: - return False - - for frame in frames: - if self.is_sdk_frame(frame): - return True - - if frame.get("in_app") is True: - return False - - return False - - def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: - function = frame.get("function") - - if function is not None: - # [SentrySDK crash] is a testing function causing a crash. - # Therefore, we don't want to mark it a as a SDK crash. - if "SentrySDK crash" in function: - return False - - functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] - for matcher in functionsMatchers: - if glob_match(frame.get("function"), matcher, ignorecase=True): - return True - - filename = frame.get("filename") - if filename is not None: - filenameMatchers = ["Sentry**"] - for matcher in filenameMatchers: - if glob_match(filename, matcher, ignorecase=True): - return True - - return False - - class SDKCrashDetection: def __init__( self, diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py new file mode 100644 index 00000000000000..486e3b688e9616 --- /dev/null +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py @@ -0,0 +1,21 @@ +from abc import ABC, abstractmethod +from typing import Any, Mapping, Sequence + + +class SDKCrashDetector(ABC): + @abstractmethod + def init(self): + raise NotImplementedError + + @abstractmethod + def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: + """ + Returns true if the stacktrace stems from an SDK crash. + + :param frames: The stacktrace frames ordered from newest to oldest. + """ + raise NotImplementedError + + @abstractmethod + def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: + raise NotImplementedError 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 7d6049c3afbacc..94c3eb699eee1f 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -4,11 +4,8 @@ import pytest from sentry.utils.safe import get_path -from sentry.utils.sdk_crashes.sdk_crash_detection import ( - CocoaSDKCrashDetector, - SDKCrashDetection, - SDKCrashReporter, -) +from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector +from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, get_crash_event, From a793793e09b46f5997a872c9f042ae491482e25e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 29 Mar 2023 09:53:17 +0200 Subject: [PATCH 19/37] fix tests --- src/sentry/utils/sdk_crashes/sdk_crash_detector.py | 4 ---- .../utils/sdk_crashes/test_event_stripper.py | 6 ++++-- .../utils/sdk_crashes/test_sdk_crash_detection.py | 14 ++++++++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py index 486e3b688e9616..500c3d40b1ba88 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py @@ -3,10 +3,6 @@ class SDKCrashDetector(ABC): - @abstractmethod - def init(self): - raise NotImplementedError - @abstractmethod def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: """ diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index 7f4f3194092ae9..8990dc4406b1d6 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -1,9 +1,11 @@ +from unittest.mock import Mock + from sentry.utils.sdk_crashes.event_stripper import EventStripper from tests.sentry.utils.sdk_crashes.test_fixture import get_crash_event def test_strip_event_data_keeps_allowed_keys(): - event_stripper = EventStripper() + event_stripper = EventStripper(Mock()) stripped_event = event_stripper.strip_event_data(get_crash_event()) @@ -29,7 +31,7 @@ def test_strip_event_data_keeps_allowed_keys(): def test_strip_event_data_strips_context(): - event_stripper = EventStripper() + event_stripper = EventStripper(Mock()) stripped_event = event_stripper.strip_event_data(get_crash_event()) 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 94c3eb699eee1f..13322e0cdd6af4 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -5,6 +5,7 @@ from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector +from sentry.utils.sdk_crashes.event_stripper import EventStripper from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, @@ -191,7 +192,7 @@ def test_strip_frames(function, in_app): frames = get_frames(function, sentry_frame_in_app=in_app) event = get_crash_event_with_frames(frames) - crash_detector, crash_reporter = given_crash_detector() + crash_detector, crash_reporter = given_crash_detector(with_real_event_stripper=True) crash_detector.detect_sdk_crash(event) crash_reporter.report.assert_called_once() @@ -205,12 +206,17 @@ def test_strip_frames(function, in_app): ), "in_app frame should be removed" -def given_crash_detector() -> Tuple[SDKCrashDetection, SDKCrashReporter]: +def given_crash_detector( + with_real_event_stripper: bool = False, +) -> Tuple[SDKCrashDetection, SDKCrashReporter]: crash_reporter = Mock() - event_stripper = Mock() cocoa_sdk_crash_detector = CocoaSDKCrashDetector() - event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) + if with_real_event_stripper: + event_stripper = EventStripper(cocoa_sdk_crash_detector) + else: + event_stripper = Mock() + event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) crash_detection = SDKCrashDetection(crash_reporter, cocoa_sdk_crash_detector, event_stripper) From 46dbf5d13205fc2e4997f0dc8dda26fc02bdc4d7 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 29 Mar 2023 10:14:40 +0200 Subject: [PATCH 20/37] move strip frames tests --- .../utils/sdk_crashes/test_event_stripper.py | 36 ++++++++++++++++- .../sdk_crashes/test_sdk_crash_detection.py | 40 ++----------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index 8990dc4406b1d6..2530ea79ddc972 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -1,7 +1,16 @@ from unittest.mock import Mock +import pytest + +from sentry.utils.safe import get_path +from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector from sentry.utils.sdk_crashes.event_stripper import EventStripper -from tests.sentry.utils.sdk_crashes.test_fixture import get_crash_event +from tests.sentry.utils.sdk_crashes.test_fixture import ( + IN_APP_FRAME, + get_crash_event, + get_crash_event_with_frames, + get_frames, +) def test_strip_event_data_keeps_allowed_keys(): @@ -40,3 +49,28 @@ def test_strip_event_data_strips_context(): assert contexts.get("app") is None assert contexts.get("os") is not None assert contexts.get("device") is not None + + +@pytest.mark.parametrize( + "function,in_app", + [ + ("SentryCrashMonitor_CPPException.cpp", True), + ("SentryCrashMonitor_CPPException.cpp", False), + ], + ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept"], +) +def test_strip_frames(function, in_app): + event_stripper = EventStripper(CocoaSDKCrashDetector()) + + frames = get_frames(function, sentry_frame_in_app=in_app) + event = get_crash_event_with_frames(frames) + + stripped_event = event_stripper.strip_event_data(event) + + stripped_frames = get_path(stripped_event, "exception", "values", -1, "stacktrace", "frames") + + assert len(stripped_frames) == 6 + assert ( + len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) + == 0 + ), "in_app frame should be removed" 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 13322e0cdd6af4..189916184306a1 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -3,15 +3,12 @@ import pytest -from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector -from sentry.utils.sdk_crashes.event_stripper import EventStripper from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, get_crash_event, get_crash_event_with_frames, - get_frames, get_sentry_frame, ) @@ -180,43 +177,12 @@ def test_report_cocoa_sdk_crash_frames(frames, should_be_reported): _run_report_test_with_event(event, should_be_reported) -@pytest.mark.parametrize( - "function,in_app", - [ - ("SentryCrashMonitor_CPPException.cpp", True), - ("SentryCrashMonitor_CPPException.cpp", False), - ], - ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept"], -) -def test_strip_frames(function, in_app): - frames = get_frames(function, sentry_frame_in_app=in_app) - event = get_crash_event_with_frames(frames) - - crash_detector, crash_reporter = given_crash_detector(with_real_event_stripper=True) - crash_detector.detect_sdk_crash(event) - - crash_reporter.report.assert_called_once() - reported_event = crash_reporter.report.call_args.args[0] - stripped_frames = get_path(reported_event, "exception", "values", -1, "stacktrace", "frames") - - assert len(stripped_frames) == 6 - assert ( - len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) - == 0 - ), "in_app frame should be removed" - - -def given_crash_detector( - with_real_event_stripper: bool = False, -) -> Tuple[SDKCrashDetection, SDKCrashReporter]: +def given_crash_detector() -> Tuple[SDKCrashDetection, SDKCrashReporter]: crash_reporter = Mock() cocoa_sdk_crash_detector = CocoaSDKCrashDetector() - if with_real_event_stripper: - event_stripper = EventStripper(cocoa_sdk_crash_detector) - else: - event_stripper = Mock() - event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) + event_stripper = Mock() + event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) crash_detection = SDKCrashDetection(crash_reporter, cocoa_sdk_crash_detector, event_stripper) From 0e29e2e4d7b52651dc45093d3d9640b3a770da00 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 29 Mar 2023 11:15:33 +0200 Subject: [PATCH 21/37] Fix compile error in event manager --- src/sentry/event_manager.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index a4676be1cde2d3..8b68ba5ffc4eca 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -144,7 +144,6 @@ detect_performance_problems, ) from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter if TYPE_CHECKING: from sentry.eventstore.models import BaseEvent, Event @@ -674,7 +673,6 @@ def save( ) _track_outcome_accepted_many(jobs) - _detect_sdk_crashes(jobs) self._data = job["event"].data.data @@ -2269,12 +2267,6 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping) ) -def _detect_sdk_crashes(jobs: Sequence[Job]): - for job in jobs: - sdk_crash_detector = SDKCrashDetection(sdk_crash_reporter=SDKCrashReporter()) - sdk_crash_detector.detect_sdk_crash(job["data"]) - - class PerformanceJob(TypedDict, total=False): performance_problems: Sequence[PerformanceProblem] event: Event From b0a6ac3e505124d2968e5944a2ef72e97e753b1d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 30 Mar 2023 09:55:03 +0200 Subject: [PATCH 22/37] Only keep in stack trace referenced debug images (#46494) Remove the debug images not referenced in the stack trace so the debug images don't contain customer data. Fixes GH-46174 Co-authored-by: Joris Bayer --- .../utils/sdk_crashes/event_stripper.py | 22 +++++++- .../utils/sdk_crashes/test_event_stripper.py | 36 ++++++++++++ .../sentry/utils/sdk_crashes/test_fixture.py | 56 ++++++++++++++++--- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 010e5305da74b7..4f137d91777009 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -30,9 +30,17 @@ 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") - stripped_frames = self._strip_frames(frames) - new_event["exception"]["values"][0]["stacktrace"]["frames"] = stripped_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 @@ -49,6 +57,16 @@ def _filter_contexts(self, pair): 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. diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index 2530ea79ddc972..c58b15a5e2a2dd 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -74,3 +74,39 @@ def test_strip_frames(function, in_app): len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) == 0 ), "in_app frame should be removed" + + +def test_strip_event_data_strips_non_referenced_dsyms(): + event_stripper = EventStripper(Mock()) + + stripped_event = event_stripper.strip_event_data(get_crash_event()) + + debug_meta_images = get_path(stripped_event, "debug_meta", "images") + + image_addresses = set(map(lambda image: image["image_addr"], debug_meta_images)) + expected_image_addresses = {"0x1025e8000", "0x102b8c000", "0x102f68000", "0x19c9eb000"} + assert image_addresses == expected_image_addresses + + +def test_strip_event_data_without_frames(): + event_stripper = EventStripper(Mock()) + + crash_event = get_crash_event() + crash_event["exception"]["values"][0]["stacktrace"]["frames"] = None + + stripped_event = event_stripper.strip_event_data(crash_event) + + debug_meta_images = get_path(stripped_event, "debug_meta", "images") + assert len(debug_meta_images) == 0 + + +def test_strip_event_data_without_debug_meta(): + event_stripper = EventStripper(Mock()) + + crash_event = get_crash_event() + crash_event["debug_meta"]["images"] = None + + stripped_event = event_stripper.strip_event_data(crash_event) + + debug_meta_images = get_path(stripped_event, "debug_meta", "images") + assert debug_meta_images is None diff --git a/tests/sentry/utils/sdk_crashes/test_fixture.py b/tests/sentry/utils/sdk_crashes/test_fixture.py index e8c43c77e25a3b..edfbb21e1eba50 100644 --- a/tests/sentry/utils/sdk_crashes/test_fixture.py +++ b/tests/sentry/utils/sdk_crashes/test_fixture.py @@ -20,6 +20,7 @@ def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: "function": function, "package": "Sentry", "in_app": in_app, + "image_addr": "0x102f68000", } @@ -31,6 +32,7 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", "package": "SentryApp", "filename": "LoginViewController.swift", + "image_addr": "0x102b8c000", }, IN_APP_FRAME, { @@ -38,30 +40,35 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", "package": "UIKitCore", "in_app": False, + "image_addr": "0x19c9eb000", }, { "function": "-[UIViewController __viewDidAppear:]", "symbol": "-[UIViewController __viewDidAppear:]", "package": "UIKitCore", "in_app": False, + "image_addr": "0x19c9eb000", }, { "function": "-[UIViewController _endAppearanceTransition:]", "symbol": "-[UIViewController _endAppearanceTransition:]", "package": "UIKitCore", "in_app": False, + "image_addr": "0x19c9eb000", }, { "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "package": "UIKitCore", "in_app": False, + "image_addr": "0x19c9eb000", }, { "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "package": "UIKitCore", "in_app": False, + "image_addr": "0x19c9eb000", }, ] @@ -139,16 +146,51 @@ def get_crash_event_with_frames( "type": "os", }, }, - # Todo add referenced in stacktrace "debug_meta": { "images": [ { - "name": "CrashProbeiOS", - "image_vmaddr": "0x0000000100000000", - "image_addr": "0x0000000100088000", - "type": "apple", - "image_size": 65536, - "uuid": "2C656702-AA16-3E5F-94D9-D4430DA53398", + "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/SentryApp.app/iOS-SentryApp", + "debug_id": "97862189-a5be-3af8-bcaa-7066ae872592", + "arch": "arm64", + "image_addr": "0x1025e8000", + "image_size": 170224, + "image_vmaddr": "0x187011000", + "type": "macho", + }, + { + "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/iOS-Swift", + "debug_id": "97862189-a5be-3af8-bcaa-7066ae872591", + "arch": "arm64", + "image_addr": "0x102b8c000", + "image_size": 180224, + "image_vmaddr": "0x100000000", + "type": "macho", + }, + { + "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/Frameworks/Sentry.framework/Sentry", + "debug_id": "4726c693-2359-3e9a-bd3a-559760d87486", + "arch": "arm64", + "image_addr": "0x102f68000", + "image_size": 786432, + "type": "macho", + }, + { + "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore", + "debug_id": "b0858d8e-7220-37bf-873f-ecc2b0a358c3", + "arch": "arm64e", + "image_addr": "0x19c9eb000", + "image_size": 25309184, + "image_vmaddr": "0x18907f000", + "type": "macho", + }, + { + "code_file": "/System/Library/Frameworks/CFNetwork.framework/CFNetwork", + "debug_id": "b2273be9-538a-3f56-b9c7-801f39550f58", + "arch": "arm64e", + "image_addr": "0x19b98e000", + "image_size": 3977216, + "image_vmaddr": "0x188022000", + "in_app": False, }, ] }, From 3b8c023072df918ecd9d5b31e9578363cbfeff52 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 3 Apr 2023 13:33:02 +0200 Subject: [PATCH 23/37] Avoid infinite processing loop (#46578) Detect when an event has the context `sdk_crash_detection`, and skip crash monitoring in that case. Fixes GH-46173 --- .../utils/sdk_crashes/sdk_crash_detection.py | 8 +++++- .../sdk_crashes/test_sdk_crash_detection.py | 27 ++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 9ba37b450edbcf..f9ba0d84e68fd7 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -1,7 +1,7 @@ from __future__ import annotations from sentry.eventstore.models import Event -from sentry.utils.safe import get_path +from sentry.utils.safe import get_path, set_path from sentry.utils.sdk_crashes.event_stripper import EventStripper from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector @@ -30,6 +30,10 @@ def detect_sdk_crash(self, event: Event) -> None: if event.get("type", None) != "error" or event.get("platform") != "cocoa": return + context = get_path(event, "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 if is_unhandled is False: return @@ -40,4 +44,6 @@ def detect_sdk_crash(self, event: Event) -> None: if self.cocoa_sdk_crash_detector.is_sdk_crash(frames): sdk_crash_event = self.event_stripper.strip_event_data(event) + + set_path(sdk_crash_event, "contexts", "sdk_crash_detection", value={"detected": True}) self.sdk_crash_reporter.report(sdk_crash_event) 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 189916184306a1..65949b71932383 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,4 +1,4 @@ -from typing import Tuple +from typing import Any, Mapping, Sequence, Tuple from unittest.mock import MagicMock, Mock import pytest @@ -177,8 +177,22 @@ def test_report_cocoa_sdk_crash_frames(frames, should_be_reported): _run_report_test_with_event(event, should_be_reported) +def test_sdk_crash_detected_event_is_not_reported(): + event = get_crash_event() + event["contexts"]["sdk_crash_detection"] = {"detected": True} + + _run_report_test_with_event(event, should_be_reported=False) + + +def test_cocoa_sdk_crash_detection_without_context(): + event = get_crash_event(function="-[SentryHub getScope]") + event["contexts"] = {} + + _run_report_test_with_event(event, True) + + def given_crash_detector() -> Tuple[SDKCrashDetection, SDKCrashReporter]: - crash_reporter = Mock() + crash_reporter = Mock(spec=SDKCrashReporter) cocoa_sdk_crash_detector = CocoaSDKCrashDetector() event_stripper = Mock() @@ -200,8 +214,13 @@ def _run_report_test_with_event(event, should_be_reported): assert_no_sdk_crash_reported(crash_reporter) -def assert_sdk_crash_reported(crash_reporter: SDKCrashReporter, expected_data: dict): - crash_reporter.report.assert_called_once_with(expected_data) +def assert_sdk_crash_reported( + crash_reporter: SDKCrashReporter, expected_event: Sequence[Mapping[str, Any]] +): + crash_reporter.report.assert_called_once_with(expected_event) + + reported_event = crash_reporter.report.call_args.args[0] + assert reported_event["contexts"]["sdk_crash_detection"]["detected"] is True def assert_no_sdk_crash_reported(crash_reporter: SDKCrashReporter): From e4d33000cec3faadad81ba256025c69f19cb92dd Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 4 Apr 2023 12:19:41 +0200 Subject: [PATCH 24/37] add __init__.py file --- src/sentry/utils/sdk_crashes/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/sentry/utils/sdk_crashes/__init__.py diff --git a/src/sentry/utils/sdk_crashes/__init__.py b/src/sentry/utils/sdk_crashes/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 From ed415e4ead2f197d041e91cacc68f8bc6b80436d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 30 May 2023 15:08:52 +0200 Subject: [PATCH 25/37] feat(sdk-crash-detection): Call code in post processing (#49170) 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 --- src/sentry/tasks/post_process.py | 14 + .../utils/sdk_crashes/event_stripper.py | 139 +++--- .../utils/sdk_crashes/sdk_crash_detection.py | 36 +- tests/sentry/tasks/test_post_process.py | 20 + .../utils/sdk_crashes/test_event_stripper.py | 188 +++++--- .../sentry/utils/sdk_crashes/test_fixture.py | 52 +- .../sdk_crashes/test_sdk_crash_detection.py | 449 ++++++++++-------- 7 files changed, 523 insertions(+), 375 deletions(-) diff --git a/src/sentry/tasks/post_process.py b/src/sentry/tasks/post_process.py index 8ced1b821a95fa..48f0900666a107 100644 --- a/src/sentry/tasks/post_process.py +++ b/src/sentry/tasks/post_process.py @@ -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. @@ -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, diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 4f137d91777009..0a8f0bba0cef72 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -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 + ] diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index f9ba0d84e68fd7..ad7da9de592258 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -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 @@ -11,7 +13,7 @@ def __init__(self): self def report(self, event: Event) -> None: - pass + return class SDKCrashDetection: @@ -19,31 +21,45 @@ 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) diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index 879be700ebffa9..9e07719ec6ddb6 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -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: @@ -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, @@ -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) diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index c58b15a5e2a2dd..4eadc5077ae0c6 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -1,10 +1,12 @@ +import abc from unittest.mock import Mock -import pytest - +from sentry.testutils import TestCase +from sentry.testutils.cases import BaseTestCase +from sentry.testutils.silo import region_silo_test from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector -from sentry.utils.sdk_crashes.event_stripper import EventStripper +from sentry.utils.sdk_crashes.event_stripper import strip_event_data from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, get_crash_event, @@ -13,100 +15,136 @@ ) -def test_strip_event_data_keeps_allowed_keys(): - event_stripper = EventStripper(Mock()) - - stripped_event = event_stripper.strip_event_data(get_crash_event()) - - keys_removed = {"tags", "user", "threads", "breadcrumbs", "environment"} - for key in keys_removed: - assert stripped_event.get(key) is None +class BaseEventStripperMixin(BaseTestCase, metaclass=abc.ABCMeta): + @abc.abstractmethod + def create_event(self, data, project_id, assert_no_errors=True): + pass - keys_kept = { - "type", - "datetime", - "timestamp", - "platform", - "sdk", - "level", - "logger", - "exception", - "debug_meta", - "contexts", - } + def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): + pass - for key in keys_kept: - assert stripped_event.get(key) is not None +class EventStripperTestMixin(BaseEventStripperMixin): + def test_strip_event_data_keeps_allowed_keys(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) -def test_strip_event_data_strips_context(): - event_stripper = EventStripper(Mock()) + stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) - stripped_event = event_stripper.strip_event_data(get_crash_event()) + 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" - contexts = stripped_event.get("contexts") - assert contexts is not None - assert contexts.get("app") is None - assert contexts.get("os") is not None - assert contexts.get("device") is not None - - -@pytest.mark.parametrize( - "function,in_app", - [ - ("SentryCrashMonitor_CPPException.cpp", True), - ("SentryCrashMonitor_CPPException.cpp", False), - ], - ids=["sentry_in_app_frame_kept", "sentry_not_in_app_frame_kept"], -) -def test_strip_frames(function, in_app): - event_stripper = EventStripper(CocoaSDKCrashDetector()) + keys_kept = { + "type", + "timestamp", + "platform", + "sdk", + "level", + "logger", + "exception", + "debug_meta", + "contexts", + } - frames = get_frames(function, sentry_frame_in_app=in_app) - event = get_crash_event_with_frames(frames) + for key in keys_kept: + assert stripped_event.data.get(key) is not None, f"key {key} should be kept" - stripped_event = event_stripper.strip_event_data(event) + def test_strip_event_data_removes_ip_address(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) - stripped_frames = get_path(stripped_event, "exception", "values", -1, "stacktrace", "frames") + assert event.ip_address is not None - assert len(stripped_frames) == 6 - assert ( - len([frame for frame in stripped_frames if frame["function"] == IN_APP_FRAME["function"]]) - == 0 - ), "in_app frame should be removed" + stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + assert stripped_event.ip_address is None -def test_strip_event_data_strips_non_referenced_dsyms(): - event_stripper = EventStripper(Mock()) + def test_strip_event_data_without_debug_meta(self): + event_data = get_crash_event() + event_data["debug_meta"]["images"] = None - stripped_event = event_stripper.strip_event_data(get_crash_event()) + event = self.create_event( + data=event_data, + project_id=self.project.id, + ) - debug_meta_images = get_path(stripped_event, "debug_meta", "images") + stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) - image_addresses = set(map(lambda image: image["image_addr"], debug_meta_images)) - expected_image_addresses = {"0x1025e8000", "0x102b8c000", "0x102f68000", "0x19c9eb000"} - assert image_addresses == expected_image_addresses + 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): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) -def test_strip_event_data_without_frames(): - event_stripper = EventStripper(Mock()) + stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) - crash_event = get_crash_event() - crash_event["exception"]["values"][0]["stacktrace"]["frames"] = None + contexts = stripped_event.data.get("contexts") + assert contexts is not None + assert contexts.get("app") is None + assert contexts.get("os") is not None + assert contexts.get("device") is not None - stripped_event = event_stripper.strip_event_data(crash_event) + def test_strip_event_data_strips_non_referenced_dsyms(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) - debug_meta_images = get_path(stripped_event, "debug_meta", "images") - assert len(debug_meta_images) == 0 + stripped_event = strip_event_data(event, Mock()) + debug_meta_images = get_path(stripped_event.data, "debug_meta", "images") -def test_strip_event_data_without_debug_meta(): - event_stripper = EventStripper(Mock()) + image_addresses = set(map(lambda image: image["image_addr"], debug_meta_images)) + expected_image_addresses = {"0x1a4e8f000", "0x100304000", "0x100260000"} + assert image_addresses == expected_image_addresses - crash_event = get_crash_event() - crash_event["debug_meta"]["images"] = None + 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) - stripped_event = event_stripper.strip_event_data(crash_event) + def test_strip_frames_sentry_non_in_app_frame_kept(self): + frames = get_frames("SentryCrashMonitor_CPPException.cpp", sentry_frame_in_app=False) + self._execute_strip_frames_test(frames) - debug_meta_images = get_path(stripped_event, "debug_meta", "images") - assert debug_meta_images is None + def _execute_strip_frames_test(self, frames): + event_data = get_crash_event_with_frames(frames) + + event = self.create_event( + data=event_data, + project_id=self.project.id, + ) + + stripped_event = strip_event_data(event, CocoaSDKCrashDetector()) + + stripped_frames = get_path( + stripped_event.data, "exception", "values", -1, "stacktrace", "frames" + ) + + assert len(stripped_frames) == 6 + assert ( + len( + [ + frame + for frame in stripped_frames + if frame["function"] == IN_APP_FRAME["function"] + ] + ) + == 0 + ), "in_app frame should be removed" + + +@region_silo_test +class EventStripperTest( + TestCase, + EventStripperTestMixin, +): + 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) diff --git a/tests/sentry/utils/sdk_crashes/test_fixture.py b/tests/sentry/utils/sdk_crashes/test_fixture.py index edfbb21e1eba50..f2a4dab467335d 100644 --- a/tests/sentry/utils/sdk_crashes/test_fixture.py +++ b/tests/sentry/utils/sdk_crashes/test_fixture.py @@ -9,9 +9,9 @@ "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", "lineno": 196, "in_app": True, - "image_addr": "0x1025e8000", + "image_addr": "0x100260000", "instruction_addr": "0x102b16630", - "symbol_addr": "0x1025e8000", + "symbol_addr": "0x100260000", } @@ -20,7 +20,7 @@ def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: "function": function, "package": "Sentry", "in_app": in_app, - "image_addr": "0x102f68000", + "image_addr": "0x100304000", } @@ -31,8 +31,9 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map "function": "LoginViewController.viewDidAppear", "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", "package": "SentryApp", + "in_app": True, "filename": "LoginViewController.swift", - "image_addr": "0x102b8c000", + "image_addr": "0x100260000", }, IN_APP_FRAME, { @@ -40,35 +41,35 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", }, { "function": "-[UIViewController __viewDidAppear:]", "symbol": "-[UIViewController __viewDidAppear:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", }, { "function": "-[UIViewController _endAppearanceTransition:]", "symbol": "-[UIViewController _endAppearanceTransition:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", }, { "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", "package": "UIKitCore", "in_app": False, - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", }, { "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", "package": "UIKitCore", "in_app": False, - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", }, ] @@ -82,14 +83,11 @@ def get_crash_event_with_frames( ) -> Sequence[Mapping[str, Any]]: result = { "event_id": "80e3496eff734ab0ac993167aaa0d1cd", - "project": 5218188, "release": "5.222.5", "type": "error", "level": "fatal", "platform": "cocoa", "tags": {"level": "fatal"}, - "datetime": "2023-02-08T23:51:35.000000Z", - "timestamp": 1675936223.0, "exception": { "values": [ { @@ -149,48 +147,40 @@ def get_crash_event_with_frames( "debug_meta": { "images": [ { - "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/SentryApp.app/iOS-SentryApp", - "debug_id": "97862189-a5be-3af8-bcaa-7066ae872592", + "code_file": "/private/var/containers/Bundle/Application/895DA2DE-5FE3-44A0-8C0F-900519EA5516/iOS-Swift.app/iOS-Swift", + "debug_id": "aa8a3697-c88a-36f9-a687-3d3596568c8d", "arch": "arm64", - "image_addr": "0x1025e8000", - "image_size": 170224, - "image_vmaddr": "0x187011000", - "type": "macho", - }, - { - "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/iOS-Swift", - "debug_id": "97862189-a5be-3af8-bcaa-7066ae872591", - "arch": "arm64", - "image_addr": "0x102b8c000", + "image_addr": "0x100260000", "image_size": 180224, "image_vmaddr": "0x100000000", "type": "macho", }, { "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/Frameworks/Sentry.framework/Sentry", - "debug_id": "4726c693-2359-3e9a-bd3a-559760d87486", + "debug_id": "e2623c4d-79c5-3cdf-90ab-2cf44e026bdd", "arch": "arm64", - "image_addr": "0x102f68000", - "image_size": 786432, + "image_addr": "0x100304000", + "image_size": 802816, "type": "macho", }, { "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore", "debug_id": "b0858d8e-7220-37bf-873f-ecc2b0a358c3", "arch": "arm64e", - "image_addr": "0x19c9eb000", + "image_addr": "0x1a4e8f000", "image_size": 25309184, - "image_vmaddr": "0x18907f000", + "image_vmaddr": "0x188ff7000", "type": "macho", }, { "code_file": "/System/Library/Frameworks/CFNetwork.framework/CFNetwork", "debug_id": "b2273be9-538a-3f56-b9c7-801f39550f58", "arch": "arm64e", - "image_addr": "0x19b98e000", + "image_addr": "0x1a3e32000", "image_size": 3977216, - "image_vmaddr": "0x188022000", + "image_vmaddr": "0x187f9a000", "in_app": False, + "type": "macho", }, ] }, 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 65949b71932383..d922c968ce2c22 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,10 +1,13 @@ -from typing import Any, Mapping, Sequence, Tuple -from unittest.mock import MagicMock, Mock - -import pytest - -from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector -from sentry.utils.sdk_crashes.sdk_crash_detection import SDKCrashDetection, SDKCrashReporter +import abc +from typing import Any, Mapping, Sequence +from unittest.mock import patch + +from sentry.issues.grouptype import PerformanceNPlusOneGroupType +from sentry.testutils import TestCase +from sentry.testutils.cases import BaseTestCase +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 from tests.sentry.utils.sdk_crashes.test_fixture import ( IN_APP_FRAME, get_crash_event, @@ -13,107 +16,164 @@ ) -@pytest.mark.parametrize( - "event,should_be_reported", - [ - ( - get_crash_event(), +class BaseSDKCrashDetectionMixin(BaseTestCase, metaclass=abc.ABCMeta): + @abc.abstractmethod + 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): + event = self.create_event( + data=event_data, + project_id=self.project.id, + ) + + sdk_crash_detection.detect_sdk_crash(event=event) + + 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 + else: + mock_sdk_crash_reporter.report.assert_not_called() + + +class PerformanceEventTestMixin(BaseSDKCrashDetectionMixin, PerfIssueTransactionTestMixin): + @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") + def test_performance_event_not_detected(self, mock_sdk_crash_reporter): + + fingerprint = "some_group" + fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}" + event = self.store_transaction( + project_id=self.project.id, + user_id="hi", + fingerprint=[fingerprint], + ) + + sdk_crash_detection.detect_sdk_crash(event=event) + + mock_sdk_crash_reporter.report.assert_not_called() + + +@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") +class CococaSDKTestMixin(BaseSDKCrashDetectionMixin): + def test_unhandled_is_detected(self, mock_sdk_crash_reporter): + self.execute_test(get_crash_event(), True, mock_sdk_crash_reporter) + + def test_handled_is_not_detected(self, mock_sdk_crash_reporter): + self.execute_test(get_crash_event(handled=True), False, mock_sdk_crash_reporter) + + def test_wrong_function_not_detected(self, mock_sdk_crash_reporter): + self.execute_test(get_crash_event(function="Senry"), False, mock_sdk_crash_reporter) + + def test_wrong_platform_not_detected(self, mock_sdk_crash_reporter): + self.execute_test(get_crash_event(platform="coco"), False, mock_sdk_crash_reporter) + + def test_no_exception_not_detected(self, mock_sdk_crash_reporter): + self.execute_test(get_crash_event(exception=[]), False, mock_sdk_crash_reporter) + + def test_sdk_crash_detected_event_is_not_reported(self, mock_sdk_crash_reporter): + event = get_crash_event() + event["contexts"]["sdk_crash_detection"] = {"detected": True} + + self.execute_test(event, False, mock_sdk_crash_reporter) + + def test_cocoa_sdk_crash_detection_without_context(self, mock_sdk_crash_reporter): + event = get_crash_event(function="-[SentryHub getScope]") + event["contexts"] = {} + + self.execute_test(event, True, mock_sdk_crash_reporter) + + +@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") +class CococaSDKFunctionTestMixin(BaseSDKCrashDetectionMixin): + def test_hub_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-[SentryHub getScope]"), True, mock_sdk_crash_reporter + ) + + def test_sentrycrash_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="sentrycrashdl_getBinaryImage"), True, mock_sdk_crash_reporter + ) + + def test_sentryisgreat_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-[sentryisgreat]"), True, mock_sdk_crash_reporter + ) + + def test_sentryswizzle_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event( + function="__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2" + ), True, - ), - ( - get_crash_event(handled=True), + mock_sdk_crash_reporter, + ) + + def test_sentrycrash_crash_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-[SentryCrash crash]"), + True, + mock_sdk_crash_reporter, + ) + + def test_senryhub_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-[SenryHub getScope]"), False, - ), - (get_crash_event(function="Senry"), False), - (get_crash_event(platform="coco"), False), - (get_crash_event(type="erro"), False), - (get_crash_event(exception=[]), False), - ], - ids=[ - "unhandled_is_detected", - "handled_not_detected", - "wrong_function_not_detected", - "wrong_platform_not_detected", - "wrong_type_not_detected", - "no_exception_not_detected", - ], -) -def test_detect_sdk_crash(event, should_be_reported): - _run_report_test_with_event(event, should_be_reported) - - -@pytest.mark.parametrize( - "function,should_be_reported", - [ - ("-[SentryHub getScope]", True), - ("sentrycrashdl_getBinaryImage", True), - ("-[sentryisgreat]", True), - ("__47-[SentryBreadcrumbTracker swizzleViewDidAppear]_block_invoke_2", True), - ("-[SentryCrash crash]", True), - ("-[SenryHub getScope]", False), - ("-SentryHub getScope]", False), - ("-[SomeSentryHub getScope]", False), - ("+[SentrySDK crash]", False), - ], -) -def test_cocoa_sdk_crash_detection(function, should_be_reported): - event = get_crash_event(function=function) + mock_sdk_crash_reporter, + ) - _run_report_test_with_event(event, should_be_reported) + def test_senryhub_no_brackets_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-SentryHub getScope]"), + False, + mock_sdk_crash_reporter, + ) + def test_somesentryhub_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="-[SomeSentryHub getScope]"), + False, + mock_sdk_crash_reporter, + ) -@pytest.mark.parametrize( - "filename,should_be_reported", - [ - ("SentryCrashMonitor_CPPException.cpp", True), - ("SentryMonitor_CPPException.cpp", True), - ("SentrMonitor_CPPException.cpp", False), - ], -) -def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): - event = get_crash_event_with_frames( - frames=[ - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", - "in_app": False, - }, - { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - { - "function": "CPPExceptionTerminate", - "raw_function": "CPPExceptionTerminate()", - "filename": filename, - "symbol": "_ZL21CPPExceptionTerminatev", - "package": "MainApp", - "in_app": False, - }, - { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", - "in_app": False, - }, - ] - ) - - _run_report_test_with_event(event, should_be_reported) - - -@pytest.mark.parametrize( - "frames,should_be_reported", - [ - ([], False), - ([{"empty": "frame"}], False), - ([get_sentry_frame("-[Sentry]")], True), - ([get_sentry_frame("-[Sentry]", in_app=True)], True), - ( - [ + # "+[SentrySDK crash]" is used for testing, so we must ignore it. + def test_sentrycrash_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event(function="+[SentrySDK crash]"), + False, + mock_sdk_crash_reporter, + ) + + +@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") +class CococaSDKFilenameTestMixin(BaseSDKCrashDetectionMixin): + def test_filename_includes_sentrycrash_reported(self, mock_sdk_crash_reporter): + self.execute_test( + self._get_crash_event("SentryCrashMonitor_CPPException.cpp"), + True, + mock_sdk_crash_reporter, + ) + + def test_filename_includes_sentrymonitor_reported(self, mock_sdk_crash_reporter): + self.execute_test( + self._get_crash_event("SentryMonitor_CPPException.cpp"), + True, + mock_sdk_crash_reporter, + ) + + def test_filename_includes_senry_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + self._get_crash_event("SentrMonitor_CPPException.cpp"), + False, + mock_sdk_crash_reporter, + ) + + def _get_crash_event(self, filename) -> Sequence[Mapping[str, Any]]: + return get_crash_event_with_frames( + frames=[ { "function": "__handleUncaughtException", "symbol": "__handleUncaughtException", @@ -126,102 +186,115 @@ def test_report_cocoa_sdk_crash_filename(filename, should_be_reported): "package": "libobjc.A.dylib", "in_app": False, }, - get_sentry_frame("sentrycrashdl_getBinaryImage"), { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", - "in_app": False, - }, - ], - True, - ), - ( - [ - IN_APP_FRAME, - { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", + "function": "CPPExceptionTerminate", + "raw_function": "CPPExceptionTerminate()", + "filename": filename, + "symbol": "_ZL21CPPExceptionTerminatev", + "package": "MainApp", "in_app": False, }, - { - "function": "_objc_terminate", - "symbol": "_ZL15_objc_terminatev", - "package": "libobjc.A.dylib", - "in_app": False, - }, - get_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "std::__terminate", "symbol": "_ZSt11__terminatePFvvE", "package": "libc++abi.dylib", "in_app": False, }, - ], - False, - ), - ], - ids=[ - "no_frames_not_detected", - "empty_frame_not_detected", - "single_frame_is_detected", - "single_in_app_frame_is_detected", - "only_non_inapp_after_sentry_frame_is_detected", - "only_inapp_after_sentry_frame_not_detected", - ], -) -def test_report_cocoa_sdk_crash_frames(frames, should_be_reported): - event = get_crash_event_with_frames(frames) - - _run_report_test_with_event(event, should_be_reported) - + ] + ) -def test_sdk_crash_detected_event_is_not_reported(): - event = get_crash_event() - event["contexts"]["sdk_crash_detection"] = {"detected": True} - _run_report_test_with_event(event, should_be_reported=False) - - -def test_cocoa_sdk_crash_detection_without_context(): - event = get_crash_event(function="-[SentryHub getScope]") - event["contexts"] = {} - - _run_report_test_with_event(event, True) - - -def given_crash_detector() -> Tuple[SDKCrashDetection, SDKCrashReporter]: - crash_reporter = Mock(spec=SDKCrashReporter) - cocoa_sdk_crash_detector = CocoaSDKCrashDetector() - - event_stripper = Mock() - event_stripper.strip_event_data = MagicMock(side_effect=lambda x: x) - - crash_detection = SDKCrashDetection(crash_reporter, cocoa_sdk_crash_detector, event_stripper) - - return crash_detection, crash_reporter - - -def _run_report_test_with_event(event, should_be_reported): - crash_detector, crash_reporter = given_crash_detector() - - crash_detector.detect_sdk_crash(event) - - if should_be_reported: - assert_sdk_crash_reported(crash_reporter, event) - else: - assert_no_sdk_crash_reported(crash_reporter) +@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") +class CococaSDKFramesTestMixin(BaseSDKCrashDetectionMixin): + def test_frames_empty_frame_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event_with_frames([{"empty": "frame"}]), + False, + mock_sdk_crash_reporter, + ) + def test_frames_single_frame_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event_with_frames([get_sentry_frame("-[Sentry]")]), + True, + mock_sdk_crash_reporter, + ) -def assert_sdk_crash_reported( - crash_reporter: SDKCrashReporter, expected_event: Sequence[Mapping[str, Any]] + def test_frames_in_app_frame_frame_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event_with_frames([get_sentry_frame("-[Sentry]", in_app=True)]), + True, + mock_sdk_crash_reporter, + ) + + def test_frames_only_non_in_app_after_sentry_frame_is_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event_with_frames( + [ + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + get_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + ] + ), + True, + mock_sdk_crash_reporter, + ) + + def test_frames_only_in_app_after_sentry_frame_not_reported(self, mock_sdk_crash_reporter): + self.execute_test( + get_crash_event_with_frames( + [ + IN_APP_FRAME, + { + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", + "in_app": False, + }, + { + "function": "_objc_terminate", + "symbol": "_ZL15_objc_terminatev", + "package": "libobjc.A.dylib", + "in_app": False, + }, + get_sentry_frame("sentrycrashdl_getBinaryImage"), + { + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", + "in_app": False, + }, + ] + ), + False, + mock_sdk_crash_reporter, + ) + + +@region_silo_test +class SDKCrashDetectionTest( + TestCase, + CococaSDKTestMixin, + CococaSDKFunctionTestMixin, + CococaSDKFilenameTestMixin, + CococaSDKFramesTestMixin, + PerformanceEventTestMixin, ): - crash_reporter.report.assert_called_once_with(expected_event) - - reported_event = crash_reporter.report.call_args.args[0] - assert reported_event["contexts"]["sdk_crash_detection"]["detected"] is True - - -def assert_no_sdk_crash_reported(crash_reporter: SDKCrashReporter): - crash_reporter.report.assert_not_called() + 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) From 90b1967ba1cd10f0bca35595eef762dbd63e893a Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 5 Jun 2023 08:58:15 +0200 Subject: [PATCH 26/37] feat(sdk-crash-detection): Add feature flag (#49528) Add a feature flag for SDK Crash Detection https://github.com/getsentry/sentry/issues/44342. Co-authored-by: Joris Bayer --- src/sentry/conf/server.py | 2 ++ src/sentry/features/__init__.py | 1 + src/sentry/tasks/post_process.py | 3 +++ .../utils/sdk_crashes/event_stripper.py | 10 +-------- tests/sentry/tasks/test_post_process.py | 21 ++++++++++++++++++- .../sdk_crashes/test_sdk_crash_detection.py | 4 ++-- 6 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index d0387249c9e23b..45ed04f81fcc49 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -1564,6 +1564,8 @@ def SOCIAL_AUTH_DEFAULT_USERNAME(): "organizations:slack-escape-messages": False, # If true, allow to create/use org auth tokens "organizations:org-auth-tokens": False, + # Enable detecting SDK crashes during event processing + "organizations:sdk-crash-detection": False, # Adds additional filters and a new section to issue alert rules. "projects:alert-filters": True, # Enable functionality to specify custom inbound filters on events. diff --git a/src/sentry/features/__init__.py b/src/sentry/features/__init__.py index 19008d3bd8f843..fc8ac7d763b513 100644 --- a/src/sentry/features/__init__.py +++ b/src/sentry/features/__init__.py @@ -178,6 +178,7 @@ default_manager.add("organizations:set-grouping-config", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) default_manager.add("organizations:slack-escape-messages", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) default_manager.add("organizations:slack-overage-notifications", OrganizationFeature, FeatureHandlerStrategy.REMOTE) +default_manager.add("organizations:sdk-crash-detection", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) default_manager.add("organizations:starfish-view", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:starfish-test-endpoint", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:streamline-targeting-context", OrganizationFeature, FeatureHandlerStrategy.REMOTE) diff --git a/src/sentry/tasks/post_process.py b/src/sentry/tasks/post_process.py index 704c843ad32ff7..a85f49e1d92f2a 100644 --- a/src/sentry/tasks/post_process.py +++ b/src/sentry/tasks/post_process.py @@ -1050,6 +1050,9 @@ def sdk_crash_monitoring(job: PostProcessJob): event = job["event"] + if not features.has("organizations:sdk-crash-detection", event.project.organization): + return + 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 0a8f0bba0cef72..997415548f5409 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -19,7 +19,7 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Event: - new_event_data = dict(filter(_filter_event, event.data.items())) + new_event_data = {key: value for key, value in event.data.items() if key in ALLOWED_EVENT_KEYS} new_event_data["contexts"] = dict(filter(_filter_contexts, new_event_data["contexts"].items())) stripped_frames = [] @@ -38,14 +38,6 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Even 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"}: diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index 4eab3ff120fe16..d2ce954ba087cc 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -1483,8 +1483,9 @@ 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): + @with_feature("organizations:sdk-crash-detection") + @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") def test_sdk_crash_monitoring_is_called_with_event(self, mock_sdk_crash_reporter): event = self.create_event( data=get_crash_event(), @@ -1500,6 +1501,24 @@ def test_sdk_crash_monitoring_is_called_with_event(self, mock_sdk_crash_reporter mock_sdk_crash_reporter.report.assert_called_once() + @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection") + def test_sdk_crash_monitoring_is_not_called_with_disabled_feature( + 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_sdk_crash_detection.py b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py index d922c968ce2c22..3da71d5bbbe26e 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -22,6 +22,7 @@ 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): + event = self.create_event( data=event_data, project_id=self.project.id, @@ -38,10 +39,9 @@ def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): mock_sdk_crash_reporter.report.assert_not_called() +@patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") class PerformanceEventTestMixin(BaseSDKCrashDetectionMixin, PerfIssueTransactionTestMixin): - @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") def test_performance_event_not_detected(self, mock_sdk_crash_reporter): - fingerprint = "some_group" fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}" event = self.store_transaction( From 082ddda4f13749803013b58da4e48f36755f5cf9 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 5 Jun 2023 09:35:13 +0200 Subject: [PATCH 27/37] feat(sdk-crash-detection): Store event to dedicated project (#49593) Stores the detected SDK crash event to a configurable Sentry project. This PR is based on https://github.com/getsentry/sentry/pull/49528. Fixes GH-46177 Co-authored-by: Joris Bayer --- src/sentry/conf/server.py | 3 ++ src/sentry/tasks/post_process.py | 6 +++ .../utils/sdk_crashes/event_stripper.py | 3 +- .../utils/sdk_crashes/sdk_crash_detection.py | 30 +++++++++----- tests/sentry/tasks/test_post_process.py | 18 ++++++++ .../utils/sdk_crashes/test_event_stripper.py | 36 ++++++---------- .../sdk_crashes/test_sdk_crash_detection.py | 41 +++++++++++++++++-- 7 files changed, 98 insertions(+), 39 deletions(-) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 45ed04f81fcc49..bfc9e34614d3ae 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 a85f49e1d92f2a..34108a8d2c49f5 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 997415548f5409..1233e2f11c20ad 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 ad7da9de592258..a7da84972e067e 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 d2ce954ba087cc..d50786494d3917 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 4eadc5077ae0c6..614066625ab9b5 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 3da71d5bbbe26e..621adf7c213cd8 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) From cf3d1fc19924377b19a0d5af3c8e1528c25340b7 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 5 Jun 2023 09:53:47 +0200 Subject: [PATCH 28/37] fix(sdk-crash-detection): Reverse frame order (#49746) The stack trace frames are ordered from caller to callee, or oldest to youngest. Therefore, we must reverse the frame order when iterating over the frames to detect an SDK crash. This PR is based on https://github.com/getsentry/sentry/pull/49593. Co-authored-by: Joris Bayer Co-authored-by: Arpad Borsos --- .../sdk_crashes/cocoa_sdk_crash_detector.py | 5 ++++- tests/sentry/utils/sdk_crashes/test_fixture.py | 7 ++++++- .../sdk_crashes/test_sdk_crash_detection.py | 16 ++++++++-------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py index 86370ce1ab74f8..e6a267dcaeef92 100644 --- a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py @@ -12,7 +12,10 @@ def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: if not frames: return False - for frame in frames: + # The frames are ordered from caller to callee, or oldest to youngest. + # The last frame is the one creating the exception. + # Therefore, we must iterate in reverse order. + for frame in reversed(frames): if self.is_sdk_frame(frame): return True diff --git a/tests/sentry/utils/sdk_crashes/test_fixture.py b/tests/sentry/utils/sdk_crashes/test_fixture.py index f2a4dab467335d..03e458adf30a89 100644 --- a/tests/sentry/utils/sdk_crashes/test_fixture.py +++ b/tests/sentry/utils/sdk_crashes/test_fixture.py @@ -25,7 +25,7 @@ def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: - return [ + frames = [ get_sentry_frame(function, sentry_frame_in_app), { "function": "LoginViewController.viewDidAppear", @@ -73,6 +73,11 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map }, ] + # The frames have to be ordered from caller to callee, or oldest to youngest. + # The last frame is the one creating the exception. + # As we usually look at stacktraces from youngest to oldest, we reverse the order. + return frames[::-1] + def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) 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 621adf7c213cd8..417aa2250fb859 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -264,26 +264,26 @@ def test_frames_only_in_app_after_sentry_frame_not_reported(self, mock_sdk_crash self.execute_test( get_crash_event_with_frames( [ - IN_APP_FRAME, { - "function": "__handleUncaughtException", - "symbol": "__handleUncaughtException", - "package": "CoreFoundation", + "function": "std::__terminate", + "symbol": "_ZSt11__terminatePFvvE", + "package": "libc++abi.dylib", "in_app": False, }, + get_sentry_frame("sentrycrashdl_getBinaryImage"), { "function": "_objc_terminate", "symbol": "_ZL15_objc_terminatev", "package": "libobjc.A.dylib", "in_app": False, }, - get_sentry_frame("sentrycrashdl_getBinaryImage"), { - "function": "std::__terminate", - "symbol": "_ZSt11__terminatePFvvE", - "package": "libc++abi.dylib", + "function": "__handleUncaughtException", + "symbol": "__handleUncaughtException", + "package": "CoreFoundation", "in_app": False, }, + IN_APP_FRAME, ] ), False, From 9870f03013b89ea8b61934a901192fc03c76d665 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 5 Jun 2023 10:40:23 +0200 Subject: [PATCH 29/37] chore(sdk-crash-detection): Add to mypy.ini (#50284) Add src/sentry/utils/sdk_crashes to mypy.ini. Fixes GH-49938 --- mypy.ini | 1 + src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py | 2 +- src/sentry/utils/sdk_crashes/event_stripper.py | 6 +++--- src/sentry/utils/sdk_crashes/sdk_crash_detection.py | 6 ++++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/mypy.ini b/mypy.ini index b92c914f856aaa..90dac8c085cb93 100644 --- a/mypy.ini +++ b/mypy.ini @@ -187,6 +187,7 @@ files = fixtures/stubs-for-mypy, src/sentry/utils/redis_metrics.py, src/sentry/utils/request_cache.py, src/sentry/utils/services.py, + src/sentry/utils/sdk_crashes/, src/sentry/utils/suspect_resolutions/commit_correlation.py, src/sentry/utils/suspect_resolutions/metric_correlation.py, src/sentry/utils/suspect_resolutions/resolved_in_active_release.py, diff --git a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py index e6a267dcaeef92..826f1f4a64727d 100644 --- a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py @@ -5,7 +5,7 @@ class CocoaSDKCrashDetector(SDKCrashDetector): - def __init__(self): + def __init__(self) -> None: self def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 1233e2f11c20ad..279c0a50aae25d 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -1,4 +1,4 @@ -from typing import Any, Mapping, Sequence +from typing import Any, Mapping, Sequence, Tuple from sentry.eventstore.models import Event from sentry.utils.safe import get_path @@ -22,7 +22,7 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Even new_event_data = {key: value for key, value in event.data.items() if key in ALLOWED_EVENT_KEYS} new_event_data["contexts"] = dict(filter(_filter_contexts, new_event_data["contexts"].items())) - stripped_frames = [] + stripped_frames: Sequence[Mapping[str, Any]] = [] frames = get_path(new_event_data, "exception", "values", -1, "stacktrace", "frames") if frames is not None: @@ -37,7 +37,7 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Even return new_event_data -def _filter_contexts(pair): +def _filter_contexts(pair: Tuple[str, Any]) -> bool: key, _ = pair if key in {"os", "device"}: return True diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index a7da84972e067e..b569325a966c1e 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -13,7 +13,7 @@ class SDKCrashReporter: - def __init__(self): + def __init__(self) -> None: self def report(self, event_data: Dict[str, Any], event_project_id: int) -> Event: @@ -41,7 +41,7 @@ def detect_sdk_crash(self, event: Event) -> Optional[Event]: and event.group.platform == "cocoa" ) if not should_detect_sdk_crash: - return + return None context = get_path(event.data, "contexts", "sdk_crash_detection") if context is not None and context.get("detected", False): @@ -68,6 +68,8 @@ def detect_sdk_crash(self, event: Event) -> Optional[Event]: sdk_crash_event_data, settings.SDK_CRASH_DETECTION_PROJECT_ID ) + return None + _crash_reporter = SDKCrashReporter() _cocoa_sdk_crash_detector = CocoaSDKCrashDetector() From a2c982d88cb0a729de9aec89361aadb9423ca39f Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 08:39:03 +0200 Subject: [PATCH 30/37] move fixture to fixtures namespace --- fixtures/sdk_crash_detection/__init__.py | 0 fixtures/sdk_crash_detection/crash_event.py | 269 ++++++++++++++++++ .../sdk_crashes/test_sdk_crash_detection.py | 12 +- 3 files changed, 275 insertions(+), 6 deletions(-) create mode 100644 fixtures/sdk_crash_detection/__init__.py create mode 100644 fixtures/sdk_crash_detection/crash_event.py diff --git a/fixtures/sdk_crash_detection/__init__.py b/fixtures/sdk_crash_detection/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/sdk_crash_detection/crash_event.py b/fixtures/sdk_crash_detection/crash_event.py new file mode 100644 index 00000000000000..03e458adf30a89 --- /dev/null +++ b/fixtures/sdk_crash_detection/crash_event.py @@ -0,0 +1,269 @@ +from typing import Any, Mapping, Sequence + +IN_APP_FRAME = { + "function": "LoginViewController.viewDidAppear", + "raw_function": "LoginViewController.viewDidAppear(Bool)", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "filename": "LoginViewController.swift", + "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", + "lineno": 196, + "in_app": True, + "image_addr": "0x100260000", + "instruction_addr": "0x102b16630", + "symbol_addr": "0x100260000", +} + + +def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: + return { + "function": function, + "package": "Sentry", + "in_app": in_app, + "image_addr": "0x100304000", + } + + +def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: + frames = [ + get_sentry_frame(function, sentry_frame_in_app), + { + "function": "LoginViewController.viewDidAppear", + "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", + "package": "SentryApp", + "in_app": True, + "filename": "LoginViewController.swift", + "image_addr": "0x100260000", + }, + IN_APP_FRAME, + { + "function": "-[UIViewController _setViewAppearState:isAnimating:]", + "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1a4e8f000", + }, + { + "function": "-[UIViewController __viewDidAppear:]", + "symbol": "-[UIViewController __viewDidAppear:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1a4e8f000", + }, + { + "function": "-[UIViewController _endAppearanceTransition:]", + "symbol": "-[UIViewController _endAppearanceTransition:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1a4e8f000", + }, + { + "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1a4e8f000", + }, + { + "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", + "package": "UIKitCore", + "in_app": False, + "image_addr": "0x1a4e8f000", + }, + ] + + # The frames have to be ordered from caller to callee, or oldest to youngest. + # The last frame is the one creating the exception. + # As we usually look at stacktraces from youngest to oldest, we reverse the order. + return frames[::-1] + + +def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: + return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) + + +def get_crash_event_with_frames( + frames: Sequence[Mapping[str, Any]], handled=False, **kwargs +) -> Sequence[Mapping[str, Any]]: + result = { + "event_id": "80e3496eff734ab0ac993167aaa0d1cd", + "release": "5.222.5", + "type": "error", + "level": "fatal", + "platform": "cocoa", + "tags": {"level": "fatal"}, + "exception": { + "values": [ + { + "stacktrace": { + "frames": frames, + }, + "type": "SIGABRT", + "mechanism": {"handled": handled}, + } + ] + }, + "breadcrumbs": { + "values": [ + { + "timestamp": 1675900265.0, + "type": "debug", + "category": "started", + "level": "info", + "message": "Breadcrumb Tracking", + }, + ] + }, + "contexts": { + "app": { + "app_start_time": "2023-02-08T23:51:05Z", + "device_app_hash": "8854fe9e3d4e4a66493baee798bfae0228efabf1", + "build_type": "app store", + "app_identifier": "com.some.company.io", + "app_name": "SomeCompany", + "app_version": "5.222.5", + "app_build": "21036", + "app_id": "397D4F75-6C01-32D1-BF46-62098979E470", + "type": "app", + }, + "device": { + "family": "iOS", + "model": "iPhone14,8", + "model_id": "D28AP", + "arch": "arm64e", + "memory_size": 5944508416, + "free_memory": 102154240, + "usable_memory": 4125687808, + "storage_size": 127854202880, + "boot_time": "2023-02-01T05:21:23Z", + "timezone": "PST", + "type": "device", + }, + "os": { + "name": "iOS", + "version": "16.3", + "build": "20D47", + "kernel_version": "Darwin Kernel Version 22.3.0: Wed Jan 4 21:25:19 PST 2023; root:xnu-8792.82.2~1/RELEASE_ARM64_T8110", + "rooted": False, + "type": "os", + }, + }, + "debug_meta": { + "images": [ + { + "code_file": "/private/var/containers/Bundle/Application/895DA2DE-5FE3-44A0-8C0F-900519EA5516/iOS-Swift.app/iOS-Swift", + "debug_id": "aa8a3697-c88a-36f9-a687-3d3596568c8d", + "arch": "arm64", + "image_addr": "0x100260000", + "image_size": 180224, + "image_vmaddr": "0x100000000", + "type": "macho", + }, + { + "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/Frameworks/Sentry.framework/Sentry", + "debug_id": "e2623c4d-79c5-3cdf-90ab-2cf44e026bdd", + "arch": "arm64", + "image_addr": "0x100304000", + "image_size": 802816, + "type": "macho", + }, + { + "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore", + "debug_id": "b0858d8e-7220-37bf-873f-ecc2b0a358c3", + "arch": "arm64e", + "image_addr": "0x1a4e8f000", + "image_size": 25309184, + "image_vmaddr": "0x188ff7000", + "type": "macho", + }, + { + "code_file": "/System/Library/Frameworks/CFNetwork.framework/CFNetwork", + "debug_id": "b2273be9-538a-3f56-b9c7-801f39550f58", + "arch": "arm64e", + "image_addr": "0x1a3e32000", + "image_size": 3977216, + "image_vmaddr": "0x187f9a000", + "in_app": False, + "type": "macho", + }, + ] + }, + "environment": "test-app", + "sdk": { + "name": "sentry.cocoa", + "version": "8.1.0", + "integrations": [ + "Crash", + "PerformanceTracking", + "MetricKit", + "WatchdogTerminationTracking", + "ViewHierarchy", + "NetworkTracking", + "ANRTracking", + "AutoBreadcrumbTracking", + "FramesTracking", + "AppStartTracking", + "Screenshot", + "FileIOTracking", + "UIEventTracking", + "AutoSessionTracking", + "CoreDataTracking", + "PreWarmedAppStartTracing", + ], + }, + "threads": { + "values": [ + { + "id": 0, + "stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "data": {"symbolicator_status": "unknown_image"}, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "raw_stacktrace": { + "frames": [ + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x1129be52e", + "symbol_addr": "0x0", + }, + { + "function": "", + "in_app": False, + "image_addr": "0x0", + "instruction_addr": "0x104405f21", + "symbol_addr": "0x0", + }, + ], + }, + "crashed": True, + } + ] + }, + "user": { + "id": "803F5C87-0F8B-41C7-8499-27BD71A92738", + "ip_address": "192.168.0.1", + "geo": {"country_code": "US", "region": "United States"}, + }, + "logger": "my.logger.name", + } + result.update(kwargs) + return result 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 417aa2250fb859..df485e1210e507 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -5,6 +5,12 @@ import pytest from django.test.utils import override_settings +from fixtures.sdk_crash_detection.crash_event import ( + IN_APP_FRAME, + get_crash_event, + get_crash_event_with_frames, + get_sentry_frame, +) from sentry.eventstore.snuba.backend import SnubaEventStorage from sentry.issues.grouptype import PerformanceNPlusOneGroupType from sentry.testutils import TestCase @@ -12,12 +18,6 @@ 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 -from tests.sentry.utils.sdk_crashes.test_fixture import ( - IN_APP_FRAME, - get_crash_event, - get_crash_event_with_frames, - get_sentry_frame, -) class BaseSDKCrashDetectionMixin(BaseTestCase, metaclass=abc.ABCMeta): From af352ddd9989f37e95e607982a8ce41e284b4e8c Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 08:50:20 +0200 Subject: [PATCH 31/37] Remove empty inits --- src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py | 3 --- src/sentry/utils/sdk_crashes/sdk_crash_detection.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py index 826f1f4a64727d..7ecdc85d7fc5f3 100644 --- a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py @@ -5,9 +5,6 @@ class CocoaSDKCrashDetector(SDKCrashDetector): - def __init__(self) -> None: - self - def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool: if not frames: return False diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index b569325a966c1e..42163ca652f8f4 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -13,9 +13,6 @@ class SDKCrashReporter: - def __init__(self) -> None: - self - def report(self, event_data: Dict[str, Any], event_project_id: int) -> Event: from sentry.event_manager import EventManager From 629e2eb365e56e768c9c017bc551dff5a3f26302 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 08:53:29 +0200 Subject: [PATCH 32/37] snake_case not CamelCase --- src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py index 7ecdc85d7fc5f3..dc9761481055ba 100644 --- a/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py @@ -30,8 +30,8 @@ def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool: if "SentrySDK crash" in function: return False - functionsMatchers = ["*sentrycrash*", "**[[]Sentry*"] - for matcher in functionsMatchers: + function_matchers = ["*sentrycrash*", "**[[]Sentry*"] + for matcher in function_matchers: if glob_match(frame.get("function"), matcher, ignorecase=True): return True From 35e2b0ef3bd63427d5131d34e8e5b91b02e77ff0 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 08:56:28 +0200 Subject: [PATCH 33/37] make allowed event keys a frozenset --- .../utils/sdk_crashes/event_stripper.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 279c0a50aae25d..6550b190677915 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -4,18 +4,20 @@ 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", -} +ALLOWED_EVENT_KEYS = frozenset( + { + "type", + "datetime", + "timestamp", + "platform", + "sdk", + "level", + "logger", + "exception", + "debug_meta", + "contexts", + } +) def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Event: From 045a9f77c24a3e8ddb38b1c82193c2539ae6f6ab Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 09:48:28 +0200 Subject: [PATCH 34/37] feat(sdk-crash-detection): Event data allowlist (#50400) Specify an allowlist for the event data to ensure only keeping essential data doesn't contain PII. Fixes GH-49935 This PR is based on https://github.com/getsentry/sentry/pull/49928. Co-authored-by: Joris Bayer Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> --- .../utils/sdk_crashes/event_stripper.py | 106 ++++++++++++++---- .../utils/sdk_crashes/sdk_crash_detection.py | 2 +- .../utils/sdk_crashes/test_event_stripper.py | 60 ++++++++-- 3 files changed, 138 insertions(+), 30 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 6550b190677915..dad9095bbd734a 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -1,28 +1,64 @@ -from typing import Any, Mapping, Sequence, Tuple +from enum import Enum, auto +from typing import Any, Dict, Mapping, Optional, Sequence from sentry.eventstore.models import Event from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector -ALLOWED_EVENT_KEYS = frozenset( - { - "type", - "datetime", - "timestamp", - "platform", - "sdk", - "level", - "logger", - "exception", - "debug_meta", - "contexts", - } -) + +class Allow(Enum): + def __init__(self, explanation: str = "") -> None: + self.explanation = explanation + + """Keeps the event data if it is of type str, int, float, bool.""" + SIMPLE_TYPE = auto() + + """Keeps the event data no matter the type.""" + ALL = auto() + + """ + Doesn't keep the event data no matter the type. This can be used to explicitly + specify that data should be removed with an explanation. + """ + NEVER = auto() + + def with_explanation(self, explanation: str) -> "Allow": + self.explanation = explanation + return self + + +EVENT_DATA_ALLOWLIST = { + "type": Allow.SIMPLE_TYPE, + "datetime": Allow.SIMPLE_TYPE, + "timestamp": Allow.SIMPLE_TYPE, + "platform": Allow.SIMPLE_TYPE, + "sdk": { + "name": Allow.SIMPLE_TYPE, + "version": Allow.SIMPLE_TYPE, + "integrations": Allow.NEVER.with_explanation("Users can add their own integrations."), + }, + "exception": Allow.ALL.with_explanation("We strip the exception data separately."), + "debug_meta": Allow.ALL, + "contexts": { + "device": { + "family": Allow.SIMPLE_TYPE, + "model": Allow.SIMPLE_TYPE, + "arch": Allow.SIMPLE_TYPE, + }, + "os": { + "name": Allow.SIMPLE_TYPE, + "version": Allow.SIMPLE_TYPE, + "build": Allow.SIMPLE_TYPE, + }, + }, +} def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Event: - new_event_data = {key: value for key, value in event.data.items() if key in ALLOWED_EVENT_KEYS} - new_event_data["contexts"] = dict(filter(_filter_contexts, new_event_data["contexts"].items())) + new_event_data = _strip_event_data_with_allowlist(event.data, EVENT_DATA_ALLOWLIST) + + if (new_event_data is None) or (new_event_data == {}): + return stripped_frames: Sequence[Mapping[str, Any]] = [] frames = get_path(new_event_data, "exception", "values", -1, "stacktrace", "frames") @@ -39,11 +75,37 @@ def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Even return new_event_data -def _filter_contexts(pair: Tuple[str, Any]) -> bool: - key, _ = pair - if key in {"os", "device"}: - return True - return False +def _strip_event_data_with_allowlist( + data: Mapping[str, Any], allowlist: Optional[Mapping[str, Any]] +) -> Optional[Mapping[str, Any]]: + """ + Recursively traverses the data and only keeps values based on the allowlist. + """ + if allowlist is None: + return None + + stripped_data: Dict[str, Any] = {} + for data_key, data_value in data.items(): + allowlist_for_data = allowlist.get(data_key) + if allowlist_for_data is None: + continue + + if isinstance(allowlist_for_data, Allow): + allowed = allowlist_for_data + + if allowed is Allow.SIMPLE_TYPE and isinstance(data_value, (str, int, float, bool)): + stripped_data[data_key] = data_value + elif allowed is Allow.ALL: + stripped_data[data_key] = data_value + else: + continue + + elif isinstance(data_value, Mapping): + stripped_data[data_key] = _strip_event_data_with_allowlist( + data_value, allowlist_for_data + ) + + return stripped_data def _strip_debug_meta( diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index 42163ca652f8f4..dc5de8a9c02c71 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -55,7 +55,7 @@ def detect_sdk_crash(self, event: Event) -> Optional[Event]: return None if self.cocoa_sdk_crash_detector.is_sdk_crash(frames): - sdk_crash_event_data = strip_event_data(event, self.cocoa_sdk_crash_detector) + sdk_crash_event_data = strip_event_data(event.data, self.cocoa_sdk_crash_detector) set_path( sdk_crash_event_data, "contexts", "sdk_crash_detection", value={"detected": True} diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index 614066625ab9b5..ee4663153993cc 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -42,8 +42,6 @@ def test_strip_event_data_keeps_allowed_keys(self): "timestamp", "platform", "sdk", - "level", - "logger", "exception", "debug_meta", "contexts", @@ -74,11 +72,59 @@ def test_strip_event_data_strips_context(self): stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) - contexts = stripped_event_data.get("contexts") - assert contexts is not None - assert contexts.get("app") is None - assert contexts.get("os") is not None - assert contexts.get("device") is not None + assert stripped_event_data.get("contexts") == { + "os": { + "name": "iOS", + "version": "16.3", + "build": "20D47", + }, + "device": { + "family": "iOS", + "model": "iPhone14,8", + "arch": "arm64e", + }, + } + + def test_strip_event_data_strips_sdk(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) + + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + + assert stripped_event_data.get("sdk") == { + "name": "sentry.cocoa", + "version": "8.1.0", + } + + def test_strip_event_data_strips_value_if_not_simple_type(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) + event.data["type"] = {"foo": "bar"} + + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + + assert stripped_event_data.get("type") is None + + def test_strip_event_data_keeps_simple_types(self): + event = self.create_event( + data=get_crash_event(), + project_id=self.project.id, + ) + event.data["type"] = True + event.data["datetime"] = 0.1 + event.data["timestamp"] = 1 + event.data["platform"] = "cocoa" + + stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + + assert stripped_event_data.get("type") is True + assert stripped_event_data.get("datetime") == 0.1 + assert stripped_event_data.get("timestamp") == 1 + assert stripped_event_data.get("platform") == "cocoa" def test_strip_event_data_strips_non_referenced_dsyms(self): event = self.create_event( From e1785ff12c36a9ce1d8a16a597ad2e05e445d0f1 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 09:59:28 +0200 Subject: [PATCH 35/37] remove duplicate test fixtures --- .../utils/sdk_crashes/test_event_stripper.py | 12 +- .../sentry/utils/sdk_crashes/test_fixture.py | 269 ------------------ 2 files changed, 6 insertions(+), 275 deletions(-) delete mode 100644 tests/sentry/utils/sdk_crashes/test_fixture.py diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index ee4663153993cc..ac6581d7fd5118 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -1,18 +1,18 @@ import abc from unittest.mock import Mock +from fixtures.sdk_crash_detection.crash_event import ( + IN_APP_FRAME, + get_crash_event, + get_crash_event_with_frames, + get_frames, +) from sentry.testutils import TestCase from sentry.testutils.cases import BaseTestCase from sentry.testutils.silo import region_silo_test from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.cocoa_sdk_crash_detector import CocoaSDKCrashDetector from sentry.utils.sdk_crashes.event_stripper import strip_event_data -from tests.sentry.utils.sdk_crashes.test_fixture import ( - IN_APP_FRAME, - get_crash_event, - get_crash_event_with_frames, - get_frames, -) class BaseEventStripperMixin(BaseTestCase, metaclass=abc.ABCMeta): diff --git a/tests/sentry/utils/sdk_crashes/test_fixture.py b/tests/sentry/utils/sdk_crashes/test_fixture.py deleted file mode 100644 index 03e458adf30a89..00000000000000 --- a/tests/sentry/utils/sdk_crashes/test_fixture.py +++ /dev/null @@ -1,269 +0,0 @@ -from typing import Any, Mapping, Sequence - -IN_APP_FRAME = { - "function": "LoginViewController.viewDidAppear", - "raw_function": "LoginViewController.viewDidAppear(Bool)", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "filename": "LoginViewController.swift", - "abs_path": "/Users/sentry/git/iOS/Sentry/LoggedOut/LoginViewController.swift", - "lineno": 196, - "in_app": True, - "image_addr": "0x100260000", - "instruction_addr": "0x102b16630", - "symbol_addr": "0x100260000", -} - - -def get_sentry_frame(function: str, in_app: bool = False) -> Mapping[str, Any]: - return { - "function": function, - "package": "Sentry", - "in_app": in_app, - "image_addr": "0x100304000", - } - - -def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Mapping[str, Any]]: - frames = [ - get_sentry_frame(function, sentry_frame_in_app), - { - "function": "LoginViewController.viewDidAppear", - "symbol": "$s8Sentry9LoginViewControllerC13viewDidAppearyySbF", - "package": "SentryApp", - "in_app": True, - "filename": "LoginViewController.swift", - "image_addr": "0x100260000", - }, - IN_APP_FRAME, - { - "function": "-[UIViewController _setViewAppearState:isAnimating:]", - "symbol": "-[UIViewController _setViewAppearState:isAnimating:]", - "package": "UIKitCore", - "in_app": False, - "image_addr": "0x1a4e8f000", - }, - { - "function": "-[UIViewController __viewDidAppear:]", - "symbol": "-[UIViewController __viewDidAppear:]", - "package": "UIKitCore", - "in_app": False, - "image_addr": "0x1a4e8f000", - }, - { - "function": "-[UIViewController _endAppearanceTransition:]", - "symbol": "-[UIViewController _endAppearanceTransition:]", - "package": "UIKitCore", - "in_app": False, - "image_addr": "0x1a4e8f000", - }, - { - "function": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "symbol": "-[UINavigationController navigationTransitionView:didEndTransition:fromView:toView:]", - "package": "UIKitCore", - "in_app": False, - "image_addr": "0x1a4e8f000", - }, - { - "function": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "symbol": "__49-[UINavigationController _startCustomTransition:]_block_invoke", - "package": "UIKitCore", - "in_app": False, - "image_addr": "0x1a4e8f000", - }, - ] - - # The frames have to be ordered from caller to callee, or oldest to youngest. - # The last frame is the one creating the exception. - # As we usually look at stacktraces from youngest to oldest, we reverse the order. - return frames[::-1] - - -def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: - return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) - - -def get_crash_event_with_frames( - frames: Sequence[Mapping[str, Any]], handled=False, **kwargs -) -> Sequence[Mapping[str, Any]]: - result = { - "event_id": "80e3496eff734ab0ac993167aaa0d1cd", - "release": "5.222.5", - "type": "error", - "level": "fatal", - "platform": "cocoa", - "tags": {"level": "fatal"}, - "exception": { - "values": [ - { - "stacktrace": { - "frames": frames, - }, - "type": "SIGABRT", - "mechanism": {"handled": handled}, - } - ] - }, - "breadcrumbs": { - "values": [ - { - "timestamp": 1675900265.0, - "type": "debug", - "category": "started", - "level": "info", - "message": "Breadcrumb Tracking", - }, - ] - }, - "contexts": { - "app": { - "app_start_time": "2023-02-08T23:51:05Z", - "device_app_hash": "8854fe9e3d4e4a66493baee798bfae0228efabf1", - "build_type": "app store", - "app_identifier": "com.some.company.io", - "app_name": "SomeCompany", - "app_version": "5.222.5", - "app_build": "21036", - "app_id": "397D4F75-6C01-32D1-BF46-62098979E470", - "type": "app", - }, - "device": { - "family": "iOS", - "model": "iPhone14,8", - "model_id": "D28AP", - "arch": "arm64e", - "memory_size": 5944508416, - "free_memory": 102154240, - "usable_memory": 4125687808, - "storage_size": 127854202880, - "boot_time": "2023-02-01T05:21:23Z", - "timezone": "PST", - "type": "device", - }, - "os": { - "name": "iOS", - "version": "16.3", - "build": "20D47", - "kernel_version": "Darwin Kernel Version 22.3.0: Wed Jan 4 21:25:19 PST 2023; root:xnu-8792.82.2~1/RELEASE_ARM64_T8110", - "rooted": False, - "type": "os", - }, - }, - "debug_meta": { - "images": [ - { - "code_file": "/private/var/containers/Bundle/Application/895DA2DE-5FE3-44A0-8C0F-900519EA5516/iOS-Swift.app/iOS-Swift", - "debug_id": "aa8a3697-c88a-36f9-a687-3d3596568c8d", - "arch": "arm64", - "image_addr": "0x100260000", - "image_size": 180224, - "image_vmaddr": "0x100000000", - "type": "macho", - }, - { - "code_file": "/private/var/containers/Bundle/Application/9EB557CD-D653-4F51-BFCE-AECE691D4347/iOS-Swift.app/Frameworks/Sentry.framework/Sentry", - "debug_id": "e2623c4d-79c5-3cdf-90ab-2cf44e026bdd", - "arch": "arm64", - "image_addr": "0x100304000", - "image_size": 802816, - "type": "macho", - }, - { - "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore", - "debug_id": "b0858d8e-7220-37bf-873f-ecc2b0a358c3", - "arch": "arm64e", - "image_addr": "0x1a4e8f000", - "image_size": 25309184, - "image_vmaddr": "0x188ff7000", - "type": "macho", - }, - { - "code_file": "/System/Library/Frameworks/CFNetwork.framework/CFNetwork", - "debug_id": "b2273be9-538a-3f56-b9c7-801f39550f58", - "arch": "arm64e", - "image_addr": "0x1a3e32000", - "image_size": 3977216, - "image_vmaddr": "0x187f9a000", - "in_app": False, - "type": "macho", - }, - ] - }, - "environment": "test-app", - "sdk": { - "name": "sentry.cocoa", - "version": "8.1.0", - "integrations": [ - "Crash", - "PerformanceTracking", - "MetricKit", - "WatchdogTerminationTracking", - "ViewHierarchy", - "NetworkTracking", - "ANRTracking", - "AutoBreadcrumbTracking", - "FramesTracking", - "AppStartTracking", - "Screenshot", - "FileIOTracking", - "UIEventTracking", - "AutoSessionTracking", - "CoreDataTracking", - "PreWarmedAppStartTracing", - ], - }, - "threads": { - "values": [ - { - "id": 0, - "stacktrace": { - "frames": [ - { - "function": "", - "in_app": False, - "data": {"symbolicator_status": "unknown_image"}, - "image_addr": "0x0", - "instruction_addr": "0x1129be52e", - "symbol_addr": "0x0", - }, - { - "function": "", - "in_app": False, - "data": {"symbolicator_status": "unknown_image"}, - "image_addr": "0x0", - "instruction_addr": "0x104405f21", - "symbol_addr": "0x0", - }, - ], - }, - "raw_stacktrace": { - "frames": [ - { - "function": "", - "in_app": False, - "image_addr": "0x0", - "instruction_addr": "0x1129be52e", - "symbol_addr": "0x0", - }, - { - "function": "", - "in_app": False, - "image_addr": "0x0", - "instruction_addr": "0x104405f21", - "symbol_addr": "0x0", - }, - ], - }, - "crashed": True, - } - ] - }, - "user": { - "id": "803F5C87-0F8B-41C7-8499-27BD71A92738", - "ip_address": "192.168.0.1", - "geo": {"country_code": "US", "region": "United States"}, - }, - "logger": "my.logger.name", - } - result.update(kwargs) - return result From c710f3636a73279a2579e1ff8a955455e4b9d051 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 10:00:47 +0200 Subject: [PATCH 36/37] fix post process tests --- tests/sentry/tasks/test_post_process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index 9686a096290cdb..755bc4c24acee6 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -11,6 +11,7 @@ from django.test import override_settings from django.utils import timezone +from fixtures.sdk_crash_detection.crash_event import get_crash_event from sentry import buffer from sentry.buffer.redis import RedisBuffer from sentry.db.postgres.roles import in_test_psql_role_override @@ -61,7 +62,6 @@ 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: From ea6b6dc281a0a833cd58f4ff2e8274e26ee9e654 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 12 Jun 2023 10:47:11 +0200 Subject: [PATCH 37/37] fix type issues --- fixtures/sdk_crash_detection/crash_event.py | 6 ++-- src/sentry/conf/server.py | 2 +- src/sentry/tasks/post_process.py | 4 ++- .../utils/sdk_crashes/event_stripper.py | 10 +++--- .../utils/sdk_crashes/sdk_crash_detection.py | 14 +++----- .../utils/sdk_crashes/test_event_stripper.py | 20 ++++++----- .../sdk_crashes/test_sdk_crash_detection.py | 36 ++++++++++--------- 7 files changed, 49 insertions(+), 43 deletions(-) diff --git a/fixtures/sdk_crash_detection/crash_event.py b/fixtures/sdk_crash_detection/crash_event.py index 03e458adf30a89..89ddebeb3f06eb 100644 --- a/fixtures/sdk_crash_detection/crash_event.py +++ b/fixtures/sdk_crash_detection/crash_event.py @@ -1,4 +1,4 @@ -from typing import Any, Mapping, Sequence +from typing import Any, Collection, Dict, Mapping, Sequence IN_APP_FRAME = { "function": "LoginViewController.viewDidAppear", @@ -79,13 +79,13 @@ def get_frames(function: str, sentry_frame_in_app: bool = False) -> Sequence[Map return frames[::-1] -def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Sequence[Mapping[str, Any]]: +def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> Dict[str, Collection[str]]: return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs) def get_crash_event_with_frames( frames: Sequence[Mapping[str, Any]], handled=False, **kwargs -) -> Sequence[Mapping[str, Any]]: +) -> Dict[str, Collection[str]]: result = { "event_id": "80e3496eff734ab0ac993167aaa0d1cd", "release": "5.222.5", diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 6fbb510f973f47..14e572a318c36c 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -3450,4 +3450,4 @@ class TopicDefinition(TypedDict): SENTRY_QUEUE_MONITORING_REDIS_CLUSTER = "default" # The project ID for SDK Crash Monitoring to save the detected SDK crashed to. -SDK_CRASH_DETECTION_PROJECT_ID = None +SDK_CRASH_DETECTION_PROJECT_ID: Optional[int] = None diff --git a/src/sentry/tasks/post_process.py b/src/sentry/tasks/post_process.py index 3f9c5d500c75a2..3a4758f1d9ebae 100644 --- a/src/sentry/tasks/post_process.py +++ b/src/sentry/tasks/post_process.py @@ -1075,7 +1075,9 @@ def sdk_crash_monitoring(job: PostProcessJob): 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) + sdk_crash_detection.detect_sdk_crash( + event=event, event_project_id=settings.SDK_CRASH_DETECTION_PROJECT_ID + ) def plugin_post_process_group(plugin_slug, event, **kwargs): diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index dad9095bbd734a..741aa916ba6d15 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -1,7 +1,7 @@ from enum import Enum, auto from typing import Any, Dict, Mapping, Optional, Sequence -from sentry.eventstore.models import Event +from sentry.db.models import NodeData from sentry.utils.safe import get_path from sentry.utils.sdk_crashes.sdk_crash_detector import SDKCrashDetector @@ -54,11 +54,13 @@ def with_explanation(self, explanation: str) -> "Allow": } -def strip_event_data(event: Event, sdk_crash_detector: SDKCrashDetector) -> Event: - new_event_data = _strip_event_data_with_allowlist(event.data, EVENT_DATA_ALLOWLIST) +def strip_event_data( + event_data: NodeData, sdk_crash_detector: SDKCrashDetector +) -> Mapping[str, Any]: + new_event_data = _strip_event_data_with_allowlist(event_data, EVENT_DATA_ALLOWLIST) if (new_event_data is None) or (new_event_data == {}): - return + return {} stripped_frames: Sequence[Mapping[str, Any]] = [] frames = get_path(new_event_data, "exception", "values", -1, "stacktrace", "frames") diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py index dc5de8a9c02c71..096a2f17f45d88 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detection.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detection.py @@ -1,8 +1,6 @@ from __future__ import annotations -from typing import Any, Dict, Optional - -from django.conf import settings +from typing import Any, Mapping, Optional from sentry.eventstore.models import Event from sentry.issues.grouptype import GroupCategory @@ -13,10 +11,10 @@ class SDKCrashReporter: - def report(self, event_data: Dict[str, Any], event_project_id: int) -> Event: + def report(self, event_data: Mapping[str, Any], event_project_id: int) -> Event: from sentry.event_manager import EventManager - manager = EventManager(event_data) + manager = EventManager(dict(event_data)) manager.normalize() return manager.save(project_id=event_project_id) @@ -31,7 +29,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) -> Optional[Event]: + def detect_sdk_crash(self, event: Event, event_project_id: int) -> Optional[Event]: should_detect_sdk_crash = ( event.group and event.group.issue_category == GroupCategory.ERROR @@ -61,9 +59,7 @@ def detect_sdk_crash(self, event: Event) -> Optional[Event]: 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 - ) + return self.sdk_crash_reporter.report(sdk_crash_event_data, event_project_id) return None diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index ac6581d7fd5118..06bc5423e6f896 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -31,7 +31,7 @@ def test_strip_event_data_keeps_allowed_keys(self): project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) keys_removed = {"tags", "user", "threads", "breadcrumbs", "environment"} for key in keys_removed: @@ -52,14 +52,16 @@ def test_strip_event_data_keeps_allowed_keys(self): def test_strip_event_data_without_debug_meta(self): event_data = get_crash_event() - event_data["debug_meta"]["images"] = None + debug_meta = dict(get_path(event_data, "debug_meta")) + debug_meta.pop("images") + event_data["debug_meta"] = debug_meta event = self.create_event( data=event_data, project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) debug_meta_images = get_path(stripped_event_data, "debug_meta", "images") assert debug_meta_images is None @@ -70,7 +72,7 @@ def test_strip_event_data_strips_context(self): project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) assert stripped_event_data.get("contexts") == { "os": { @@ -91,7 +93,7 @@ def test_strip_event_data_strips_sdk(self): project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) assert stripped_event_data.get("sdk") == { "name": "sentry.cocoa", @@ -105,7 +107,7 @@ def test_strip_event_data_strips_value_if_not_simple_type(self): ) event.data["type"] = {"foo": "bar"} - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) assert stripped_event_data.get("type") is None @@ -119,7 +121,7 @@ def test_strip_event_data_keeps_simple_types(self): event.data["timestamp"] = 1 event.data["platform"] = "cocoa" - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) assert stripped_event_data.get("type") is True assert stripped_event_data.get("datetime") == 0.1 @@ -132,7 +134,7 @@ def test_strip_event_data_strips_non_referenced_dsyms(self): project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, Mock()) + stripped_event_data = strip_event_data(event.data, Mock()) debug_meta_images = get_path(stripped_event_data, "debug_meta", "images") @@ -156,7 +158,7 @@ def _execute_strip_frames_test(self, frames): project_id=self.project.id, ) - stripped_event_data = strip_event_data(event, CocoaSDKCrashDetector()) + stripped_event_data = strip_event_data(event.data, CocoaSDKCrashDetector()) stripped_frames = get_path( stripped_event_data, "exception", "values", -1, "stacktrace", "frames" 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 df485e1210e507..e537d82b204f05 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -1,9 +1,8 @@ import abc -from typing import Any, Mapping, Sequence +from typing import Collection, Dict from unittest.mock import patch import pytest -from django.test.utils import override_settings from fixtures.sdk_crash_detection.crash_event import ( IN_APP_FRAME, @@ -17,6 +16,7 @@ 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.safe import set_path from sentry.utils.sdk_crashes.sdk_crash_detection import sdk_crash_detection @@ -32,7 +32,7 @@ def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): project_id=self.project.id, ) - sdk_crash_detection.detect_sdk_crash(event=event) + sdk_crash_detection.detect_sdk_crash(event=event, event_project_id=1234) if should_be_reported: mock_sdk_crash_reporter.report.assert_called_once() @@ -44,7 +44,9 @@ def execute_test(self, event_data, should_be_reported, mock_sdk_crash_reporter): @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter") -class PerformanceEventTestMixin(BaseSDKCrashDetectionMixin, PerfIssueTransactionTestMixin): +class PerformanceEventTestMixin( + BaseSDKCrashDetectionMixin, SnubaTestCase, PerfIssueTransactionTestMixin +): def test_performance_event_not_detected(self, mock_sdk_crash_reporter): fingerprint = "some_group" fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}" @@ -54,7 +56,7 @@ def test_performance_event_not_detected(self, mock_sdk_crash_reporter): fingerprint=[fingerprint], ) - sdk_crash_detection.detect_sdk_crash(event=event) + sdk_crash_detection.detect_sdk_crash(event=event, event_project_id=1234) mock_sdk_crash_reporter.report.assert_not_called() @@ -78,7 +80,8 @@ def test_no_exception_not_detected(self, mock_sdk_crash_reporter): def test_sdk_crash_detected_event_is_not_reported(self, mock_sdk_crash_reporter): event = get_crash_event() - event["contexts"]["sdk_crash_detection"] = {"detected": True} + + set_path(event, "contexts", "sdk_crash_detection", value={"detected": True}) self.execute_test(event, False, mock_sdk_crash_reporter) @@ -175,7 +178,7 @@ def test_filename_includes_senry_not_reported(self, mock_sdk_crash_reporter): mock_sdk_crash_reporter, ) - def _get_crash_event(self, filename) -> Sequence[Mapping[str, Any]]: + def _get_crash_event(self, filename) -> Dict[str, Collection[str]]: return get_crash_event_with_frames( frames=[ { @@ -307,18 +310,19 @@ def test_sdk_crash_event_stored_to_sdk_crash_project(self): 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) + sdk_crash_event = sdk_crash_detection.detect_sdk_crash( + event=event, event_project_id=cocoa_sdk_crashes_project.id + ) - assert sdk_crash_event is not None + 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 - ) + 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 + 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