From d8ad39213b89a91d3e729502e30753205ba59f04 Mon Sep 17 00:00:00 2001 From: Bartek Ogryczak Date: Fri, 3 Nov 2023 14:33:26 -0700 Subject: [PATCH 1/5] Adding SDK name as a tag to event_manager grouping metrics --- src/sentry/event_manager.py | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 46064712ec9c57..2c64e2f52c5ac8 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -451,7 +451,10 @@ def save( return jobs[0]["event"] else: - metric_tags = {"platform": job["event"].platform or "unknown"} + metric_tags = { + "platform": job["event"].platform or "unknown", + "sdk": job["event"].data.get("sdk", {}).get("name") or "unknown", + } # This metric allows differentiating from all calls to the `event_manager.save` metric # and adds support for differentiating based on platforms with metrics.timer("event_manager.save_error_events", tags=metric_tags): @@ -776,7 +779,7 @@ def _auto_update_grouping(project: Project) -> None: "sentry:secondary_grouping_expiry": expiry, "sentry:grouping_config": new_grouping, } - for (key, value) in changes.items(): + for key, value in changes.items(): project.update_option(key, value) create_system_audit_entry( organization=project.organization, @@ -1581,7 +1584,6 @@ def _save_aggregate( kwargs["data"]["last_received"] = received_timestamp if existing_grouphash is None: - if killswitch_matches_context( "store.load-shed-group-creation-projects", { @@ -1621,7 +1623,6 @@ def _save_aggregate( root_hierarchical_grouphash = None if existing_grouphash is None: - group = _create_group(project, event, **kwargs) if ( @@ -1654,7 +1655,10 @@ def _save_aggregate( metrics.incr( "group.created", skip_internal=True, - tags={"platform": event.platform or "unknown"}, + tags={ + "platform": event.platform or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", + }, ) # This only applies to events with stacktraces @@ -1665,6 +1669,7 @@ def _save_aggregate( sample_rate=1.0, tags={ "platform": event.platform or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", "frame_mix": frame_mix, }, ) @@ -1826,7 +1831,10 @@ def _create_group(project: Project, event: Event, **kwargs: Any) -> Group: except OperationalError: metrics.incr( "next_short_id.timeout", - tags={"platform": event.platform or "unknown"}, + tags={ + "platform": event.platform or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", + }, ) sentry_sdk.capture_message("short_id.timeout") raise HashDiscarded("Timeout when getting next_short_id", reason="timeout") @@ -2207,7 +2215,10 @@ def discard_event(job: Job, attachments: Sequence[Attachment]) -> None: metrics.incr( "events.discarded", skip_internal=True, - tags={"platform": job["platform"]}, + tags={ + "platform": job["platform"], + "sdk": job["event"].data.get("sdk", {}).get("name") or "unknown", + }, ) @@ -2466,6 +2477,7 @@ def _calculate_event_grouping( metric_tags: MutableTags = { "grouping_config": grouping_config["id"], "platform": event.platform or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", } with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags): @@ -2518,7 +2530,10 @@ def _calculate_span_grouping(jobs: Sequence[Job], projects: ProjectsMapping) -> metrics.incr( "save_event.transaction.span_group_count.default", amount=len(unique_default_hashes), - tags={"platform": job["platform"] or "unknown"}, + tags={ + "platform": job["platform"] or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", + }, ) except Exception: sentry_sdk.capture_exception() From 5ab4528d6a5a169d361e321abfba2aadc0ba772b Mon Sep 17 00:00:00 2001 From: Bartek Ogryczak Date: Fri, 3 Nov 2023 15:09:38 -0700 Subject: [PATCH 2/5] adding testing --- tests/sentry/event_manager/test_event_manager.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/sentry/event_manager/test_event_manager.py b/tests/sentry/event_manager/test_event_manager.py index 51b60700dc6b9b..0cc19bfe103d82 100644 --- a/tests/sentry/event_manager/test_event_manager.py +++ b/tests/sentry/event_manager/test_event_manager.py @@ -2301,7 +2301,6 @@ def test_perf_issue_creation(self): @override_options({"performance.issues.all.problem-detection": 1.0}) @override_options({"performance.issues.n_plus_one_db.problem-creation": 1.0}) def test_perf_issue_update(self): - with mock.patch("sentry_sdk.tracing.Span.containing_transaction"): event = self.create_performance_issue( event_data=make_event(**get_event("n-plus-one-in-django-index-view")) @@ -2441,7 +2440,9 @@ def attempt_to_generate_slow_db_issue() -> Event: @patch("sentry.event_manager.metrics.incr") def test_new_group_metrics_logging(self, mock_metrics_incr: MagicMock) -> None: - manager = EventManager(make_event(platform="javascript")) + manager = EventManager( + make_event(platform="javascript", sdk={"name": "sentry.javascript.foo"}) + ) manager.normalize() manager.save(self.project.id) @@ -2450,12 +2451,15 @@ def test_new_group_metrics_logging(self, mock_metrics_incr: MagicMock) -> None: skip_internal=True, tags={ "platform": "javascript", + "sdk": "sentry.javascript.foo", }, ) def test_new_group_metrics_logging_with_frame_mix(self) -> None: with patch("sentry.event_manager.metrics.incr") as mock_metrics_incr: - manager = EventManager(make_event(platform="javascript")) + manager = EventManager( + make_event(platform="javascript", sdk={"name": "sentry.javascript.foo"}) + ) manager.normalize() # IRL, `normalize_stacktraces_for_grouping` adds frame mix metadata to the event, but we # can't mock that because it's imported inside its calling function to avoid circular imports @@ -2468,6 +2472,7 @@ def test_new_group_metrics_logging_with_frame_mix(self) -> None: tags={ "platform": "javascript", "frame_mix": "in-app-only", + "sdk": "sentry.javascript.foo", }, ) From 4fe1f2b3cba0c89ef800a7cb398b626a2f0980be Mon Sep 17 00:00:00 2001 From: Bartek Ogryczak Date: Fri, 3 Nov 2023 15:37:33 -0700 Subject: [PATCH 3/5] adding test for unknown SDK --- .../sentry/event_manager/test_event_manager.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/sentry/event_manager/test_event_manager.py b/tests/sentry/event_manager/test_event_manager.py index 0cc19bfe103d82..54a683b4ef2d39 100644 --- a/tests/sentry/event_manager/test_event_manager.py +++ b/tests/sentry/event_manager/test_event_manager.py @@ -2455,6 +2455,23 @@ def test_new_group_metrics_logging(self, mock_metrics_incr: MagicMock) -> None: }, ) + @patch("sentry.event_manager.metrics.incr") + def test_new_group_metrics_logging_no_platform_no_sdk( + self, mock_metrics_incr: MagicMock + ) -> None: + manager = EventManager(make_event(platform=None, sdk=None)) + manager.normalize() + manager.save(self.project.id) + + mock_metrics_incr.assert_any_call( + "group.created", + skip_internal=True, + tags={ + "platform": "other", + "sdk": "unknown", + }, + ) + def test_new_group_metrics_logging_with_frame_mix(self) -> None: with patch("sentry.event_manager.metrics.incr") as mock_metrics_incr: manager = EventManager( From 24bd476c9d4622d40c3148b84e08c996a0cf9aca Mon Sep 17 00:00:00 2001 From: Bartek Ogryczak Date: Fri, 3 Nov 2023 17:11:05 -0700 Subject: [PATCH 4/5] not adding SDK tag to 'next_short_id.timeout' metric --- src/sentry/event_manager.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 2c64e2f52c5ac8..4142bdaca25053 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -1832,9 +1832,8 @@ def _create_group(project: Project, event: Event, **kwargs: Any) -> Group: metrics.incr( "next_short_id.timeout", tags={ - "platform": event.platform or "unknown", - "sdk": event.data.get("sdk", {}).get("name") or "unknown", - }, + "platform": event.platform or "unknown" + }, # TODO: remove this tag, it's nor relevant ) sentry_sdk.capture_message("short_id.timeout") raise HashDiscarded("Timeout when getting next_short_id", reason="timeout") From 08f9e0365b08dd701bd6b1019d5f62dadab52469 Mon Sep 17 00:00:00 2001 From: Bartek Ogryczak Date: Fri, 3 Nov 2023 17:28:28 -0700 Subject: [PATCH 5/5] more tags --- src/sentry/event_manager.py | 71 ++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 4142bdaca25053..509b8ac42b3c05 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -789,11 +789,16 @@ def _auto_update_grouping(project: Project) -> None: ) -@metrics.wraps("event_manager.background_grouping") def _calculate_background_grouping( project: Project, event: Event, config: GroupingConfig ) -> CalculatedHashes: - return _calculate_event_grouping(project, event, config) + metric_tags: MutableTags = { + "grouping_config": config["id"], + "platform": event.platform or "unknown", + "sdk": event.data.get("sdk", {}).get("name") or "unknown", + } + with metrics.timer("event_manager.background_grouping", tags=metric_tags): + return _calculate_event_grouping(project, event, config) def _run_background_grouping(project: Project, job: Job) -> None: @@ -2465,7 +2470,6 @@ def _materialize_event_metrics(jobs: Sequence[Job]) -> None: job["event_metrics"] = event_metrics -@metrics.wraps("save_event.calculate_event_grouping") def _calculate_event_grouping( project: Project, event: Event, grouping_config: GroupingConfig ) -> CalculatedHashes: @@ -2479,38 +2483,39 @@ def _calculate_event_grouping( "sdk": event.data.get("sdk", {}).get("name") or "unknown", } - with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags): - with sentry_sdk.start_span(op="event_manager.normalize_stacktraces_for_grouping"): - event.normalize_stacktraces_for_grouping(load_grouping_config(grouping_config)) - - # Detect & set synthetic marker if necessary - detect_synthetic_exception(event.data, grouping_config) - - with metrics.timer("event_manager.apply_server_fingerprinting"): - # The active grouping config was put into the event in the - # normalize step before. We now also make sure that the - # fingerprint was set to `'{{ default }}' just in case someone - # removed it from the payload. The call to get_hashes will then - # look at `grouping_config` to pick the right parameters. - event.data["fingerprint"] = event.data.data.get("fingerprint") or ["{{ default }}"] - apply_server_fingerprinting( - event.data.data, - get_fingerprinting_config_for_project(project), - allow_custom_title=True, - ) + with metrics.timer("save_event.calculate_event_grouping", tags=metric_tags): + with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags): + with sentry_sdk.start_span(op="event_manager.normalize_stacktraces_for_grouping"): + event.normalize_stacktraces_for_grouping(load_grouping_config(grouping_config)) + + # Detect & set synthetic marker if necessary + detect_synthetic_exception(event.data, grouping_config) + + with metrics.timer("event_manager.apply_server_fingerprinting", tags=metric_tags): + # The active grouping config was put into the event in the + # normalize step before. We now also make sure that the + # fingerprint was set to `'{{ default }}' just in case someone + # removed it from the payload. The call to get_hashes will then + # look at `grouping_config` to pick the right parameters. + event.data["fingerprint"] = event.data.data.get("fingerprint") or ["{{ default }}"] + apply_server_fingerprinting( + event.data.data, + get_fingerprinting_config_for_project(project), + allow_custom_title=True, + ) - with metrics.timer("event_manager.event.get_hashes", tags=metric_tags): - # Here we try to use the grouping config that was requested in the - # event. If that config has since been deleted (because it was an - # experimental grouping config) we fall back to the default. - try: - hashes = event.get_hashes(grouping_config) - except GroupingConfigNotFound: - event.data["grouping_config"] = get_grouping_config_dict_for_project(project) - hashes = event.get_hashes() + with metrics.timer("event_manager.event.get_hashes", tags=metric_tags): + # Here we try to use the grouping config that was requested in the + # event. If that config has since been deleted (because it was an + # experimental grouping config) we fall back to the default. + try: + hashes = event.get_hashes(grouping_config) + except GroupingConfigNotFound: + event.data["grouping_config"] = get_grouping_config_dict_for_project(project) + hashes = event.get_hashes() - hashes.write_to_event(event.data) - return hashes + hashes.write_to_event(event.data) + return hashes @metrics.wraps("save_event.calculate_span_grouping")