chore(seer grouping): Stop using similarity-embeddings-grouping feature flag (#77116)

lobsterkatie · web-flow · commit cffc4ff21df1 · 2024-09-09T11:14:23.000-07:00
This removes all uses of the `projects:similarity-embeddings-grouping` feature flag, which has been superseded by the `sentry:similarity_backfill_completed` project option. The flag itself will be removed in a follow-up PR.
diff --git a/src/sentry/api/endpoints/project_details.py b/src/sentry/api/endpoints/project_details.py
@@ -954,9 +954,7 @@ def delete(self, request: Request, project) -> Response:
             project.rename_on_pending_deletion()
 
             # Tell seer to delete all the project's grouping records
-            if features.has(
-                "projects:similarity-embeddings-grouping", project
-            ) or project.get_option("sentry:similarity_backfill_completed"):
+            if project.get_option("sentry:similarity_backfill_completed"):
                 call_seer_delete_project_grouping_records.apply_async(args=[project.id])
 
         return Response(status=204)
diff --git a/src/sentry/grouping/ingest/seer.py b/src/sentry/grouping/ingest/seer.py
@@ -5,7 +5,7 @@
 import sentry_sdk
 from django.conf import settings
 
-from sentry import features, options
+from sentry import options
 from sentry import ratelimits as ratelimiter
 from sentry.conf.server import SEER_SIMILARITY_MODEL_VERSION
 from sentry.eventstore.models import Event
@@ -61,21 +61,19 @@ def should_call_seer_for_grouping(event: Event, primary_hashes: CalculatedHashes
 
 
 def _project_has_similarity_grouping_enabled(project: Project) -> bool:
-    has_seer_grouping_flag_on = features.has("projects:similarity-embeddings-grouping", project)
-
     # TODO: This is a hack to get ingest to turn on for projects as soon as they're backfilled. When
     # the backfill script completes, we turn on this option, enabling ingest immediately rather than
     # forcing the project to wait until it's been manually added to a feature handler. Once all
     # projects have been backfilled, the option (and this check) can go away.
-    has_been_backfilled = project.get_option("sentry:similarity_backfill_completed")
+    has_been_backfilled = bool(project.get_option("sentry:similarity_backfill_completed"))
 
     metrics.incr(
         "grouping.similarity.event_project_backfill_status",
         sample_rate=options.get("seer.similarity.metrics_sample_rate"),
-        tags={"backfilled": bool(has_seer_grouping_flag_on or has_been_backfilled)},
+        tags={"backfilled": has_been_backfilled},
     )
 
-    return has_seer_grouping_flag_on or has_been_backfilled
+    return has_been_backfilled
 
 
 # TODO: Here we're including events with hybrid fingerprints (ones which are `{{ default }}`
diff --git a/src/sentry/tasks/delete_seer_grouping_records.py b/src/sentry/tasks/delete_seer_grouping_records.py
@@ -1,7 +1,7 @@
 import logging
 from typing import Any
 
-from sentry import features, options
+from sentry import options
 from sentry.models.group import Group
 from sentry.models.grouphash import GroupHash
 from sentry.seer.similarity.grouping_records import (
@@ -56,10 +56,7 @@ def call_delete_seer_grouping_records_by_hash(
         project = group.project if group else None
     if (
         project
-        and (
-            features.has("projects:similarity-embeddings-grouping", project)
-            or project.get_option("sentry:similarity_backfill_completed")
-        )
+        and project.get_option("sentry:similarity_backfill_completed")
         and not killswitch_enabled(project.id)
         and not options.get("seer.similarity-embeddings-delete-by-hash-killswitch.enabled")
     ):
diff --git a/tests/sentry/api/endpoints/test_project_details.py b/tests/sentry/api/endpoints/test_project_details.py
@@ -1591,13 +1591,13 @@ def test_internal_project(self):
             model_name="Project", object_id=self.project.id
         ).exists()
 
-    @with_feature("projects:similarity-embeddings-grouping")
     @mock.patch(
         "sentry.tasks.delete_seer_grouping_records.call_seer_delete_project_grouping_records.apply_async"
     )
     def test_delete_project_and_delete_grouping_records(
         self, mock_call_seer_delete_project_grouping_records
     ):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
         self._delete_project_and_assert_deleted()
         mock_call_seer_delete_project_grouping_records.assert_called_with(args=[self.project.id])
 
diff --git a/tests/sentry/api/helpers/test_group_index.py b/tests/sentry/api/helpers/test_group_index.py
@@ -1,4 +1,5 @@
 from datetime import UTC, datetime, timedelta
+from time import time
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
@@ -1085,7 +1086,6 @@ def test_delete_groups_simple(self, send_robust: Mock):
         )
         assert send_robust.called
 
-    @with_feature("projects:similarity-embeddings-grouping")
     @patch(
         "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
     )
@@ -1094,6 +1094,8 @@ def test_delete_groups_simple(self, send_robust: Mock):
     def test_delete_groups_deletes_seer_records_by_hash(
         self, send_robust: Mock, mock_logger: Mock, mock_delete_seer_grouping_records_by_hash
     ):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         groups = [self.create_group(), self.create_group()]
         group_ids = [group.id for group in groups]
         request = self.make_request(user=self.user, method="GET")
diff --git a/tests/sentry/deletions/test_group.py b/tests/sentry/deletions/test_group.py
@@ -1,3 +1,4 @@
+from time import time
 from unittest import mock
 from uuid import uuid4
 
@@ -16,7 +17,6 @@
 from sentry.tasks.deletion.groups import delete_groups
 from sentry.testutils.cases import SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
-from sentry.testutils.helpers.features import with_feature
 
 
 class DeleteGroupTest(TestCase, SnubaTestCase):
@@ -170,13 +170,13 @@ def test_cleanup(self, nodestore_delete_multi, os_environ):
 
         assert nodestore_delete_multi.call_count == 0
 
-    @with_feature("projects:similarity-embeddings-grouping")
     @mock.patch(
         "sentry.tasks.delete_seer_grouping_records.delete_seer_grouping_records_by_hash.apply_async"
     )
     def test_delete_groups_delete_grouping_records_by_hash(
         self, mock_delete_seer_grouping_records_by_hash_apply_async
     ):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
         other_event = self.store_event(
             data={
                 "event_id": "d" * 32,
diff --git a/tests/sentry/event_manager/grouping/test_seer_grouping.py b/tests/sentry/event_manager/grouping/test_seer_grouping.py
@@ -1,4 +1,5 @@
 from dataclasses import asdict
+from time import time
 from typing import Any
 from unittest.mock import MagicMock, patch
 
@@ -7,9 +8,7 @@
 from sentry.models.grouphash import GroupHash
 from sentry.seer.similarity.types import SeerSimilarIssueData
 from sentry.testutils.cases import TestCase
-from sentry.testutils.helpers import Feature
 from sentry.testutils.helpers.eventprocessing import save_new_event
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.pytest.mocking import capture_results
 from sentry.utils.types import NonNone
 
@@ -50,45 +49,47 @@ def test_obeys_seer_similarity_flags(self):
             ),
         ):
 
-            with Feature({"projects:similarity-embeddings-grouping": False}):
-                new_event = save_new_event({"message": "Adopt don't shop"}, self.project)
-
-                # We checked whether to make the call, but didn't go through with it
-                assert should_call_seer_spy.call_count == 1
-                assert get_seer_similar_issues_spy.call_count == 0
-
-                # No metadata stored, parent group not used (even though `should_group` is True)
-                assert "seer_similarity" not in NonNone(new_event.group).data["metadata"]
-                assert "seer_similarity" not in new_event.data
-                assert new_event.group_id != existing_event.group_id
-
-                should_call_seer_spy.reset_mock()
-                get_seer_similar_issues_spy.reset_mock()
-
-            with Feature({"projects:similarity-embeddings-grouping": True}):
-                new_event = save_new_event({"message": "Maisey is silly"}, self.project)
-                expected_metadata = {
-                    "similarity_model_version": SEER_SIMILARITY_MODEL_VERSION,
-                    "results": [asdict(seer_result_data)],
-                }
-                # In real life just filtering on group id wouldn't be enough to guarantee us a
-                # single, specific GroupHash record, but since the database resets before each test,
-                # here it's okay
-                expected_grouphash = GroupHash.objects.filter(
-                    group_id=NonNone(existing_event.group_id)
-                ).first()
-
-                # We checked whether to make the call, and then made it
-                assert should_call_seer_spy.call_count == 1
-                assert get_seer_similar_issues_spy.call_count == 1
-
-                # Metadata returned and stored
-                assert get_seer_similar_issues_return_values[0][0] == expected_metadata
-                assert new_event.data["seer_similarity"] == expected_metadata
-
-                # Parent grouphash returned and parent group used
-                assert get_seer_similar_issues_return_values[0][1] == expected_grouphash
-                assert new_event.group_id == existing_event.group_id
+            # Project option not set
+            self.project.update_option("sentry:similarity_backfill_completed", None)
+            new_event = save_new_event({"message": "Adopt don't shop"}, self.project)
+
+            # We checked whether to make the call, but didn't go through with it
+            assert should_call_seer_spy.call_count == 1
+            assert get_seer_similar_issues_spy.call_count == 0
+
+            # No metadata stored, parent group not used (even though `should_group` is True)
+            assert "seer_similarity" not in NonNone(new_event.group).data["metadata"]
+            assert "seer_similarity" not in new_event.data
+            assert new_event.group_id != existing_event.group_id
+
+            should_call_seer_spy.reset_mock()
+            get_seer_similar_issues_spy.reset_mock()
+
+            # Project option set
+            self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+            new_event = save_new_event({"message": "Maisey is silly"}, self.project)
+            expected_metadata = {
+                "similarity_model_version": SEER_SIMILARITY_MODEL_VERSION,
+                "results": [asdict(seer_result_data)],
+            }
+            # In real life just filtering on group id wouldn't be enough to guarantee us a
+            # single, specific GroupHash record, but since the database resets before each test,
+            # here it's okay
+            expected_grouphash = GroupHash.objects.filter(
+                group_id=NonNone(existing_event.group_id)
+            ).first()
+
+            # We checked whether to make the call, and then made it
+            assert should_call_seer_spy.call_count == 1
+            assert get_seer_similar_issues_spy.call_count == 1
+
+            # Metadata returned and stored
+            assert get_seer_similar_issues_return_values[0][0] == expected_metadata
+            assert new_event.data["seer_similarity"] == expected_metadata
+
+            # Parent grouphash returned and parent group used
+            assert get_seer_similar_issues_return_values[0][1] == expected_grouphash
+            assert new_event.group_id == existing_event.group_id
 
     @patch("sentry.grouping.ingest.seer.should_call_seer_for_grouping", return_value=True)
     @patch("sentry.grouping.ingest.seer.get_seer_similar_issues", return_value=({}, None))
@@ -159,7 +160,6 @@ def test_stores_seer_results_in_metadata(self, _):
 
                 assert new_event.data["seer_similarity"] == expected_metadata
 
-    @with_feature("projects:similarity-embeddings-grouping")
     @patch("sentry.grouping.ingest.seer.should_call_seer_for_grouping", return_value=True)
     def test_assigns_event_to_neighbor_group_if_found(self, _):
         for use_optimized_grouping, existing_event_message, new_event_message in [
@@ -191,7 +191,6 @@ def test_assigns_event_to_neighbor_group_if_found(self, _):
 
                     mock_get_similarity_data.reset_mock()
 
-    @with_feature("projects:similarity-embeddings-grouping")
     @patch("sentry.grouping.ingest.seer.should_call_seer_for_grouping", return_value=True)
     def test_creates_new_group_if_no_neighbor_found(self, _):
         for use_optimized_grouping, existing_event_message, new_event_message in [
diff --git a/tests/sentry/grouping/ingest/test_seer.py b/tests/sentry/grouping/ingest/test_seer.py
@@ -1,4 +1,5 @@
 from dataclasses import asdict
+from time import time
 from unittest.mock import MagicMock, patch
 
 from sentry.conf.server import SEER_SIMILARITY_MODEL_VERSION
@@ -8,9 +9,7 @@
 from sentry.models.grouphash import GroupHash
 from sentry.seer.similarity.types import SeerSimilarIssueData
 from sentry.testutils.cases import TestCase
-from sentry.testutils.helpers import Feature
 from sentry.testutils.helpers.eventprocessing import save_new_event
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.options import override_options
 from sentry.utils.types import NonNone
 
@@ -47,23 +46,17 @@ def setUp(self):
         self.primary_hashes = self.event.get_hashes()
 
     def test_obeys_feature_enablement_check(self):
-        for grouping_flag, backfill_completed_option, expected_result in [
-            (False, None, False),
-            (True, None, True),
-            (False, 11211231, True),
-            (True, 11211231, True),
-        ]:
-            with Feature({"projects:similarity-embeddings-grouping": grouping_flag}):
-                self.project.update_option(
-                    "sentry:similarity_backfill_completed", backfill_completed_option
-                )
-                assert (
-                    should_call_seer_for_grouping(self.event, self.primary_hashes)
-                    is expected_result
-                ), f"Case (grouping {grouping_flag}, backfill completed {backfill_completed_option}) failed."
+        for backfill_completed_option, expected_result in [(None, False), (11211231, True)]:
+            self.project.update_option(
+                "sentry:similarity_backfill_completed", backfill_completed_option
+            )
+            assert (
+                should_call_seer_for_grouping(self.event, self.primary_hashes) is expected_result
+            ), f"Case {backfill_completed_option} failed."
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_content_filter(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for content_eligibility, expected_result in [(True, True), (False, False)]:
             with patch(
                 "sentry.grouping.ingest.seer.event_content_is_seer_eligible",
@@ -74,26 +67,29 @@ def test_obeys_content_filter(self):
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_global_seer_killswitch(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for killswitch_enabled, expected_result in [(True, False), (False, True)]:
             with override_options({"seer.global-killswitch.enabled": killswitch_enabled}):
                 assert (
                     should_call_seer_for_grouping(self.event, self.primary_hashes)
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_similarity_service_killswitch(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for killswitch_enabled, expected_result in [(True, False), (False, True)]:
             with override_options({"seer.similarity-killswitch.enabled": killswitch_enabled}):
                 assert (
                     should_call_seer_for_grouping(self.event, self.primary_hashes)
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_project_specific_killswitch(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for blocked_projects, expected_result in [([self.project.id], False), ([], True)]:
             with override_options(
                 {"seer.similarity.grouping_killswitch_projects": blocked_projects}
@@ -103,8 +99,9 @@ def test_obeys_project_specific_killswitch(self):
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_global_ratelimit(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for ratelimit_enabled, expected_result in [(True, False), (False, True)]:
             with patch(
                 "sentry.grouping.ingest.seer.ratelimiter.backend.is_limited",
@@ -117,8 +114,9 @@ def test_obeys_global_ratelimit(self):
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_project_ratelimit(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for ratelimit_enabled, expected_result in [(True, False), (False, True)]:
             with patch(
                 "sentry.grouping.ingest.seer.ratelimiter.backend.is_limited",
@@ -133,8 +131,9 @@ def test_obeys_project_ratelimit(self):
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_circuit_breaker(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         for request_allowed, expected_result in [(True, True), (False, False)]:
             with patch(
                 "sentry.grouping.ingest.seer.CircuitBreaker.should_allow_request",
@@ -145,8 +144,9 @@ def test_obeys_circuit_breaker(self):
                     is expected_result
                 )
 
-    @with_feature("projects:similarity-embeddings-grouping")
     def test_obeys_customized_fingerprint_check(self):
+        self.project.update_option("sentry:similarity_backfill_completed", int(time()))
+
         default_fingerprint_event = Event(
             project_id=self.project.id,
             event_id="11212012123120120415201309082013",
diff --git a/tests/sentry/tasks/test_delete_seer_grouping_records.py b/tests/sentry/tasks/test_delete_seer_grouping_records.py
