fix(recommended-event): Add subquery to limit ordering to last 1000 events (#97792)

When there is a very large number of recent events, the recommended
event query will time out. The current logic is:

- Query for events in the past 7 days
- Sort by a number of properties
- Return the top result

This PR adds an `inner_limit` option to `get_events_snql()` that will
create a initial subquery for the most recent n events before applying
the sort order on top of it. The recommended event query supplies
`inner_limit=1000` which should retain similar behavior and be much more
performant when there are a large number of events.

Author: malwilley

src/sentry/models/group.py

@@ -314,6 +314,7 @@ def get_recommended_event(
         referrer="Group.get_helpful",
         dataset=dataset,
         tenant_ids={"organization_id": group.project.organization_id},
+        inner_limit=1000,
     )
 
     if events:

src/sentry/services/eventstore/base.py

@@ -191,6 +191,7 @@ def get_events_snql(
         conditions: Sequence[Condition],
         orderby: Sequence[str],
         limit: int = 100,
+        inner_limit: int | None = None,
         offset: int = 0,
         referrer: str = "eventstore.get_events_snql",
         dataset: Dataset = Dataset.Events,

src/sentry/services/eventstore/snuba/backend.py

@@ -75,6 +75,7 @@ def get_events_snql(
         conditions: Sequence[Condition],
         orderby: Sequence[str],
         limit: int = DEFAULT_LIMIT,
+        inner_limit: int | None = None,
         offset: int = DEFAULT_OFFSET,
         referrer: str = "eventstore.get_events_snql",
         dataset: Dataset = Dataset.Events,
@@ -83,6 +84,7 @@ def get_events_snql(
         cols = self.__get_columns(dataset)
 
         resolved_order_by = []
+        order_by_col_names: set[str] = set()
         for order_field_alias in orderby:
             if order_field_alias.startswith("-"):
                 direction = Direction.DESC
@@ -91,6 +93,7 @@ def get_events_snql(
                 direction = Direction.ASC
             resolved_column_or_none = DATASETS[dataset].get(order_field_alias)
             if resolved_column_or_none:
+                order_by_col_names.add(resolved_column_or_none)
                 # special-case handling for nullable column values and proper ordering based on direction
                 # null values are always last in the sort order regardless of Desc or Asc ordering
                 if order_field_alias == Columns.NUM_PROCESSING_ERRORS.value.alias:
@@ -131,25 +134,61 @@ def get_events_snql(
             [group_id],
         )
 
-        snql_request = Request(
-            dataset=dataset.value,
-            app_id="eventstore",
-            query=Query(
-                match=Entity(dataset.value),
-                select=[Column(col) for col in cols],
-                where=[
-                    Condition(
-                        Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]), Op.GTE, start
+        match_entity = Entity(dataset.value)
+        event_filters = [
+            Condition(Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]), Op.GTE, start),
+            Condition(Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]), Op.LT, end),
+        ] + list(conditions)
+
+        common_request_kwargs = {
+            "app_id": "eventstore",
+            "dataset": dataset.value,
+            "tenant_ids": tenant_ids or dict(),
+        }
+
+        common_query_kwargs = {
+            "select": [Column(col) for col in cols],
+            "orderby": resolved_order_by,
+            "limit": Limit(limit),
+            "offset": Offset(offset),
+        }
+
+        # If inner_limit provided, first limit to the most recent N rows, then apply final ordering
+        # and pagination on top of that subquery.
+        if inner_limit and inner_limit > 0:
+            select_and_orderby_cols = set(cols) | order_by_col_names
+            inner_query = Query(
+                match=match_entity,
+                select=[Column(col) for col in select_and_orderby_cols],
+                where=event_filters,
+                orderby=[
+                    OrderBy(
+                        Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
+                        direction=Direction.DESC,
                     ),
-                    Condition(Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]), Op.LT, end),
-                ]
-                + list(conditions),
-                orderby=resolved_order_by,
-                limit=Limit(limit),
-                offset=Offset(offset),
-            ),
-            tenant_ids=tenant_ids or dict(),
-        )
+                    OrderBy(
+                        Column(DATASETS[dataset][Columns.EVENT_ID.value.alias]),
+                        direction=Direction.DESC,
+                    ),
+                ],
+                limit=Limit(inner_limit),
+            )
+
+            outer_query = Query(
+                **common_query_kwargs,
+                match=inner_query,
+            )
+
+            snql_request = Request(**common_request_kwargs, query=outer_query)
+        else:
+            snql_request = Request(
+                **common_request_kwargs,
+                query=Query(
+                    **common_query_kwargs,
+                    match=match_entity,
+                    where=event_filters,
+                ),
+            )
 
         result = raw_snql_query(snql_request, referrer, use_cache=False)
 

