fix(alerts): Don't group on time (#89447)

Issue alerts that have "slow" conditions over a large time period (1
day, 1 week, 1 month) can hit the 10,000 limit we have for results when
there are a lot of group to process as we return timestamped data per
group (e.g. group id 1 has data for 12:01, 12:02, 12:03, and so on and
that's multiplied by the number of groups we're querying) and we'll drop
the remaining data past the limit which results in lower numbers than
we'd expect. This can cause alerts to not fire as the data we're
receiving isn't accurate.

To fix this we'll disable `group_on_time` and instead of receiving all
the timestamp data that we add up later, we'll simply receive the sum
without the timestamps.

I renamed `get_sums` to `get_timeseries_sums` and replaced the usages
that rely on the timeseries data with that. The places that use
`get_sums` now expect to receive the aggregate data in a simple
dictionary `{group_id: count}`.

Author: ceorourke

src/sentry/digests/notifications.py

@@ -2,8 +2,8 @@
 
 import logging
 from collections import defaultdict
-from collections.abc import Sequence
-from typing import NamedTuple, TypeAlias
+from collections.abc import Mapping, Sequence
+from typing import Any, NamedTuple, TypeAlias
 
 from sentry import tsdb
 from sentry.digests.types import Notification, Record, RecordWithRuleObjects
@@ -22,7 +22,7 @@
 class DigestInfo(NamedTuple):
     digest: Digest
     event_counts: dict[int, int]
