From 9a12aae42d8c5d25b5a6c3e54ef1df66abd74458 Mon Sep 17 00:00:00 2001 From: Yuval Mandelboum Date: Mon, 10 Nov 2025 10:24:16 -0800 Subject: [PATCH 1/5] test(deletions): add issue platform group deletion test back --- tests/sentry/deletions/test_group.py | 65 +++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py index 97b164bd5b992b..7fb5866679b2c5 100644 --- a/tests/sentry/deletions/test_group.py +++ b/tests/sentry/deletions/test_group.py @@ -11,7 +11,7 @@ from sentry import deletions, nodestore from sentry.deletions.defaults.group import update_group_hash_metadata_in_batches from sentry.deletions.tasks.groups import delete_groups_for_project -from sentry.issues.grouptype import GroupCategory +from sentry.issues.grouptype import FeedbackGroup, GroupCategory from sentry.issues.issue_occurrence import IssueOccurrence from sentry.models.activity import Activity from sentry.models.eventattachment import EventAttachment @@ -441,6 +441,61 @@ def select_rows( def tenant_ids(self) -> dict[str, str]: return {"referrer": self.referrer, "organization_id": self.organization.id} + def test_simple_issue_platform(self) -> None: + # Adding this query here to make sure that the cache is not being used + assert self.select_error_events(self.project.id) is None + assert self.select_issue_platform_events(self.project.id) is None + + # Create initial error event and occurrence related to it; two different groups will exist + event = self.store_event(data={}, project_id=self.project.id) + # XXX: We need a different way of creating occurrences which will insert into the nodestore + occurrence_event, issue_platform_group = self.create_occurrence( + event, type_id=FeedbackGroup.type_id + ) + + # Assertions after creation + assert occurrence_event.id != event.event_id + assert event.group_id != issue_platform_group.id + assert event.group.issue_category == GroupCategory.ERROR + assert issue_platform_group.issue_category == GroupCategory.FEEDBACK + assert issue_platform_group.type == FeedbackGroup.type_id + + # Assert that the error event has been inserted in the nodestore & Snuba + event_node_id = Event.generate_node_id(event.project_id, event.event_id) + assert nodestore.backend.get(event_node_id) + expected_error = {"event_id": event.event_id, "group_id": event.group_id} + assert self.select_error_events(self.project.id) == expected_error + + # Assert that the occurrence event has been inserted in the nodestore & Snuba + # occurrence_node_id = Event.generate_node_id( + # occurrence_event.project_id, occurrence_event.id + # ) + # assert nodestore.backend.get(occurrence_node_id) + expected_occurrence_event = { + "event_id": occurrence_event.event_id, + "group_id": issue_platform_group.id, + "occurrence_id": occurrence_event.id, + } + assert self.select_issue_platform_events(self.project.id) == expected_occurrence_event + + # This will delete the group and the events from the node store and Snuba + with self.tasks(): + delete_groups_for_project( + object_ids=[issue_platform_group.id], + transaction_id=uuid4().hex, + project_id=self.project.id, + ) + + # The original error event and group still exist + assert Group.objects.filter(id=event.group_id).exists() + assert nodestore.backend.get(event_node_id) + assert self.select_error_events(self.project.id) == expected_error + + # The Issue Platform group and occurrence have been deleted + assert not Group.objects.filter(id=issue_platform_group.id).exists() + # assert not nodestore.backend.get(occurrence_node_id) + assert self.select_issue_platform_events(self.project.id) is None + @mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries") def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> None: # Patch max_rows_to_delete to a small value for testing @@ -455,10 +510,10 @@ def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> No group4 = self.create_group(project=self.project) # Set times_seen for each group - Group.objects.filter(id=group1.id).update(times_seen=3, type=GroupCategory.FEEDBACK) - Group.objects.filter(id=group2.id).update(times_seen=1, type=GroupCategory.FEEDBACK) - Group.objects.filter(id=group3.id).update(times_seen=3, type=GroupCategory.FEEDBACK) - Group.objects.filter(id=group4.id).update(times_seen=3, type=GroupCategory.FEEDBACK) + Group.objects.filter(id=group1.id).update(times_seen=3, type=FeedbackGroup.type_id) + Group.objects.filter(id=group2.id).update(times_seen=1, type=FeedbackGroup.type_id) + Group.objects.filter(id=group3.id).update(times_seen=3, type=FeedbackGroup.type_id) + Group.objects.filter(id=group4.id).update(times_seen=3, type=FeedbackGroup.type_id) # This will delete the group and the events from the node store and Snuba with self.tasks(): From f4393ae2a0a4429177857a6e04fa923f96a8cfd8 Mon Sep 17 00:00:00 2001 From: Yuval Mandelboum Date: Mon, 10 Nov 2025 11:03:25 -0800 Subject: [PATCH 2/5] refacotr snuba assertion --- tests/sentry/deletions/test_group.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py index 7fb5866679b2c5..03de8299ddf914 100644 --- a/tests/sentry/deletions/test_group.py +++ b/tests/sentry/deletions/test_group.py @@ -23,6 +23,7 @@ from sentry.models.groupmeta import GroupMeta from sentry.models.groupredirect import GroupRedirect from sentry.models.userreport import UserReport +from sentry.services import eventstore from sentry.services.eventstore.models import Event from sentry.snuba.dataset import Dataset, EntityKey from sentry.snuba.referrer import Referrer @@ -494,7 +495,14 @@ def test_simple_issue_platform(self) -> None: # The Issue Platform group and occurrence have been deleted assert not Group.objects.filter(id=issue_platform_group.id).exists() # assert not nodestore.backend.get(occurrence_node_id) - assert self.select_issue_platform_events(self.project.id) is None + conditions = eventstore.Filter( + project_ids=[self.project.id], group_ids=[issue_platform_group.id] + ) + tenant_ids = {"organization_id": self.organization.id, "referrer": "test_deletions"} + events = eventstore.backend.get_events( + conditions, dataset=Dataset.IssuePlatform, tenant_ids=tenant_ids + ) + assert len(events) == 0 @mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries") def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> None: From a2f68a085f96074d5ef6c15016c8d1108f642c5c Mon Sep 17 00:00:00 2001 From: Yuval Mandelboum Date: Mon, 10 Nov 2025 12:01:28 -0800 Subject: [PATCH 3/5] poll to wait for event to be deleted --- tests/sentry/deletions/test_group.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py index 03de8299ddf914..af0f006c3152a5 100644 --- a/tests/sentry/deletions/test_group.py +++ b/tests/sentry/deletions/test_group.py @@ -1,7 +1,7 @@ import os import random from datetime import datetime, timedelta -from time import time +from time import sleep, time from typing import Any from unittest import mock from uuid import uuid4 @@ -23,7 +23,6 @@ from sentry.models.groupmeta import GroupMeta from sentry.models.groupredirect import GroupRedirect from sentry.models.userreport import UserReport -from sentry.services import eventstore from sentry.services.eventstore.models import Event from sentry.snuba.dataset import Dataset, EntityKey from sentry.snuba.referrer import Referrer @@ -479,6 +478,8 @@ def test_simple_issue_platform(self) -> None: } assert self.select_issue_platform_events(self.project.id) == expected_occurrence_event + sleep(2.0) + # This will delete the group and the events from the node store and Snuba with self.tasks(): delete_groups_for_project( @@ -492,17 +493,22 @@ def test_simple_issue_platform(self) -> None: assert nodestore.backend.get(event_node_id) assert self.select_error_events(self.project.id) == expected_error - # The Issue Platform group and occurrence have been deleted + # The Issue Platform group and occurrence have been deleted from Postgres and Snuba assert not Group.objects.filter(id=issue_platform_group.id).exists() # assert not nodestore.backend.get(occurrence_node_id) - conditions = eventstore.Filter( - project_ids=[self.project.id], group_ids=[issue_platform_group.id] - ) - tenant_ids = {"organization_id": self.organization.id, "referrer": "test_deletions"} - events = eventstore.backend.get_events( - conditions, dataset=Dataset.IssuePlatform, tenant_ids=tenant_ids - ) - assert len(events) == 0 + + # Poll to ensure the event is actually deleted from Snuba + max_attempts = 100 + event_deleted = False + for attempt in range(max_attempts): + result = self.select_issue_platform_events(self.project.id) + if result is None: + event_deleted = True + break + if attempt < max_attempts - 1: + sleep(0.1) # Wait 100ms between attempts + + assert event_deleted, f"Event still exists in Snuba after deletion: {result}" @mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries") def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> None: From 451e74f26ae04114b08c03df6d75ae71d14f5de5 Mon Sep 17 00:00:00 2001 From: Yuval Mandelboum Date: Mon, 10 Nov 2025 14:17:37 -0800 Subject: [PATCH 4/5] assert on the call to snuba rather than actual deletion in snuba to get around CI issues --- tests/sentry/deletions/test_group.py | 36 +++++++++++----------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py index af0f006c3152a5..edd08b0983ed29 100644 --- a/tests/sentry/deletions/test_group.py +++ b/tests/sentry/deletions/test_group.py @@ -1,12 +1,12 @@ import os import random from datetime import datetime, timedelta -from time import sleep, time +from time import time from typing import Any from unittest import mock from uuid import uuid4 -from snuba_sdk import Column, Condition, Entity, Function, Op, Query, Request +from snuba_sdk import Column, Condition, DeleteQuery, Entity, Function, Op, Query, Request from sentry import deletions, nodestore from sentry.deletions.defaults.group import update_group_hash_metadata_in_batches @@ -441,7 +441,8 @@ def select_rows( def tenant_ids(self) -> dict[str, str]: return {"referrer": self.referrer, "organization_id": self.organization.id} - def test_simple_issue_platform(self) -> None: + @mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries") + def test_simple_issue_platform(self, mock_bulk_snuba_queries: mock.Mock) -> None: # Adding this query here to make sure that the cache is not being used assert self.select_error_events(self.project.id) is None assert self.select_issue_platform_events(self.project.id) is None @@ -467,10 +468,6 @@ def test_simple_issue_platform(self) -> None: assert self.select_error_events(self.project.id) == expected_error # Assert that the occurrence event has been inserted in the nodestore & Snuba - # occurrence_node_id = Event.generate_node_id( - # occurrence_event.project_id, occurrence_event.id - # ) - # assert nodestore.backend.get(occurrence_node_id) expected_occurrence_event = { "event_id": occurrence_event.event_id, "group_id": issue_platform_group.id, @@ -478,8 +475,6 @@ def test_simple_issue_platform(self) -> None: } assert self.select_issue_platform_events(self.project.id) == expected_occurrence_event - sleep(2.0) - # This will delete the group and the events from the node store and Snuba with self.tasks(): delete_groups_for_project( @@ -493,22 +488,19 @@ def test_simple_issue_platform(self) -> None: assert nodestore.backend.get(event_node_id) assert self.select_error_events(self.project.id) == expected_error - # The Issue Platform group and occurrence have been deleted from Postgres and Snuba + # The Issue Platform group and occurrence have been deleted from Postgres assert not Group.objects.filter(id=issue_platform_group.id).exists() # assert not nodestore.backend.get(occurrence_node_id) - # Poll to ensure the event is actually deleted from Snuba - max_attempts = 100 - event_deleted = False - for attempt in range(max_attempts): - result = self.select_issue_platform_events(self.project.id) - if result is None: - event_deleted = True - break - if attempt < max_attempts - 1: - sleep(0.1) # Wait 100ms between attempts - - assert event_deleted, f"Event still exists in Snuba after deletion: {result}" + # Verify that a DELETE query was sent to Snuba with the correct conditions + mock_bulk_snuba_queries.assert_called_once() + requests = mock_bulk_snuba_queries.call_args[0][0] + assert len(requests) == 1 + delete_request = requests[0] + assert isinstance(delete_request.query, DeleteQuery) + assert delete_request.dataset == "search_issues" + assert delete_request.query.column_conditions["project_id"] == [self.project.id] + assert delete_request.query.column_conditions["group_id"] == [issue_platform_group.id] @mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries") def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> None: From 0bdf4b2bc5a2054c3c2b37cd362f33e9cb75fabc Mon Sep 17 00:00:00 2001 From: Yuval Mandelboum Date: Mon, 10 Nov 2025 16:00:17 -0800 Subject: [PATCH 5/5] correct nested self.tasks --- tests/sentry/deletions/test_group.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py index edd08b0983ed29..530fcacf0ae0f7 100644 --- a/tests/sentry/deletions/test_group.py +++ b/tests/sentry/deletions/test_group.py @@ -522,12 +522,11 @@ def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> No Group.objects.filter(id=group4.id).update(times_seen=3, type=FeedbackGroup.type_id) # This will delete the group and the events from the node store and Snuba - with self.tasks(): - delete_groups_for_project( - object_ids=[group1.id, group2.id, group3.id, group4.id], - transaction_id=uuid4().hex, - project_id=self.project.id, - ) + delete_groups_for_project( + object_ids=[group1.id, group2.id, group3.id, group4.id], + transaction_id=uuid4().hex, + project_id=self.project.id, + ) assert mock_bulk_snuba_queries.call_count == 1 # There should be two batches with max_rows_to_delete=6