Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/sentry/digests/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/models/groupsnooze.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
86 changes: 75 additions & 11 deletions src/sentry/rules/conditions/event_frequency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand All @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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(
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand All @@ -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"
Expand Down Expand Up @@ -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] = {}
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/tasks/beacon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Loading
Loading