src/sentry/testutils/pytest/metrics.py

@@ -4,7 +4,7 @@
 from unittest import mock
 
 import pytest
-from snuba_sdk import Entity, Join
+from snuba_sdk import Entity, Join, Query
 
 from sentry.sentry_metrics.use_case_id_registry import UseCaseID
 
@@ -70,7 +70,11 @@ def new_build_results(*args, **kwargs):
                 query = args[0][0].request.query
                 is_performance_metrics = False
                 is_metrics = False
-                if not isinstance(query, MetricsQuery) and not isinstance(query.match, Join):
+                if (
+                    not isinstance(query, MetricsQuery)
+                    and not isinstance(query.match, Join)
+                    and not isinstance(query.match, Query)
+                ):
                     is_performance_metrics = query.match.name.startswith("generic")
                     is_metrics = "metrics" in query.match.name
 
@@ -93,7 +97,7 @@ def new_create_snql_in_snuba(
                 query = snql_query.query
                 is_performance_metrics = False
                 is_metrics = False
-                if isinstance(query.match, Entity):
+                if isinstance(query.match, Entity) and not isinstance(query.match, Query):
                     is_performance_metrics = query.match.name.startswith("generic")
                     is_metrics = "metrics" in query.match.name
 

tests/sentry/services/eventstore/snuba/test_backend.py

@@ -7,6 +7,7 @@
 from sentry.services.eventstore.base import Filter
 from sentry.services.eventstore.models import Event
 from sentry.services.eventstore.snuba.backend import SnubaEventStorage
+from sentry.snuba.dataset import Dataset
 from sentry.testutils.cases import PerformanceIssueTestCase, SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now
 from sentry.utils import snuba
@@ -198,6 +199,83 @@ def test_get_event_by_id_cached(self) -> None:
         assert event.project_id == self.project2.id
         assert event.group_id == event.group.id
 
+    def test_get_events_snql_with_inner_limit(self) -> None:
+        project = self.create_project()
+        ts_old = before_now(minutes=6).isoformat()
+        ts_mid = before_now(minutes=5).isoformat()
+        ts_new = before_now(minutes=4).isoformat()
+
+        event_oldest = self.store_event(
+            data={
+                "event_id": "1" * 32,
+                "type": "default",
+                "platform": "python",
+                "fingerprint": ["inner-limit-group"],
+                "timestamp": ts_old,
+            },
+            project_id=project.id,
+        )
+        event_middle = self.store_event(
+            data={
+                "event_id": "2" * 32,
+                "type": "default",
+                "platform": "python",
+                "fingerprint": ["inner-limit-group"],
+                "timestamp": ts_mid,
+            },
+            project_id=project.id,
+        )
+        event_newest = self.store_event(
+            data={
+                "event_id": "3" * 32,
+                "type": "default",
+                "platform": "python",
+                "fingerprint": ["inner-limit-group"],
+                "timestamp": ts_new,
+            },
+            project_id=project.id,
+        )
+
+        group_id = event_oldest.group_id
+        assert group_id is not None
+        eventstore = SnubaEventStorage()
+
+        # Ascending order should return oldest first when no inner limit is used
+        no_inner = eventstore.get_events_snql(
+            organization_id=project.organization.id,
+            group_id=group_id,
+            start=before_now(minutes=10),
+            end=before_now(minutes=0),
+            conditions=[
+                Condition(Column("project_id"), Op.IN, [project.id]),
+                Condition(Column("group_id"), Op.IN, [group_id]),
+            ],
+            orderby=["timestamp", "event_id"],
+            dataset=Dataset.Events,
+            tenant_ids={"organization_id": project.organization.id},
+            limit=2,
+        )
+        assert [e.event_id for e in no_inner] == [event_oldest.event_id, event_middle.event_id]
+
+        # With inner_limit=2 we first query for the two most recent events, THEN apply sorting
+        # The result of which should be [event_middle, event_newest]
+        with_inner = eventstore.get_events_snql(
+            organization_id=project.organization.id,
+            group_id=group_id,
+            start=before_now(minutes=10),
+            end=before_now(minutes=0),
+            conditions=[
+                Condition(Column("project_id"), Op.IN, [project.id]),
+                Condition(Column("group_id"), Op.IN, [group_id]),
+            ],
+            orderby=["timestamp", "event_id"],
+            dataset=Dataset.Events,
+            tenant_ids={"organization_id": project.organization.id},
+            limit=2,
+            inner_limit=2,
+        )
+        assert [e.event_id for e in with_inner] == [event_middle.event_id, event_newest.event_id]
+
     def test_get_event_beyond_retention(self) -> None:
         event = self.store_event(
             data={