Skip to content

Commit 54cf7e5

Browse files
authored
ref(rules): Add batch_query_hook to EventFrequencyCondition (#68559)
Add a `batch_query_hook` to the `EventFrequencyCondition` so that when we process "slow" rule conditions we can batch the queries by group ids. Partially closes getsentry/team-core-product-foundations#239
1 parent 9381168 commit 54cf7e5

File tree

2 files changed

+159
-12
lines changed

2 files changed

+159
-12
lines changed

src/sentry/rules/conditions/event_frequency.py

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import contextlib
55
import logging
66
import re
7+
from collections import defaultdict
78
from collections.abc import Mapping, Sequence
89
from datetime import datetime, timedelta
910
from typing import Any
@@ -15,15 +16,19 @@
1516
from sentry import release_health, tsdb
1617
from sentry.eventstore.models import GroupEvent
1718
from sentry.issues.constants import get_issue_tsdb_group_model, get_issue_tsdb_user_group_model
19+
from sentry.issues.grouptype import GroupCategory
20+
from sentry.models.group import Group
1821
from sentry.receivers.rules import DEFAULT_RULE_LABEL, DEFAULT_RULE_LABEL_NEW
1922
from sentry.rules import EventState
2023
from sentry.rules.conditions.base import EventCondition
24+
from sentry.tsdb.base import TSDBModel
2125
from sentry.types.condition_activity import (
2226
FREQUENCY_CONDITION_BUCKET_SIZE,
2327
ConditionActivity,
2428
round_to_five_minute,
2529
)
2630
from sentry.utils import metrics
31+
from sentry.utils.iterators import chunked
2732
from sentry.utils.snuba import options_override
2833

2934
standard_intervals = {
@@ -49,6 +54,7 @@
4954
COMPARISON_TYPE_COUNT: COMPARISON_TYPE_COUNT,
5055
COMPARISON_TYPE_PERCENT: COMPARISON_TYPE_PERCENT,
5156
}
57+
SNUBA_LIMIT = 10000
5258

5359

5460
class EventFrequencyForm(forms.Form):
@@ -241,6 +247,29 @@ def get_rate(self, event: GroupEvent, interval: str, environment_id: str) -> int
241247

242248
return result
243249

250+
def get_sums(
251+
self,
252+
keys: list[int],
253+
group: Group,
254+
model: TSDBModel,
255+
start: datetime,
256+
end: datetime,
257+
environment_id: str,
258+
referrer_suffix: str,
259+
) -> Mapping[int, int]:
260+
sums: Mapping[int, int] = self.tsdb.get_sums(
261+
model=model,
262+
keys=keys,
263+
start=start,
264+
end=end,
265+
environment_id=environment_id,
266+
use_cache=True,
267+
jitter_value=group.id,
268+
tenant_ids={"organization_id": group.project.organization_id},
269+
referrer_suffix=referrer_suffix,
270+
)
271+
return sums
272+
244273
@property
245274
def is_guessed_to_be_created_on_project_creation(self) -> bool:
246275
"""
@@ -265,19 +294,70 @@ class EventFrequencyCondition(BaseEventFrequencyCondition):
265294
def query_hook(
266295
self, event: GroupEvent, start: datetime, end: datetime, environment_id: str
267296
) -> int:
268-
sums: Mapping[int, int] = self.tsdb.get_sums(
269-
model=get_issue_tsdb_group_model(event.group.issue_category),
297+
sums: Mapping[int, int] = self.get_sums(
270298
keys=[event.group_id],
299+
group=event.group,
300+
model=get_issue_tsdb_group_model(event.group.issue_category),
271301
start=start,
272302
end=end,
273303
environment_id=environment_id,
274-
use_cache=True,
275-
jitter_value=event.group_id,
276-
tenant_ids={"organization_id": event.group.project.organization_id},
277304
referrer_suffix="alert_event_frequency",
278305
)
279306
return sums[event.group_id]
280307

308+
def get_chunked_sums(
309+
self,
310+
groups: list[Group],
311+
start: datetime,
312+
end: datetime,
313+
environment_id: str,
314+
referrer_suffix: str,
315+
) -> dict[int, int]:
316+
batch_sums: dict[int, int] = defaultdict(int)
317+
for group_chunk in chunked(groups, SNUBA_LIMIT):
318+
group = groups[0]
319+
sums = self.get_sums(
320+
keys=[group.id for group in groups],
321+
group=group,
322+
model=get_issue_tsdb_group_model(group.issue_category),
323+
start=start,
324+
end=end,
325+
environment_id=environment_id,
326+
referrer_suffix=referrer_suffix,
327+
)
328+
batch_sums.update(sums)
329+
return batch_sums
330+
331+
def batch_query_hook(
332+
self, group_ids: Sequence[int], start: datetime, end: datetime, environment_id: str
333+
) -> dict[int, int]:
334+
batch_sums: dict[int, int] = defaultdict(int)
335+
groups = Group.objects.filter(id__in=group_ids)
336+
error_issues = [group for group in groups if group.issue_category == GroupCategory.ERROR]
337+
generic_issues = [group for group in groups if group.issue_category != GroupCategory.ERROR]
338+
339+
if error_issues:
340+
error_sums = self.get_chunked_sums(
341+
groups=error_issues,
342+
start=start,
343+
end=end,
344+
environment_id=environment_id,
345+
referrer_suffix="alert_event_frequency",
346+
)
347+
batch_sums.update(error_sums)
348+
349+
if generic_issues:
350+
generic_sums = self.get_chunked_sums(
351+
groups=generic_issues,
352+
start=start,
353+
end=end,
354+
environment_id=environment_id,
355+
referrer_suffix="alert_event_frequency",
356+
)
357+
batch_sums.update(generic_sums)
358+
359+
return batch_sums
360+
281361
def get_preview_aggregate(self) -> tuple[str, str]:
282362
return "count", "roundedTime"
283363

@@ -397,16 +477,13 @@ def query_hook(
397477
percent_intervals[self.get_option("interval")][1].total_seconds() // 60
398478
)
399479
avg_sessions_in_interval = session_count_last_hour / (60 / interval_in_minutes)
400-
401-
issue_count = self.tsdb.get_sums(
402-
model=get_issue_tsdb_group_model(event.group.issue_category),
480+
issue_count = self.get_sums(
403481
keys=[event.group_id],
482+
group=event.group,
483+
model=get_issue_tsdb_group_model(event.group.issue_category),
404484
start=start,
405485
end=end,
406486
environment_id=environment_id,
407-
use_cache=True,
408-
jitter_value=event.group_id,
409-
tenant_ids={"organization_id": event.group.project.organization_id},
410487
referrer_suffix="alert_event_frequency_percent",
411488
)[event.group_id]
412489
if issue_count > avg_sessions_in_interval:

tests/snuba/rules/conditions/test_event_frequency.py

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,76 @@
2828
pytestmark = [pytest.mark.sentry_metrics, requires_snuba]
2929

3030

31+
class EventFrequencyQueryTest(SnubaTestCase, RuleTestCase, PerformanceIssueTestCase):
32+
rule_cls = EventFrequencyCondition
33+
34+
def test_batch_query(self):
35+
event = self.store_event(
36+
data={
37+
"event_id": "a" * 32,
38+
"environment": self.environment.name,
39+
"timestamp": iso_format(before_now(seconds=30)),
40+
"fingerprint": ["group-1"],
41+
},
42+
project_id=self.project.id,
43+
)
44+
event2 = self.store_event(
45+
data={
46+
"event_id": "b" * 32,
47+
"environment": self.environment.name,
48+
"timestamp": iso_format(before_now(seconds=12)),
49+
"fingerprint": ["group-2"],
50+
},
51+
project_id=self.project.id,
52+
)
53+
environment2 = self.create_environment(name="staging")
54+
event3 = self.store_event(
55+
data={
56+
"event_id": "c" * 32,
57+
"environment": environment2.name,
58+
"timestamp": iso_format(before_now(seconds=12)),
59+
"fingerprint": ["group-3"],
60+
},
61+
project_id=self.project.id,
62+
)
63+
64+
fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-something_random"
65+
event_data = load_data(
66+
"transaction-n-plus-one",
67+
timestamp=before_now(seconds=12),
68+
start_timestamp=before_now(seconds=12),
69+
fingerprint=[fingerprint],
70+
)
71+
event_data["user"] = {"id": uuid4().hex}
72+
event_data["environment"] = self.environment.name
73+
74+
# Store a performance event
75+
perf_event = self.create_performance_issue(
76+
event_data=event_data,
77+
project_id=self.project.id,
78+
fingerprint=fingerprint,
79+
)
80+
start = before_now(minutes=1)
81+
end = timezone.now()
82+
83+
condition_inst = self.rule_cls(event.group.project)
84+
batch_query = condition_inst.batch_query_hook(
85+
group_ids=[event.group_id, event2.group_id, perf_event.group_id],
86+
start=start,
87+
end=end,
88+
environment_id=self.environment.id,
89+
)
90+
assert batch_query == {event.group_id: 1, event2.group_id: 1, perf_event.group_id: 1}
91+
92+
batch_query = condition_inst.batch_query_hook(
93+
group_ids=[event3.group_id],
94+
start=start,
95+
end=end,
96+
environment_id=environment2.id,
97+
)
98+
assert batch_query == {event3.group_id: 1}
99+
100+
31101
class ErrorEventMixin(SnubaTestCase):
32102
def add_event(self, data, project_id, timestamp):
33103
data["timestamp"] = iso_format(timestamp)
@@ -72,7 +142,7 @@ def add_event(self, data, project_id, timestamp):
72142

73143

74144
@pytest.mark.snuba_ci
75-
class StandardIntervalTestBase(SnubaTestCase, RuleTestCase):
145+
class StandardIntervalTestBase(SnubaTestCase, RuleTestCase, PerformanceIssueTestCase):
76146
__test__ = Abstract(__module__, __qualname__)
77147

78148
def add_event(self, data, project_id, timestamp):

0 commit comments

Comments
 (0)