-    user_counts: dict[int, int]
+    user_counts: Mapping[int, int]
 
 
 def split_key(
@@ -113,7 +113,7 @@ def _group_records(
 
 
 def _sort_digest(
-    digest: Digest, event_counts: dict[int, int], user_counts: dict[int, int]
+    digest: Digest, event_counts: dict[int, int], user_counts: Mapping[Any, int]
 ) -> Digest:
     # sort inner groups dict by (event_count, user_count) descending
     for key, rule_groups in digest.items():
@@ -142,7 +142,7 @@ def _build_digest_impl(
     groups: dict[int, Group],
     rules: dict[int, Rule],
     event_counts: dict[int, int],
-    user_counts: dict[int, int],
+    user_counts: Mapping[Any, int],
 ) -> Digest:
     # sans-io implementation details
     bound_records = _bind_records(records, groups, rules)
@@ -171,7 +171,7 @@ def build_digest(project: Project, records: Sequence[Record]) -> DigestInfo:
         assert rule.project_id == project.id, "Rule must belong to Project"
 
     tenant_ids = {"organization_id": project.organization_id}
-    event_counts = tsdb.backend.get_sums(
+    event_counts = tsdb.backend.get_timeseries_sums(
         TSDBModel.group,
         group_ids,
         start,

src/sentry/models/groupsnooze.py

@@ -118,7 +118,7 @@ def test_frequency_rates(self) -> bool:
         end = timezone.now()
         start = end - timedelta(minutes=self.window)
 
-        rate = tsdb.backend.get_sums(
+        rate = tsdb.backend.get_timeseries_sums(
             model=get_issue_tsdb_group_model(self.group.issue_category),
             keys=[self.group_id],
             start=start,

src/sentry/rules/conditions/event_frequency.py

@@ -230,7 +230,11 @@ def query(
         return self.query_hook(event, start, end, environment_id)
 
     def query_hook(
-        self, event: GroupEvent, start: datetime, end: datetime, environment_id: int
+        self,
+        event: GroupEvent,
+        start: datetime,
+        end: datetime,
+        environment_id: int,
     ) -> int | float:
         """
         Abstract method that specifies how to query Snuba for a single group
@@ -244,10 +248,15 @@ def batch_query(
         """
         Queries Snuba for a unique condition for multiple groups.
         """
-        return self.batch_query_hook(group_ids, start, end, environment_id)
+        return self.batch_query_hook(group_ids, start, end, environment_id, False)
 
     def batch_query_hook(
-        self, group_ids: set[int], start: datetime, end: datetime, environment_id: int
+        self,
+        group_ids: set[int],
+        start: datetime,
+        end: datetime,
+        environment_id: int,
+        group_on_time: bool,
     ) -> dict[int, int | float]:
         """
         Abstract method that specifies how to query Snuba for multiple groups
@@ -339,6 +348,7 @@ def get_snuba_query_result(
         end: datetime,
         environment_id: int,
         referrer_suffix: str,
+        group_on_time: bool = False,
     ) -> Mapping[int, int]:
         result: Mapping[int, int] = tsdb_function(
             model=model,
@@ -350,6 +360,7 @@ def get_snuba_query_result(
             jitter_value=group_id,
             tenant_ids={"organization_id": organization_id},
             referrer_suffix=referrer_suffix,
+            group_on_time=group_on_time,
         )
         return result
 
@@ -363,6 +374,7 @@ def get_chunked_result(
         end: datetime,
         environment_id: int,
         referrer_suffix: str,
+        group_on_time: bool = False,
     ) -> dict[int, int]:
         batch_totals: dict[int, int] = defaultdict(int)
         group_id = group_ids[0]
@@ -377,6 +389,7 @@ def get_chunked_result(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix=referrer_suffix,
+                group_on_time=group_on_time,
             )
             batch_totals.update(result)
         return batch_totals
@@ -419,7 +432,11 @@ class EventFrequencyCondition(BaseEventFrequencyCondition):
     label = "The issue is seen more than {value} times in {interval}"
 
     def query_hook(
-        self, event: GroupEvent, start: datetime, end: datetime, environment_id: int
+        self,
+        event: GroupEvent,
+        start: datetime,
+        end: datetime,
+        environment_id: int,
     ) -> int:
         sums: Mapping[int, int] = self.get_snuba_query_result(
             tsdb_function=self.tsdb.get_sums,
@@ -431,11 +448,17 @@ def query_hook(
             end=end,
             environment_id=environment_id,
             referrer_suffix="alert_event_frequency",
+            group_on_time=False,
         )
         return sums[event.group_id]
 
     def batch_query_hook(
-        self, group_ids: set[int], start: datetime, end: datetime, environment_id: int
+        self,
+        group_ids: set[int],
+        start: datetime,
+        end: datetime,
+        environment_id: int,
+        group_on_time: bool = False,
     ) -> dict[int, int | float]:
         batch_sums: dict[int, int | float] = defaultdict(int)
         groups = Group.objects.filter(id__in=group_ids).values(
@@ -454,6 +477,7 @@ def batch_query_hook(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_frequency",
+                group_on_time=group_on_time,
             )
             batch_sums.update(error_sums)
 
@@ -468,6 +492,7 @@ def batch_query_hook(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_frequency",
+                group_on_time=group_on_time,
             )
             batch_sums.update(generic_sums)
 
@@ -482,7 +507,11 @@ class EventUniqueUserFrequencyCondition(BaseEventFrequencyCondition):
     label = "The issue is seen by more than {value} users in {interval}"
 
     def query_hook(
-        self, event: GroupEvent, start: datetime, end: datetime, environment_id: int
+        self,
+        event: GroupEvent,
+        start: datetime,
+        end: datetime,
+        environment_id: int,
     ) -> int:
         totals: Mapping[int, int] = self.get_snuba_query_result(
             tsdb_function=self.tsdb.get_distinct_counts_totals,
@@ -494,11 +523,17 @@ def query_hook(
             end=end,
             environment_id=environment_id,
             referrer_suffix="alert_event_uniq_user_frequency",
+            group_on_time=False,
         )
         return totals[event.group_id]
 
     def batch_query_hook(
-        self, group_ids: set[int], start: datetime, end: datetime, environment_id: int
+        self,
+        group_ids: set[int],
+        start: datetime,
+        end: datetime,
+        environment_id: int,
+        group_on_time: bool = False,
     ) -> dict[int, int | float]:
         batch_totals: dict[int, int | float] = defaultdict(int)
         groups = Group.objects.filter(id__in=group_ids).values(
@@ -517,6 +552,7 @@ def batch_query_hook(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_uniq_user_frequency",
+                group_on_time=group_on_time,
             )
             batch_totals.update(error_totals)
 
@@ -531,6 +567,7 @@ def batch_query_hook(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_uniq_user_frequency",
+                group_on_time=group_on_time,
             )
             batch_totals.update(generic_totals)
 
@@ -545,7 +582,11 @@ class EventUniqueUserFrequencyConditionWithConditions(EventUniqueUserFrequencyCo
     label = "The issue is seen by more than {value} users in {interval} with conditions"
 
     def query_hook(
-        self, event: GroupEvent, start: datetime, end: datetime, environment_id: int
+        self,
+        event: GroupEvent,
+        start: datetime,
+        end: datetime,
+        environment_id: int,
     ) -> int:
         assert self.rule
         if not features.has(
@@ -580,11 +621,17 @@ def query_hook(
             environment_id=environment_id,
             referrer_suffix="batch_alert_event_uniq_user_frequency",
             conditions=conditions,
+            group_on_time=False,
         )
         return total[event.group.id]
 
     def batch_query_hook(
-        self, group_ids: set[int], start: datetime, end: datetime, environment_id: int
+        self,
+        group_ids: set[int],
+        start: datetime,
+        end: datetime,
+        environment_id: int,
+        group_on_time: bool = False,
     ) -> dict[int, int | float]:
         logger = logging.getLogger(
             "sentry.rules.event_frequency.EventUniqueUserFrequencyConditionWithConditions"
@@ -643,6 +690,7 @@ def batch_query_hook(
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_uniq_user_frequency",
                 conditions=conditions,
+                group_on_time=group_on_time,
             )
             batch_totals.update(error_totals)
 
@@ -657,6 +705,7 @@ def batch_query_hook(
                 environment_id=environment_id,
                 referrer_suffix="batch_alert_event_uniq_user_frequency",
                 conditions=conditions,
+                group_on_time=group_on_time,
             )
             batch_totals.update(error_totals)
 
@@ -677,6 +726,7 @@ def get_snuba_query_result(
         end: datetime,
         environment_id: int,
         referrer_suffix: str,
+        group_on_time: bool = False,
         conditions: list[tuple[str, str, str | list[str]]] | None = None,
     ) -> Mapping[int, int]:
         result: Mapping[int, int] = tsdb_function(
@@ -690,6 +740,7 @@ def get_snuba_query_result(
             tenant_ids={"organization_id": organization_id},
             referrer_suffix=referrer_suffix,
             conditions=conditions,
+            group_on_time=group_on_time,
         )
         return result
 
@@ -703,6 +754,7 @@ def get_chunked_result(
         end: datetime,
         environment_id: int,
         referrer_suffix: str,
+        group_on_time: bool = False,
         conditions: list[tuple[str, str, str | list[str]]] | None = None,
     ) -> dict[int, int]:
         batch_totals: dict[int, int] = defaultdict(int)
@@ -719,6 +771,7 @@ def get_chunked_result(
                 environment_id=environment_id,
                 referrer_suffix=referrer_suffix,
                 conditions=conditions,
+                group_on_time=group_on_time,
             )
             batch_totals.update(result)
         return batch_totals
@@ -864,7 +917,11 @@ def get_session_interval(self, session_count: int, interval: str) -> int | None:
         return None
 
     def query_hook(
-        self, event: GroupEvent, start: datetime, end: datetime, environment_id: int
+        self,
+        event: GroupEvent,
+        start: datetime,
+        end: datetime,
+        environment_id: int,
     ) -> float:
         project_id = event.project_id
         session_count_last_hour = self.get_session_count(project_id, environment_id, start, end)
@@ -882,6 +939,7 @@ def query_hook(
                 end=end,
                 environment_id=environment_id,
                 referrer_suffix="alert_event_frequency_percent",
+                group_on_time=False,
             )[event.group_id]
 
             if issue_count > avg_sessions_in_interval:
@@ -900,7 +958,12 @@ def query_hook(
         return 0
 
     def batch_query_hook(
-        self, group_ids: set[int], start: datetime, end: datetime, environment_id: int
+        self,
+        group_ids: set[int],
+        start: datetime,
+        end: datetime,
+        environment_id: int,
+        group_on_time: bool = False,
     ) -> dict[int, int | float]:
         groups = Group.objects.filter(id__in=group_ids).values(
             "id", "type", "project_id", "project__organization_id"
@@ -933,6 +996,7 @@ def batch_query_hook(
             end=end,
             environment_id=environment_id,
             referrer_suffix="batch_alert_event_frequency_percent",
+            group_on_time=group_on_time,
         )
 
         batch_percents: dict[int, int | float] = {}

src/sentry/tasks/beacon.py

@@ -68,7 +68,7 @@ def get_events_24h() -> int:
     end = timezone.now()
     sum_events = 0
     for organization_id in organization_ids:
-        events_per_org_24h = tsdb.backend.get_sums(
+        events_per_org_24h = tsdb.backend.get_timeseries_sums(
             model=TSDBModel.organization_total_received,
             keys=[organization_id],
             start=end - timedelta(hours=24),