Browse Source

fix(sort): special aggregation for sorted issue platform issues (#50129)

Workaround to 'support' betterPriority sort for issue platform backed
issues.

This basically sets Performance and Profiling issue scores to zero so
they end up at the very last in issue search sort order.

Resolves SENTRY-11SS
Gilbert Szeto 1 year ago
parent
commit
24b682004c
2 changed files with 88 additions and 2 deletions
  1. 20 1
      src/sentry/search/snuba/executors.py
  2. 68 1
      tests/snuba/search/test_backend.py

+ 20 - 1
src/sentry/search/snuba/executors.py

@@ -224,6 +224,7 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
         end: datetime,
         having: Sequence[Sequence[Any]],
         aggregate_kwargs: Optional[PrioritySortWeights] = None,
+        replace_better_priority_aggregation: Optional[bool] = False,
     ) -> list[Any]:
         extra_aggregations = self.dependency_aggregations.get(sort_field, [])
         required_aggregations = set([sort_field, "total"] + extra_aggregations)
@@ -234,6 +235,9 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
         aggregations = []
         for alias in required_aggregations:
             aggregation = self.aggregation_defs[alias]
+            # TODO: remove this hack once we can properly support better_priority sort on issue platform dataset
+            if replace_better_priority_aggregation and alias == "better_priority":
+                aggregation = self.aggregation_defs["force_last"]  # type:ignore[call-overload]
             if callable(aggregation):
                 if aggregate_kwargs:
                     aggregation = aggregation(start, end, aggregate_kwargs.get(alias, {}))
@@ -285,7 +289,18 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
                 else:
                     conditions.append(converted_filter)
 
-        aggregations = self._prepare_aggregations(sort_field, start, end, having, aggregate_kwargs)
+        if (
+            sort_field == "better_priority"
+            and group_category is not GroupCategory.ERROR.value
+            and features.has("organizations:issue-list-better-priority-sort", organization)
+        ):
+            aggregations = self._prepare_aggregations(
+                sort_field, start, end, having, aggregate_kwargs, True
+            )
+        else:
+            aggregations = self._prepare_aggregations(
+                sort_field, start, end, having, aggregate_kwargs
+            )
 
         if cursor is not None:
             having.append((sort_field, ">=" if cursor.is_prev else "<=", cursor.value))
@@ -624,6 +639,10 @@ class PostgresSnubaQueryExecutor(AbstractQueryExecutor):
         "total": ["uniq", ISSUE_FIELD_NAME],
         "user_count": ["uniq", "tags[sentry:user]"],
         "better_priority": better_priority_aggregation,
+        "force_last": [
+            "least(min(organization_id), 0)",
+            "",
+        ],  # hack aggregation to force issue-platform sort scores to be zero
     }
 
     @property

+ 68 - 1
tests/snuba/search/test_backend.py

@@ -2068,7 +2068,7 @@ class EventsSnubaSearchTest(SharedSnubaTest):
         assert len(results) == 0
 
 
-class EventsBetterPriorityTest(SharedSnubaTest):
+class EventsBetterPriorityTest(SharedSnubaTest, OccurrenceTestMixin):
     @property
     def backend(self):
         return EventsDatasetSnubaSearchBackend()
@@ -2405,6 +2405,73 @@ class EventsBetterPriorityTest(SharedSnubaTest):
         group2_score_after = results[1][1]
         assert group1_score_after < group2_score_after
 
+    def test_better_priority_mixed_group_types(self):
+        base_datetime = (datetime.utcnow() - timedelta(hours=1)).replace(tzinfo=pytz.utc)
+
+        error_event = self.store_event(
+            data={
+                "fingerprint": ["put-me-in-group1"],
+                "event_id": "a" * 32,
+                "timestamp": iso_format(base_datetime - timedelta(hours=1)),
+                "message": "foo",
+                "stacktrace": {"frames": [{"module": "group1"}]},
+                "environment": "staging",
+                "level": "fatal",
+            },
+            project_id=self.project.id,
+        )
+        error_group = error_event.group
+
+        profile_event_id = uuid.uuid4().hex
+        _, group_info = process_event_and_issue_occurrence(
+            self.build_occurrence_data(event_id=profile_event_id),
+            {
+                "event_id": profile_event_id,
+                "project_id": self.project.id,
+                "title": "some problem",
+                "platform": "python",
+                "tags": {"my_tag": "1"},
+                "timestamp": before_now(minutes=1).isoformat(),
+                "received": before_now(minutes=1).isoformat(),
+            },
+        )
+        profile_group_1 = group_info.group
+
+        agg_kwargs = {
+            "better_priority": {
+                "log_level": 0,
+                "has_stacktrace": 0,
+                "relative_volume": 1,
+                "event_halflife_hours": 4,
+                "issue_halflife_hours": 24 * 7,
+                "v2": False,
+                "norm": False,
+            }
+        }
+        query_executor = self.backend._get_query_executor()
+        with self.feature(
+            [
+                "organizations:issue-platform",
+                ProfileFileIOGroupType.build_visible_feature_name(),
+                "organizations:issue-list-better-priority-sort",
+            ]
+        ):
+            results = query_executor.snuba_search(
+                start=None,
+                end=None,
+                project_ids=[self.project.id],
+                environment_ids=[],
+                sort_field="better_priority",
+                organization=self.organization,
+                group_ids=[profile_group_1.id, error_group.id],
+                limit=150,
+                aggregate_kwargs=agg_kwargs,
+            )[0]
+        error_group_score = results[0][1]
+        profile_group_score = results[1][1]
+        assert error_group_score > 0
+        assert profile_group_score == 0
+
 
 class EventsTransactionsSnubaSearchTest(SharedSnubaTest):
     @property