From 86f86052319a5a10b630042814533614044840a3 Mon Sep 17 00:00:00 2001 From: William Mak Date: Tue, 22 Nov 2022 10:41:24 -0500 Subject: [PATCH 1/7] feat(fileio): Introduce file io performance problem - Introduce the file io on main thread performance problem. Not hooking up the options or anything yet. Just want to get the main core of the code ready to go first - I think this won't actually create any of the file io perf problems yet since the option is only added to default.py with 0 --- .../file-io-on-main-thread.json | 1 + src/sentry/options/defaults.py | 1 + src/sentry/projectoptions/defaults.py | 2 + src/sentry/types/issues.py | 3 ++ .../performance_detection.py | 51 +++++++++++++++++++ .../test_performance_detection.py | 25 +++++++++ 6 files changed, 83 insertions(+) create mode 100644 fixtures/events/performance_problems/file-io-on-main-thread.json diff --git a/fixtures/events/performance_problems/file-io-on-main-thread.json b/fixtures/events/performance_problems/file-io-on-main-thread.json new file mode 100644 index 00000000000000..88380d9df2933a --- /dev/null +++ b/fixtures/events/performance_problems/file-io-on-main-thread.json @@ -0,0 +1 @@ +{"event_id":"c119e45a9d724b1891df4651ebf9e6db","project":5428559,"release":"io.sentry.samples.android@1.1.0+2","dist":"2","platform":"java","message":"","datetime":"2022-11-21T11:57:38.806589+00:00","tags":[["device","Android SDK built for x86"],["device.family","Android"],["environment","debug"],["isSideLoaded","true"],["level","info"],["os","Android 10"],["os.name","Android"],["os.rooted","no"],["dist","2"],["release","io.sentry.samples.android@1.1.0+2"],["user","id:0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e"],["transaction","MainActivity.add_attachment"]],"_metrics":{"bytes.ingested.event":3725,"bytes.stored.event":4851},"breadcrumbs":{"values":[{"timestamp":1669031851.824,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"created"}},{"timestamp":1669031851.942,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"started"}},{"timestamp":1669031851.943,"type":"session","category":"app.lifecycle","level":"info","data":{"state":"start"}},{"timestamp":1669031851.95,"type":"navigation","category":"app.lifecycle","level":"info","data":{"state":"foreground"}},{"timestamp":1669031851.953,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"resumed"}},{"timestamp":1669031858.676,"type":"user","category":"ui.click","level":"info","data":{"view.class":"androidx.appcompat.widget.AppCompatButton","view.id":"add_attachment"}}]},"breakdowns":{"span_ops":{"total.time":{"value":94.411134,"unit":"millisecond"}}},"contexts":{"app":{"app_start_time":"2022-11-21T11:57:31.523Z","app_identifier":"io.sentry.samples.android","app_name":"Sentry sample","app_version":"1.1.0","app_build":"2","permissions":{"ACCESS_NETWORK_STATE":"granted","CAMERA":"not_granted","FOREGROUND_SERVICE":"granted","INTERNET":"granted","READ_EXTERNAL_STORAGE":"not_granted","READ_PHONE_STATE":"not_granted","WRITE_EXTERNAL_STORAGE":"not_granted"},"type":"app"},"device":{"name":"Android SDK built for x86","family":"Android","model":"Android SDK built for x86","model_id":"QSR1.190920.001","orientation":"portrait","manufacturer":"Google","brand":"google","screen_density":2.625,"screen_dpi":420,"simulator":true,"boot_time":"2022-11-21T10:57:17.182Z","timezone":"Europe/Vienna","archs":["x86"],"id":"0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e","language":"en","locale":"en_US","screen_height_pixels":1794,"screen_width_pixels":1080,"type":"device"},"os":{"name":"Android","version":"10","build":"QSR1.190920.001","kernel_version":"4.14.112+","rooted":false,"type":"os"},"trace":{"trace_id":"b2a33f3f79fe4a7c8de3426725a045cb","span_id":"b93d2be92cd64fd5","op":"ui.action.click","status":"ok","exclusive_time":35.177709,"client_sample_rate":1.0,"hash":"1ff9a18d6a6b09a8","type":"trace"}},"culprit":"MainActivity.add_attachment","environment":"debug","grouping_config":{"enhancements":"eJybzDRxY3J-bm5-npWRgaGlroGxrpHxxEkT1-Zm5usVp-aVFFXqaWlNZAQAKGsOFg","id":"newstyle:2019-10-29"},"hashes":[],"ingest_path":[{"version":"22.11.0","public_key":"XE7QiyuNlja9PZ7I9qJlwQotzecWrUIN91BAO7Q5R38"}],"key_id":"1336851","level":"info","location":"MainActivity.add_attachment","logger":"","metadata":{"location":"MainActivity.add_attachment","title":"MainActivity.add_attachment"},"nodestore_insert":1669031864.427658,"received":1669031861.899161,"sdk":{"name":"sentry.java.android.timber","version":"6.8.0","packages":[{"name":"maven:io.sentry:sentry","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-core","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-ndk","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-timber","version":"6.8.0"}]},"span_grouping_config":{"id":"default:2022-10-27"},"spans":[{"timestamp":1669031858.806411,"start_timestamp":1669031857.712,"exclusive_time":1094.411134,"description":"1669031858711_file.txt (4.0 kB)","op":"file.write","span_id":"054ba3a374d543eb","parent_span_id":"b93d2be92cd64fd5","trace_id":"b2a33f3f79fe4a7c8de3426725a045cb","status":"ok","data":{"blocked_ui_thread":true,"call_stack":[{"function":"onClick","in_app":true,"lineno":2,"module":"io.sentry.samples.android.MainActivity$$ExternalSyntheticLambda6","native":false},{"filename":"MainActivity.java","function":"lambda$onCreate$5$io-sentry-samples-android-MainActivity","in_app":true,"lineno":93,"module":"io.sentry.samples.android.MainActivity","native":false}],"file.path":"/data/user/0/io.sentry.samples.android/files/1669031858711_file.txt","file.size":4010},"hash":"8add714f71a52ef2"}],"start_timestamp":1669031858.677,"timestamp":1669031858.806589,"title":"MainActivity.add_attachment","transaction":"MainActivity.add_attachment","transaction_info":{"source":"component"},"type":"transaction","user":{"id":"0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e","ip_address":"86.56.213.87","geo":{"country_code":"AT","city":"Enns","region":"Austria"}},"version":"7"} diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index e644acdd15029a..ed2049b4d687ec 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -523,6 +523,7 @@ register("performance.issues.n_plus_one_db.problem-detection", default=0.0) register("performance.issues.n_plus_one_db.problem-creation", default=0.0) register("performance.issues.n_plus_one_db_ext.problem-creation", default=0.0) +register("performance.issues.file_io_main_thread-creation", default=0.0) # System-wide options for default performance detection settings for any org opted into the performance-issues-ingest feature. Meant for rollout. register("performance.issues.n_plus_one_db.count_threshold", default=5) diff --git a/src/sentry/projectoptions/defaults.py b/src/sentry/projectoptions/defaults.py index 07802b2bd55a8f..8cfee200c5ee9f 100644 --- a/src/sentry/projectoptions/defaults.py +++ b/src/sentry/projectoptions/defaults.py @@ -102,6 +102,8 @@ "n_plus_one_db_issue_rate": 0, "n_plus_one_db_count": 5, "n_plus_one_db_duration_threshold": 500, + # 16ms is when frame drops will start being evident + "file_io_main_thread_duration_threshold": 16, }, ) diff --git a/src/sentry/types/issues.py b/src/sentry/types/issues.py index 31c6eb542986cd..a01e88aa43dfe4 100644 --- a/src/sentry/types/issues.py +++ b/src/sentry/types/issues.py @@ -12,6 +12,7 @@ class GroupType(Enum): PERFORMANCE_DUPLICATE_SPANS = 1005 PERFORMANCE_N_PLUS_ONE_DB_QUERIES = 1006 PERFORMANCE_CONSECUTIVE_DB_OP = 1007 + PERFORMANCE_FILE_IO_MAIN_THREAD = 1008 class GroupCategory(Enum): @@ -28,6 +29,7 @@ class GroupCategory(Enum): GroupType.PERFORMANCE_RENDER_BLOCKING_ASSET_SPAN: GroupCategory.PERFORMANCE, GroupType.PERFORMANCE_DUPLICATE_SPANS: GroupCategory.PERFORMANCE, GroupType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES: GroupCategory.PERFORMANCE, + GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD: GroupCategory.PERFORMANCE, } GROUP_TYPE_TO_TEXT = { @@ -39,6 +41,7 @@ class GroupCategory(Enum): GroupType.PERFORMANCE_RENDER_BLOCKING_ASSET_SPAN: "Render Blocking Asset Span", GroupType.PERFORMANCE_DUPLICATE_SPANS: "Duplicate Spans", GroupType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES: "N+1 Query", + GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD: "File IO on Main Thread", } diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index f0cefed7cbc4e1..47b9d2b29f47c3 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -48,6 +48,7 @@ class DetectorType(Enum): N_PLUS_ONE_DB_QUERIES = "n_plus_one_db" N_PLUS_ONE_DB_QUERIES_EXTENDED = "n_plus_one_db_ext" CONSECUTIVE_DB_OP = "consecutive_db" + FILE_IO_MAIN_THREAD = "file_io_main_thread" DETECTOR_TYPE_TO_GROUP_TYPE = { @@ -62,12 +63,14 @@ class DetectorType(Enum): DetectorType.N_PLUS_ONE_DB_QUERIES: GroupType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES, DetectorType.N_PLUS_ONE_DB_QUERIES_EXTENDED: GroupType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES, DetectorType.CONSECUTIVE_DB_OP: GroupType.PERFORMANCE_CONSECUTIVE_DB_OP, + DetectorType.FILE_IO_MAIN_THREAD: GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD, } # Detector and the corresponding system option must be added to this list to have issues created. DETECTOR_TYPE_ISSUE_CREATION_TO_SYSTEM_OPTION = { DetectorType.N_PLUS_ONE_DB_QUERIES: "performance.issues.n_plus_one_db.problem-creation", DetectorType.N_PLUS_ONE_DB_QUERIES_EXTENDED: "performance.issues.n_plus_one_db_ext.problem-creation", + DetectorType.FILE_IO_MAIN_THREAD: "performance.issues.file_io_main_thread-creation", } @@ -298,6 +301,11 @@ def get_detection_settings(project_id: Optional[str] = None): "duration_threshold": 0, # ms "consecutive_count_threshold": 2, }, + DetectorType.FILE_IO_MAIN_THREAD: [ + { + "duration_threshold": settings["file_io_main_thread_duration_threshold"], + } + ], } @@ -321,6 +329,7 @@ def _detect_performance_problems(data: Event, sdk_span: Any) -> List[Performance DetectorType.N_PLUS_ONE_DB_QUERIES_EXTENDED: NPlusOneDBSpanDetectorExtended( detection_settings, data ), + DetectorType.FILE_IO_MAIN_THREAD: FileIOMainThreadDetector(detection_settings, data), } for _, detector in detectors.items(): @@ -1205,6 +1214,48 @@ class NPlusOneDBSpanDetectorExtended(NPlusOneDBSpanDetector): ) +class FileIOMainThreadDetector(PerformanceDetector): + """ + Checks for a file io span on the main thread + """ + + __slots__ = ("spans_involved", "stored_problems") + + settings_key = DetectorType.FILE_IO_MAIN_THREAD + + def init(self): + self.spans_involved = {} + self.most_recent_start_time = {} + self.most_recent_hash = {} + self.stored_problems = {} + + def visit_span(self, span: Span): + if self._is_file_io_on_main_thread(span): + settings_for_span = self.settings_for_span(span) + if not settings_for_span: + return + + op, span_id, op_prefix, span_duration, settings = settings_for_span + if span_duration.total_seconds() * 1000 > settings["duration_threshold"]: + call_stack = ".".join( + [item["function"] for item in span.get("data", {}).get("call_stack", [])] + ).encode("utf8") + hashed_stack = hashlib.sha1(call_stack).hexdigest() + fingerprint = f"1-{GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD}-{hashed_stack}" + self.stored_problems[fingerprint] = PerformanceProblem( + fingerprint=fingerprint, + op=span.get("op"), + desc=span.get("description", ""), + parent_span_ids=[], + type=GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD, + cause_span_ids=[span.get("span_id", None)], + offender_span_ids=[span.get("span_id", None)], + ) + + def _is_file_io_on_main_thread(self, span: Span): + return span.get("data", {}).get("blocked_ui_thread", False) + + # Reports metrics and creates spans for detection def report_metrics_for_detectors( event: Event, event_id: Optional[str], detectors: Dict[str, PerformanceDetector], sdk_span: Any diff --git a/tests/sentry/utils/performance_issues/test_performance_detection.py b/tests/sentry/utils/performance_issues/test_performance_detection.py index c890234d2d4805..2239702e520ef7 100644 --- a/tests/sentry/utils/performance_issues/test_performance_detection.py +++ b/tests/sentry/utils/performance_issues/test_performance_detection.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import Mock, call, patch +import pytest + from sentry import projectoptions from sentry.eventstore.models import Event from sentry.testutils import TestCase @@ -28,10 +30,12 @@ BASE_DETECTOR_OPTIONS = { "performance.issues.n_plus_one_db.problem-creation": 1.0, "performance.issues.n_plus_one_db_ext.problem-creation": 1.0, + "performance.issues.file_io_main_thread-creation": 1.0, } BASE_DETECTOR_OPTIONS_OFF = { "performance.issues.n_plus_one_db.problem-creation": 0.0, "performance.issues.n_plus_one_db_ext.problem-creation": 0.0, + "performance.issues.file_io_main_thread-creation": 0.0, } @@ -60,6 +64,7 @@ def assert_n_plus_one_db_problem(perf_problems): ] +@pytest.mark.django_db class PerformanceDetectionTest(unittest.TestCase): def setUp(self): super().setUp() @@ -980,6 +985,26 @@ def test_detects_consecutive_db_spans(self): ] ) + def test_detects_file_io_main_thread(self): + file_io_event = EVENTS["file-io-on-main-thread"] + sdk_span_mock = Mock() + + _detect_performance_problems(file_io_event, sdk_span_mock) + + assert sdk_span_mock.containing_transaction.set_tag.call_count == 5 + sdk_span_mock.containing_transaction.set_tag.assert_has_calls( + [ + call("_pi_all_issue_count", 1), + call("_pi_sdk_name", "sentry.java.android.timber"), + call("_pi_transaction", "c119e45a9d724b1891df4651ebf9e6db"), + call( + "_pi_file_io_main_thread_fp", + "1-GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD-2fdacf08551ceadba1cb7cf4eace5e288be4f9cf", + ), + call("_pi_file_io_main_thread", "054ba3a374d543eb"), + ] + ) + class PrepareProblemForGroupingTest(unittest.TestCase): def test(self): From 6cd537a53d8dc03dab77ec667e1a1b1a77d6fea5 Mon Sep 17 00:00:00 2001 From: William Mak Date: Tue, 22 Nov 2022 10:52:10 -0500 Subject: [PATCH 2/7] lets be safe, remove the option entirely --- src/sentry/utils/performance_issues/performance_detection.py | 1 - .../utils/performance_issues/test_performance_detection.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 47b9d2b29f47c3..314b986ee5e51a 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -70,7 +70,6 @@ class DetectorType(Enum): DETECTOR_TYPE_ISSUE_CREATION_TO_SYSTEM_OPTION = { DetectorType.N_PLUS_ONE_DB_QUERIES: "performance.issues.n_plus_one_db.problem-creation", DetectorType.N_PLUS_ONE_DB_QUERIES_EXTENDED: "performance.issues.n_plus_one_db_ext.problem-creation", - DetectorType.FILE_IO_MAIN_THREAD: "performance.issues.file_io_main_thread-creation", } diff --git a/tests/sentry/utils/performance_issues/test_performance_detection.py b/tests/sentry/utils/performance_issues/test_performance_detection.py index 2239702e520ef7..022aa5261bb6a8 100644 --- a/tests/sentry/utils/performance_issues/test_performance_detection.py +++ b/tests/sentry/utils/performance_issues/test_performance_detection.py @@ -30,12 +30,10 @@ BASE_DETECTOR_OPTIONS = { "performance.issues.n_plus_one_db.problem-creation": 1.0, "performance.issues.n_plus_one_db_ext.problem-creation": 1.0, - "performance.issues.file_io_main_thread-creation": 1.0, } BASE_DETECTOR_OPTIONS_OFF = { "performance.issues.n_plus_one_db.problem-creation": 0.0, "performance.issues.n_plus_one_db_ext.problem-creation": 0.0, - "performance.issues.file_io_main_thread-creation": 0.0, } From 448a0765147f1ff724d624708efa6b7442b72ed3 Mon Sep 17 00:00:00 2001 From: William Mak Date: Tue, 22 Nov 2022 12:11:28 -0500 Subject: [PATCH 3/7] Removing projectsetting since we don't really need it --- src/sentry/projectoptions/defaults.py | 2 -- src/sentry/utils/performance_issues/performance_detection.py | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sentry/projectoptions/defaults.py b/src/sentry/projectoptions/defaults.py index 8cfee200c5ee9f..07802b2bd55a8f 100644 --- a/src/sentry/projectoptions/defaults.py +++ b/src/sentry/projectoptions/defaults.py @@ -102,8 +102,6 @@ "n_plus_one_db_issue_rate": 0, "n_plus_one_db_count": 5, "n_plus_one_db_duration_threshold": 500, - # 16ms is when frame drops will start being evident - "file_io_main_thread_duration_threshold": 16, }, ) diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 314b986ee5e51a..8a8eea4e2d9976 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -302,7 +302,8 @@ def get_detection_settings(project_id: Optional[str] = None): }, DetectorType.FILE_IO_MAIN_THREAD: [ { - "duration_threshold": settings["file_io_main_thread_duration_threshold"], + # 16ms is when frame drops will start being evident + "duration_threshold": 16, } ], } From 237d999f4e72f32b086e44e479b8dfb890e5521f Mon Sep 17 00:00:00 2001 From: William Mak Date: Tue, 22 Nov 2022 14:20:24 -0500 Subject: [PATCH 4/7] fix: handle data being potentially None --- .../utils/performance_issues/performance_detection.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 8a8eea4e2d9976..0ef555b9fe0c6d 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -1252,8 +1252,12 @@ def visit_span(self, span: Span): offender_span_ids=[span.get("span_id", None)], ) - def _is_file_io_on_main_thread(self, span: Span): - return span.get("data", {}).get("blocked_ui_thread", False) + def _is_file_io_on_main_thread(self, span: Span) -> bool: + data = span.get("data", {}) + if data is None: + return False + # doing is True since the value can be any type + return data.get("blocked_ui_thread", False) is True # Reports metrics and creates spans for detection From b588538f4535a153e25422cba9e7614788982129 Mon Sep 17 00:00:00 2001 From: William Mak Date: Wed, 23 Nov 2022 13:50:19 -0500 Subject: [PATCH 5/7] ref: Address PR comments - Remove cause_span_ids - Check for op prefix is file - Include module in the call stack hashing --- .../performance_issues/performance_detection.py | 12 +++++++++--- .../performance_issues/test_performance_detection.py | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 0ef555b9fe0c6d..5a32060c937bca 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -80,7 +80,9 @@ class PerformanceProblem: desc: str type: GroupType parent_span_ids: Optional[Sequence[str]] + # For related spans that caused the bad spans cause_span_ids: Optional[Sequence[str]] + # The actual bad spans offender_span_ids: Sequence[str] def to_dict( @@ -1238,7 +1240,10 @@ def visit_span(self, span: Span): op, span_id, op_prefix, span_duration, settings = settings_for_span if span_duration.total_seconds() * 1000 > settings["duration_threshold"]: call_stack = ".".join( - [item["function"] for item in span.get("data", {}).get("call_stack", [])] + [ + f"{item.get('module', '')}.{item.get('function', '')}" + for item in span.get("data", {}).get("call_stack", []) + ] ).encode("utf8") hashed_stack = hashlib.sha1(call_stack).hexdigest() fingerprint = f"1-{GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD}-{hashed_stack}" @@ -1248,16 +1253,17 @@ def visit_span(self, span: Span): desc=span.get("description", ""), parent_span_ids=[], type=GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD, - cause_span_ids=[span.get("span_id", None)], + cause_span_ids=[], offender_span_ids=[span.get("span_id", None)], ) def _is_file_io_on_main_thread(self, span: Span) -> bool: data = span.get("data", {}) + op = span.get("op", "") if data is None: return False # doing is True since the value can be any type - return data.get("blocked_ui_thread", False) is True + return op.startswith("file") and data.get("blocked_ui_thread", False) is True # Reports metrics and creates spans for detection diff --git a/tests/sentry/utils/performance_issues/test_performance_detection.py b/tests/sentry/utils/performance_issues/test_performance_detection.py index 022aa5261bb6a8..5f0e6abc7c1645 100644 --- a/tests/sentry/utils/performance_issues/test_performance_detection.py +++ b/tests/sentry/utils/performance_issues/test_performance_detection.py @@ -997,7 +997,7 @@ def test_detects_file_io_main_thread(self): call("_pi_transaction", "c119e45a9d724b1891df4651ebf9e6db"), call( "_pi_file_io_main_thread_fp", - "1-GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD-2fdacf08551ceadba1cb7cf4eace5e288be4f9cf", + "1-GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD-153198dd61706844cf3d9a922f6f82543df8125f", ), call("_pi_file_io_main_thread", "054ba3a374d543eb"), ] From 7954b7fb5448c1b17bebe2a625e014fbff0bf906 Mon Sep 17 00:00:00 2001 From: William Mak Date: Wed, 23 Nov 2022 15:38:14 -0500 Subject: [PATCH 6/7] ref: remove the op check --- src/sentry/utils/performance_issues/performance_detection.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 5a32060c937bca..74e6a4ea205f42 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -1259,11 +1259,10 @@ def visit_span(self, span: Span): def _is_file_io_on_main_thread(self, span: Span) -> bool: data = span.get("data", {}) - op = span.get("op", "") if data is None: return False # doing is True since the value can be any type - return op.startswith("file") and data.get("blocked_ui_thread", False) is True + return data.get("blocked_ui_thread", False) is True # Reports metrics and creates spans for detection From bdfae8dbf59c534099e4390728aef111b7561f4e Mon Sep 17 00:00:00 2001 From: William Mak Date: Thu, 24 Nov 2022 11:54:51 -0500 Subject: [PATCH 7/7] address pr commetns --- .../file-io-on-main-thread.json | 296 +++++++++++++++++- .../performance_detection.py | 19 +- .../test_performance_detection.py | 9 + 3 files changed, 315 insertions(+), 9 deletions(-) diff --git a/fixtures/events/performance_problems/file-io-on-main-thread.json b/fixtures/events/performance_problems/file-io-on-main-thread.json index 88380d9df2933a..e313d471424170 100644 --- a/fixtures/events/performance_problems/file-io-on-main-thread.json +++ b/fixtures/events/performance_problems/file-io-on-main-thread.json @@ -1 +1,295 @@ -{"event_id":"c119e45a9d724b1891df4651ebf9e6db","project":5428559,"release":"io.sentry.samples.android@1.1.0+2","dist":"2","platform":"java","message":"","datetime":"2022-11-21T11:57:38.806589+00:00","tags":[["device","Android SDK built for x86"],["device.family","Android"],["environment","debug"],["isSideLoaded","true"],["level","info"],["os","Android 10"],["os.name","Android"],["os.rooted","no"],["dist","2"],["release","io.sentry.samples.android@1.1.0+2"],["user","id:0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e"],["transaction","MainActivity.add_attachment"]],"_metrics":{"bytes.ingested.event":3725,"bytes.stored.event":4851},"breadcrumbs":{"values":[{"timestamp":1669031851.824,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"created"}},{"timestamp":1669031851.942,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"started"}},{"timestamp":1669031851.943,"type":"session","category":"app.lifecycle","level":"info","data":{"state":"start"}},{"timestamp":1669031851.95,"type":"navigation","category":"app.lifecycle","level":"info","data":{"state":"foreground"}},{"timestamp":1669031851.953,"type":"navigation","category":"ui.lifecycle","level":"info","data":{"screen":"MainActivity","state":"resumed"}},{"timestamp":1669031858.676,"type":"user","category":"ui.click","level":"info","data":{"view.class":"androidx.appcompat.widget.AppCompatButton","view.id":"add_attachment"}}]},"breakdowns":{"span_ops":{"total.time":{"value":94.411134,"unit":"millisecond"}}},"contexts":{"app":{"app_start_time":"2022-11-21T11:57:31.523Z","app_identifier":"io.sentry.samples.android","app_name":"Sentry sample","app_version":"1.1.0","app_build":"2","permissions":{"ACCESS_NETWORK_STATE":"granted","CAMERA":"not_granted","FOREGROUND_SERVICE":"granted","INTERNET":"granted","READ_EXTERNAL_STORAGE":"not_granted","READ_PHONE_STATE":"not_granted","WRITE_EXTERNAL_STORAGE":"not_granted"},"type":"app"},"device":{"name":"Android SDK built for x86","family":"Android","model":"Android SDK built for x86","model_id":"QSR1.190920.001","orientation":"portrait","manufacturer":"Google","brand":"google","screen_density":2.625,"screen_dpi":420,"simulator":true,"boot_time":"2022-11-21T10:57:17.182Z","timezone":"Europe/Vienna","archs":["x86"],"id":"0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e","language":"en","locale":"en_US","screen_height_pixels":1794,"screen_width_pixels":1080,"type":"device"},"os":{"name":"Android","version":"10","build":"QSR1.190920.001","kernel_version":"4.14.112+","rooted":false,"type":"os"},"trace":{"trace_id":"b2a33f3f79fe4a7c8de3426725a045cb","span_id":"b93d2be92cd64fd5","op":"ui.action.click","status":"ok","exclusive_time":35.177709,"client_sample_rate":1.0,"hash":"1ff9a18d6a6b09a8","type":"trace"}},"culprit":"MainActivity.add_attachment","environment":"debug","grouping_config":{"enhancements":"eJybzDRxY3J-bm5-npWRgaGlroGxrpHxxEkT1-Zm5usVp-aVFFXqaWlNZAQAKGsOFg","id":"newstyle:2019-10-29"},"hashes":[],"ingest_path":[{"version":"22.11.0","public_key":"XE7QiyuNlja9PZ7I9qJlwQotzecWrUIN91BAO7Q5R38"}],"key_id":"1336851","level":"info","location":"MainActivity.add_attachment","logger":"","metadata":{"location":"MainActivity.add_attachment","title":"MainActivity.add_attachment"},"nodestore_insert":1669031864.427658,"received":1669031861.899161,"sdk":{"name":"sentry.java.android.timber","version":"6.8.0","packages":[{"name":"maven:io.sentry:sentry","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-core","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-ndk","version":"6.8.0"},{"name":"maven:io.sentry:sentry-android-timber","version":"6.8.0"}]},"span_grouping_config":{"id":"default:2022-10-27"},"spans":[{"timestamp":1669031858.806411,"start_timestamp":1669031857.712,"exclusive_time":1094.411134,"description":"1669031858711_file.txt (4.0 kB)","op":"file.write","span_id":"054ba3a374d543eb","parent_span_id":"b93d2be92cd64fd5","trace_id":"b2a33f3f79fe4a7c8de3426725a045cb","status":"ok","data":{"blocked_ui_thread":true,"call_stack":[{"function":"onClick","in_app":true,"lineno":2,"module":"io.sentry.samples.android.MainActivity$$ExternalSyntheticLambda6","native":false},{"filename":"MainActivity.java","function":"lambda$onCreate$5$io-sentry-samples-android-MainActivity","in_app":true,"lineno":93,"module":"io.sentry.samples.android.MainActivity","native":false}],"file.path":"/data/user/0/io.sentry.samples.android/files/1669031858711_file.txt","file.size":4010},"hash":"8add714f71a52ef2"}],"start_timestamp":1669031858.677,"timestamp":1669031858.806589,"title":"MainActivity.add_attachment","transaction":"MainActivity.add_attachment","transaction_info":{"source":"component"},"type":"transaction","user":{"id":"0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e","ip_address":"86.56.213.87","geo":{"country_code":"AT","city":"Enns","region":"Austria"}},"version":"7"} +{ + "event_id": "c119e45a9d724b1891df4651ebf9e6db", + "project": 5428559, + "release": "io.sentry.samples.android@1.1.0+2", + "dist": "2", + "platform": "java", + "message": "", + "datetime": "2022-11-21T11:57:38.806589+00:00", + "tags": [ + [ + "device", + "Android SDK built for x86" + ], + [ + "device.family", + "Android" + ], + [ + "environment", + "debug" + ], + [ + "isSideLoaded", + "true" + ], + [ + "level", + "info" + ], + [ + "os", + "Android 10" + ], + [ + "os.name", + "Android" + ], + [ + "os.rooted", + "no" + ], + [ + "dist", + "2" + ], + [ + "release", + "io.sentry.samples.android@1.1.0+2" + ], + [ + "user", + "id:0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e" + ], + [ + "transaction", + "MainActivity.add_attachment" + ] + ], + "_metrics": { + "bytes.ingested.event": 3725, + "bytes.stored.event": 4851 + }, + "breadcrumbs": { + "values": [ + { + "timestamp": 1669031851.824, + "type": "navigation", + "category": "ui.lifecycle", + "level": "info", + "data": { + "screen": "MainActivity", + "state": "created" + } + }, + { + "timestamp": 1669031851.942, + "type": "navigation", + "category": "ui.lifecycle", + "level": "info", + "data": { + "screen": "MainActivity", + "state": "started" + } + }, + { + "timestamp": 1669031851.943, + "type": "session", + "category": "app.lifecycle", + "level": "info", + "data": { + "state": "start" + } + }, + { + "timestamp": 1669031851.95, + "type": "navigation", + "category": "app.lifecycle", + "level": "info", + "data": { + "state": "foreground" + } + }, + { + "timestamp": 1669031851.953, + "type": "navigation", + "category": "ui.lifecycle", + "level": "info", + "data": { + "screen": "MainActivity", + "state": "resumed" + } + }, + { + "timestamp": 1669031858.676, + "type": "user", + "category": "ui.click", + "level": "info", + "data": { + "view.class": "androidx.appcompat.widget.AppCompatButton", + "view.id": "add_attachment" + } + } + ] + }, + "breakdowns": { + "span_ops": { + "total.time": { + "value": 94.411134, + "unit": "millisecond" + } + } + }, + "contexts": { + "app": { + "app_start_time": "2022-11-21T11:57:31.523Z", + "app_identifier": "io.sentry.samples.android", + "app_name": "Sentry sample", + "app_version": "1.1.0", + "app_build": "2", + "permissions": { + "ACCESS_NETWORK_STATE": "granted", + "CAMERA": "not_granted", + "FOREGROUND_SERVICE": "granted", + "INTERNET": "granted", + "READ_EXTERNAL_STORAGE": "not_granted", + "READ_PHONE_STATE": "not_granted", + "WRITE_EXTERNAL_STORAGE": "not_granted" + }, + "type": "app" + }, + "device": { + "name": "Android SDK built for x86", + "family": "Android", + "model": "Android SDK built for x86", + "model_id": "QSR1.190920.001", + "orientation": "portrait", + "manufacturer": "Google", + "brand": "google", + "screen_density": 2.625, + "screen_dpi": 420, + "simulator": true, + "boot_time": "2022-11-21T10:57:17.182Z", + "timezone": "Europe/Vienna", + "archs": [ + "x86" + ], + "id": "0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e", + "language": "en", + "locale": "en_US", + "screen_height_pixels": 1794, + "screen_width_pixels": 1080, + "type": "device" + }, + "os": { + "name": "Android", + "version": "10", + "build": "QSR1.190920.001", + "kernel_version": "4.14.112+", + "rooted": false, + "type": "os" + }, + "trace": { + "trace_id": "b2a33f3f79fe4a7c8de3426725a045cb", + "span_id": "b93d2be92cd64fd5", + "op": "ui.action.click", + "status": "ok", + "exclusive_time": 35.177709, + "client_sample_rate": 1, + "hash": "1ff9a18d6a6b09a8", + "type": "trace" + } + }, + "culprit": "MainActivity.add_attachment", + "environment": "debug", + "grouping_config": { + "enhancements": "eJybzDRxY3J-bm5-npWRgaGlroGxrpHxxEkT1-Zm5usVp-aVFFXqaWlNZAQAKGsOFg", + "id": "newstyle:2019-10-29" + }, + "hashes": [], + "ingest_path": [ + { + "version": "22.11.0", + "public_key": "XE7QiyuNlja9PZ7I9qJlwQotzecWrUIN91BAO7Q5R38" + } + ], + "key_id": "1336851", + "level": "info", + "location": "MainActivity.add_attachment", + "logger": "", + "metadata": { + "location": "MainActivity.add_attachment", + "title": "MainActivity.add_attachment" + }, + "nodestore_insert": 1669031864.427658, + "received": 1669031861.899161, + "sdk": { + "name": "sentry.java.android.timber", + "version": "6.8.0", + "packages": [ + { + "name": "maven:io.sentry:sentry", + "version": "6.8.0" + }, + { + "name": "maven:io.sentry:sentry-android-core", + "version": "6.8.0" + }, + { + "name": "maven:io.sentry:sentry-android-ndk", + "version": "6.8.0" + }, + { + "name": "maven:io.sentry:sentry-android-timber", + "version": "6.8.0" + } + ] + }, + "span_grouping_config": { + "id": "default:2022-10-27" + }, + "spans": [ + { + "timestamp": 1669031858.806411, + "start_timestamp": 1669031857.712, + "exclusive_time": 1094.411134, + "description": "1669031858711_file.txt (4.0 kB)", + "op": "file.write", + "span_id": "054ba3a374d543eb", + "parent_span_id": "b93d2be92cd64fd5", + "trace_id": "b2a33f3f79fe4a7c8de3426725a045cb", + "status": "ok", + "data": { + "blocked_ui_thread": true, + "call_stack": [ + { + "function": "onClick", + "in_app": true, + "lineno": 2, + "module": "io.sentry.samples.android.MainActivity$$ExternalSyntheticLambda6", + "native": false + }, + { + "filename": "MainActivity.java", + "function": "lambda$onCreate$5$io-sentry-samples-android-MainActivity", + "in_app": true, + "lineno": 93, + "module": "io.sentry.samples.android.MainActivity", + "native": false + } + ], + "file.path": "/data/user/0/io.sentry.samples.android/files/1669031858711_file.txt", + "file.size": 4010 + }, + "hash": "8add714f71a52ef2" + } + ], + "start_timestamp": 1669031858.677, + "timestamp": 1669031858.806589, + "title": "MainActivity.add_attachment", + "transaction": "MainActivity.add_attachment", + "transaction_info": { + "source": "component" + }, + "type": "transaction", + "user": { + "id": "0f38dda2-6ef7-4d9a-b7f4-6bcc80b85b2e", + "ip_address": "86.56.213.87", + "geo": { + "country_code": "AT", + "city": "Enns", + "region": "Austria" + } + }, + "version": "7" +} diff --git a/src/sentry/utils/performance_issues/performance_detection.py b/src/sentry/utils/performance_issues/performance_detection.py index 74e6a4ea205f42..b688f228714220 100644 --- a/src/sentry/utils/performance_issues/performance_detection.py +++ b/src/sentry/utils/performance_issues/performance_detection.py @@ -1239,14 +1239,7 @@ def visit_span(self, span: Span): op, span_id, op_prefix, span_duration, settings = settings_for_span if span_duration.total_seconds() * 1000 > settings["duration_threshold"]: - call_stack = ".".join( - [ - f"{item.get('module', '')}.{item.get('function', '')}" - for item in span.get("data", {}).get("call_stack", []) - ] - ).encode("utf8") - hashed_stack = hashlib.sha1(call_stack).hexdigest() - fingerprint = f"1-{GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD}-{hashed_stack}" + fingerprint = self._fingerprint(span) self.stored_problems[fingerprint] = PerformanceProblem( fingerprint=fingerprint, op=span.get("op"), @@ -1257,6 +1250,16 @@ def visit_span(self, span: Span): offender_span_ids=[span.get("span_id", None)], ) + def _fingerprint(self, span) -> str: + call_stack = ".".join( + [ + f"{item.get('module', '')}.{item.get('function', '')}" + for item in span.get("data", {}).get("call_stack", []) + ] + ).encode("utf8") + hashed_stack = hashlib.sha1(call_stack).hexdigest() + return f"1-{GroupType.PERFORMANCE_FILE_IO_MAIN_THREAD}-{hashed_stack}" + def _is_file_io_on_main_thread(self, span: Span) -> bool: data = span.get("data", {}) if data is None: diff --git a/tests/sentry/utils/performance_issues/test_performance_detection.py b/tests/sentry/utils/performance_issues/test_performance_detection.py index 5f0e6abc7c1645..ab64a20d38b7ae 100644 --- a/tests/sentry/utils/performance_issues/test_performance_detection.py +++ b/tests/sentry/utils/performance_issues/test_performance_detection.py @@ -1003,6 +1003,15 @@ def test_detects_file_io_main_thread(self): ] ) + def test_does_not_detect_file_io_main_thread(self): + file_io_event = EVENTS["file-io-on-main-thread"] + file_io_event["spans"][0]["data"]["blocked_ui_thread"] = False + sdk_span_mock = Mock() + + _detect_performance_problems(file_io_event, sdk_span_mock) + + assert sdk_span_mock.containing_transaction.set_tag.call_count == 0 + class PrepareProblemForGroupingTest(unittest.TestCase): def test(self):