From 90ca8408b8ef561a05a6d86385b3aa2bdde069f9 Mon Sep 17 00:00:00 2001 From: Jodi Jang Date: Fri, 26 Jul 2024 14:38:43 -0700 Subject: [PATCH 1/3] ref(similarity): Catch all seer exceptions in backfill --- src/sentry/tasks/embeddings_grouping/utils.py | 8 +-- .../test_backfill_seer_grouping_records.py | 61 +++++++++++++------ 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/sentry/tasks/embeddings_grouping/utils.py b/src/sentry/tasks/embeddings_grouping/utils.py index bb0ba7591363bc..0c3d528029e71f 100644 --- a/src/sentry/tasks/embeddings_grouping/utils.py +++ b/src/sentry/tasks/embeddings_grouping/utils.py @@ -352,12 +352,12 @@ def _make_seer_call( create_grouping_records_request, retries=3, delay=2, - exceptions=ServiceUnavailable, + exceptions=Exception, ) - except ServiceUnavailable: + except Exception as e: logger.exception( - "tasks.backfill_seer_grouping_records.seer_service_unavailable", - extra={"project_id": project_id}, + "tasks.backfill_seer_grouping_records.seer_exception_after_retries", + extra={"project_id": project_id, "error": e.message}, ) raise diff --git a/tests/sentry/tasks/test_backfill_seer_grouping_records.py b/tests/sentry/tasks/test_backfill_seer_grouping_records.py index 331aa89c2d7fed..b543e868e85695 100644 --- a/tests/sentry/tasks/test_backfill_seer_grouping_records.py +++ b/tests/sentry/tasks/test_backfill_seer_grouping_records.py @@ -4,7 +4,7 @@ from random import choice from string import ascii_uppercase from typing import Any -from unittest.mock import call, patch +from unittest.mock import ANY, call, patch import pytest from django.test import override_settings @@ -240,15 +240,15 @@ def test_lookup_group_data_stacktrace_bulk_exceptions( mock_get_multi.side_effect = exception with pytest.raises(Exception): lookup_group_data_stacktrace_bulk(self.project, rows) - mock_logger.exception.assert_called_with( - "tasks.backfill_seer_grouping_records.bulk_event_lookup_exception", - extra={ - "organization_id": self.project.organization.id, - "project_id": self.project.id, - "group_data": json.dumps(rows), - "error": exception.message, - }, - ) + mock_logger.exception.assert_called_with( + "tasks.backfill_seer_grouping_records.bulk_event_lookup_exception", + extra={ + "organization_id": self.project.organization.id, + "project_id": self.project.id, + "node_keys": ANY, + "error": exception.message, + }, + ) def test_lookup_group_data_stacktrace_bulk_not_stacktrace_grouping(self): """ @@ -445,7 +445,7 @@ def test_get_data_from_snuba(self): assert group_id in group_ids_results @patch("sentry.tasks.embeddings_grouping.utils.logger") - @patch("sentry.utils.snuba.bulk_snuba_queries") + @patch("sentry.tasks.embeddings_grouping.utils.bulk_snuba_queries") def test_get_data_from_snuba_exception(self, mock_bulk_snuba_queries, mock_logger): mock_bulk_snuba_queries.side_effect = RateLimitExceeded @@ -454,14 +454,14 @@ def test_get_data_from_snuba_exception(self, mock_bulk_snuba_queries, mock_logge } with pytest.raises(Exception): get_data_from_snuba(self.project, group_ids_last_seen) - mock_logger.exception.assert_called_with( - "tasks.backfill_seer_grouping_records.snuba_query_exception", - extra={ - "organization_id": self.project.organization.id, - "project_id": self.project.id, - "error": "Snuba Rate Limit Exceeded", - }, - ) + mock_logger.exception.assert_called_with( + "tasks.backfill_seer_grouping_records.snuba_query_exception", + extra={ + "organization_id": self.project.organization.id, + "project_id": self.project.id, + "error": "Snuba Rate Limit Exceeded", + }, + ) @with_feature("projects:similarity-embeddings-backfill") @patch("sentry.tasks.embeddings_grouping.utils.post_bulk_grouping_records") @@ -1540,3 +1540,26 @@ def test_backfill_seer_grouping_records_no_enable_ingestion( } assert self.project.get_option("sentry:similarity_backfill_completed") is None + + @with_feature("projects:similarity-embeddings-backfill") + @patch("time.sleep", return_value=None) + @patch("sentry.tasks.embeddings_grouping.utils.logger") + @patch("sentry.tasks.embeddings_grouping.utils.post_bulk_grouping_records") + def test_backfill_seer_grouping_records_seer_exception( + self, mock_post_bulk_grouping_records, mock_logger, mock_sleep + ): + """ + Test log after seer exception and retries. + """ + exception = ServiceUnavailable(message="Service Unavailable") + mock_post_bulk_grouping_records.side_effect = exception + with pytest.raises(Exception), TaskRunner(): + backfill_seer_grouping_records_for_project(self.project.id, None) + + mock_logger.exception.assert_called_with( + "tasks.backfill_seer_grouping_records.seer_exception_after_retries", + extra={ + "project_id": self.project.id, + "error": exception.message, + }, + ) From 4c65286c8243b976c6373b3f454e91b3b5fb162a Mon Sep 17 00:00:00 2001 From: Jodi Jang Date: Fri, 26 Jul 2024 15:04:04 -0700 Subject: [PATCH 2/3] fix: Typing --- src/sentry/tasks/embeddings_grouping/utils.py | 2 +- tests/sentry/tasks/test_backfill_seer_grouping_records.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/tasks/embeddings_grouping/utils.py b/src/sentry/tasks/embeddings_grouping/utils.py index 0c3d528029e71f..7556ffd07c445f 100644 --- a/src/sentry/tasks/embeddings_grouping/utils.py +++ b/src/sentry/tasks/embeddings_grouping/utils.py @@ -357,7 +357,7 @@ def _make_seer_call( except Exception as e: logger.exception( "tasks.backfill_seer_grouping_records.seer_exception_after_retries", - extra={"project_id": project_id, "error": e.message}, + extra={"project_id": project_id, "error": e}, ) raise diff --git a/tests/sentry/tasks/test_backfill_seer_grouping_records.py b/tests/sentry/tasks/test_backfill_seer_grouping_records.py index b543e868e85695..21106facc24773 100644 --- a/tests/sentry/tasks/test_backfill_seer_grouping_records.py +++ b/tests/sentry/tasks/test_backfill_seer_grouping_records.py @@ -1560,6 +1560,6 @@ def test_backfill_seer_grouping_records_seer_exception( "tasks.backfill_seer_grouping_records.seer_exception_after_retries", extra={ "project_id": self.project.id, - "error": exception.message, + "error": exception, }, ) From 377b2c732b6ffa4a081b8ef93b4b430b3b13853c Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Mon, 29 Jul 2024 16:44:40 +0000 Subject: [PATCH 3/3] :hammer_and_wrench: apply pre-commit fixes --- tests/sentry/tasks/test_backfill_seer_grouping_records.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/tasks/test_backfill_seer_grouping_records.py b/tests/sentry/tasks/test_backfill_seer_grouping_records.py index 03fa1c9b068553..d25618f635ffc7 100644 --- a/tests/sentry/tasks/test_backfill_seer_grouping_records.py +++ b/tests/sentry/tasks/test_backfill_seer_grouping_records.py @@ -1563,7 +1563,7 @@ def test_backfill_seer_grouping_records_seer_exception( "error": exception, }, ) - + @with_feature("projects:similarity-embeddings-backfill") @patch("sentry.tasks.embeddings_grouping.backfill_seer_grouping_records_for_project.logger") def test_backfill_seer_grouping_records_skip_project_already_processed(self, mock_logger):