Skip to content

Commit 7e5cd9b

Browse files
yuvmenandrewshie-sentry
authored andcommitted
feat(upsampling) - Upsample Error counts for ACI Event Frequency Alerts (#97517)
- Thread project_ids through ACI Event Frequency handlers to enable Snuba upsampling. - Add tests: upsampling enabled (10) vs disabled (2). - Ended up adding project_ids to all tsdb_functions to avoid the ambiguity in code, at the expense of adding it where it isnt relevant. We have other parameters that follow this pattern there, so it seemed the lesser evil.
1 parent 40aabbc commit 7e5cd9b

File tree

7 files changed

+113
-28
lines changed

7 files changed

+113
-28
lines changed

src/sentry/rules/conditions/event_frequency.py

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -351,34 +351,19 @@ def get_snuba_query_result(
351351
group_on_time: bool = False,
352352
project_ids: list[int] | None = None,
353353
) -> Mapping[int, int]:
354-
kwargs = {
355-
"model": model,
356-
"keys": keys,
357-
"start": start,
358-
"end": end,
359-
"environment_id": environment_id,
360-
"use_cache": True,
361-
"jitter_value": group_id,
362-
"tenant_ids": {"organization_id": organization_id},
363-
"referrer_suffix": referrer_suffix,
364-
"group_on_time": group_on_time,
365-
}
366-
367-
# Try to pass project_ids if provided, but fall back gracefully if not supported
368-
result: Mapping[int, int]
369-
if project_ids is not None:
370-
try:
371-
kwargs["project_ids"] = project_ids
372-
result = tsdb_function(**kwargs)
373-
except TypeError as e:
374-
if "project_ids" in str(e):
375-
# Function doesn't support project_ids, try without it
376-
kwargs.pop("project_ids", None)
377-
result = tsdb_function(**kwargs)
378-
else:
379-
raise
380-
else:
381-
result = tsdb_function(**kwargs)
354+
result: Mapping[int, int] = tsdb_function(
355+
model=model,
356+
keys=keys,
357+
start=start,
358+
end=end,
359+
environment_id=environment_id,
360+
use_cache=True,
361+
jitter_value=group_id,
362+
tenant_ids={"organization_id": organization_id},
363+
referrer_suffix=referrer_suffix,
364+
group_on_time=group_on_time,
365+
project_ids=project_ids,
366+
)
382367
return result
383368

384369
def get_chunked_result(

src/sentry/tsdb/base.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,7 @@ def get_distinct_counts_series(
588588
rollup: int | None = None,
589589
environment_id: int | None = None,
590590
tenant_ids: dict[str, str | int] | None = None,
591+
project_ids: Sequence[int] | None = None,
591592
) -> dict[int, list[tuple[int, Any]]]:
592593
"""
593594
Fetch counts of distinct items for each rollup interval within the range.
@@ -608,6 +609,7 @@ def get_distinct_counts_totals(
608609
referrer_suffix: str | None = None,
609610
conditions: list[SnubaCondition] | None = None,
610611
group_on_time: bool = False,
612+
project_ids: Sequence[int] | None = None,
611613
) -> Mapping[TSDBKey, int]:
612614
"""
613615
Count distinct items during a time range with optional conditions
@@ -665,6 +667,7 @@ def get_frequency_series(
665667
rollup: int | None = None,
666668
environment_id: int | None = None,
667669
tenant_ids: dict[str, str | int] | None = None,
670+
project_ids: Sequence[int] | None = None,
668671
) -> dict[TSDBKey, list[tuple[float, dict[TSDBItem, float]]]]:
669672
"""
670673
Retrieve the frequency of known items in a table over time.

src/sentry/tsdb/dummy.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def get_distinct_counts_series(
5858
rollup: int | None = None,
5959
environment_id: int | None = None,
6060
tenant_ids: dict[str, str | int] | None = None,
61+
project_ids: Sequence[int] | None = None,
6162
) -> dict[int, list[tuple[int, Any]]]:
6263
self.validate_arguments([model], [environment_id])
6364
_, series = self.get_optimal_rollup_series(start, end, rollup)
@@ -77,6 +78,7 @@ def get_distinct_counts_totals(
7778
referrer_suffix=None,
7879
conditions=None,
7980
group_on_time: bool = False,
81+
project_ids: Sequence[int] | None = None,
8082
):
8183
self.validate_arguments([model], [environment_id])
8284
return {k: 0 for k in keys}
@@ -108,6 +110,7 @@ def get_frequency_series(
108110
rollup: int | None = None,
109111
environment_id: int | None = None,
110112
tenant_ids: dict[str, str | int] | None = None,
113+
project_ids: Sequence[int] | None = None,
111114
) -> dict[TSDBKey, list[tuple[float, dict[TSDBItem, float]]]]:
112115
self.validate_arguments([model], [environment_id])
113116
rollup, series = self.get_optimal_rollup_series(start, end, rollup)

src/sentry/tsdb/redis.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,7 @@ def get_distinct_counts_series(
501501
rollup: int | None = None,
502502
environment_id: int | None = None,
503503
tenant_ids: dict[str, str | int] | None = None,
504+
project_ids: Sequence[int] | None = None,
504505
) -> dict[int, list[tuple[int, Any]]]:
505506
"""
506507
Fetch counts of distinct items for each rollup interval within the range.
@@ -542,6 +543,7 @@ def get_distinct_counts_totals(
542543
referrer_suffix: str | None = None,
543544
conditions: list[SnubaCondition] | None = None,
544545
group_on_time: bool = False,
546+
project_ids: Sequence[int] | None = None,
545547
) -> Mapping[TSDBKey, int]:
546548
"""
547549
Count distinct items during a time range.
@@ -753,6 +755,7 @@ def get_frequency_series(
753755
rollup: int | None = None,
754756
environment_id: int | None = None,
755757
tenant_ids: dict[str, str | int] | None = None,
758+
project_ids: Sequence[int] | None = None,
756759
) -> dict[TSDBKey, list[tuple[float, dict[TSDBItem, float]]]]:
757760
self.validate_arguments([model], [environment_id])
758761

src/sentry/tsdb/snuba.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,7 @@ def get_distinct_counts_series(
839839
rollup=None,
840840
environment_id=None,
841841
tenant_ids=None,
842+
project_ids: Sequence[int] | None = None,
842843
):
843844
result = self.get_data(
844845
model,
@@ -871,6 +872,7 @@ def get_distinct_counts_totals(
871872
referrer_suffix=None,
872873
conditions=None,
873874
group_on_time: bool = False,
875+
project_ids: Sequence[int] | None = None,
874876
) -> Mapping[TSDBKey, int]:
875877
return self.get_data(
876878
model,
@@ -897,6 +899,7 @@ def get_frequency_series(
897899
rollup: int | None = None,
898900
environment_id: int | None = None,
899901
tenant_ids: dict[str, str | int] | None = None,
902+
project_ids: Sequence[int] | None = None,
900903
) -> dict[TSDBKey, list[tuple[float, dict[TSDBItem, float]]]]:
901904
result = self.get_data(
902905
model,

src/sentry/workflow_engine/handlers/condition/event_frequency_query_handlers.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def __call__(
5555
referrer_suffix: str | None = None,
5656
conditions: list[SnubaCondition] | None = None,
5757
group_on_time: bool = False,
58+
project_ids: list[int] | None = None,
5859
) -> Mapping[TSDBKey, int]: ...
5960

6061

@@ -101,6 +102,7 @@ def get_snuba_query_result(
101102
referrer_suffix: str,
102103
conditions: list[SnubaCondition] | None = None,
103104
group_on_time: bool = False,
105+
project_ids: list[int] | None = None,
104106
) -> Mapping[int, int]:
105107
result: Mapping[int, int] = tsdb_function(
106108
model=model,
@@ -114,6 +116,7 @@ def get_snuba_query_result(
114116
referrer_suffix=referrer_suffix,
115117
conditions=conditions,
116118
group_on_time=group_on_time,
119+
project_ids=project_ids,
117120
)
118121
return result
119122

@@ -129,6 +132,7 @@ def get_chunked_result(
129132
referrer_suffix: str,
130133
filters: list[QueryFilter] | None = None,
131134
group_on_time: bool = False,
135+
project_ids: list[int] | None = None,
132136
) -> dict[int, int]:
133137
batch_totals: dict[int, int] = defaultdict(int)
134138
group_id = group_ids[0]
@@ -146,6 +150,7 @@ def get_chunked_result(
146150
referrer_suffix=referrer_suffix,
147151
conditions=conditions,
148152
group_on_time=group_on_time,
153+
project_ids=project_ids,
149154
)
150155
batch_totals.update(result)
151156
return batch_totals
@@ -332,6 +337,8 @@ def batch_query(
332337
batch_sums: QueryResult = defaultdict(int)
333338
category_group_ids = self.get_group_ids_by_category(groups)
334339
organization_id = self.get_value_from_groups(groups, "project__organization_id")
340+
# Build project_ids list from incoming groups
341+
project_ids = list({g["project_id"] for g in groups}) if groups else []
335342

336343
if not organization_id:
337344
return batch_sums
@@ -350,6 +357,7 @@ def batch_query(
350357
referrer_suffix="wf_batch_alert_event_frequency",
351358
filters=filters,
352359
group_on_time=False,
360+
project_ids=project_ids,
353361
)
354362
except InvalidFilter:
355363
# Filter is not supported for this issue type

tests/sentry/workflow_engine/handlers/condition/test_event_frequency_query_handlers.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,86 @@ def test_batch_query(self) -> None:
4242
)
4343
assert batch_query == {self.event3.group_id: 1}
4444

45+
def test_batch_query_with_upsampling_enabled_counts_upsampled(self) -> None:
46+
# Create two sampled error events in a dedicated group
47+
event_a = self.store_event(
48+
data={
49+
"event_id": "d" * 32,
50+
"environment": self.environment.name,
51+
"timestamp": before_now(seconds=20).isoformat(),
52+
"fingerprint": ["upsampled-group"],
53+
"contexts": {"error_sampling": {"client_sample_rate": 0.2}},
54+
"exception": {"values": [{"type": "ValueError", "value": "a"}]},
55+
},
56+
project_id=self.project.id,
57+
)
58+
self.store_event(
59+
data={
60+
"event_id": "e" * 32,
61+
"environment": self.environment.name,
62+
"timestamp": before_now(seconds=10).isoformat(),
63+
"fingerprint": ["upsampled-group"],
64+
"contexts": {"error_sampling": {"client_sample_rate": 0.2}},
65+
"exception": {"values": [{"type": "ValueError", "value": "b"}]},
66+
},
67+
project_id=self.project.id,
68+
)
69+
70+
groups = list(
71+
Group.objects.filter(id=event_a.group_id).values(
72+
"id", "type", "project_id", "project__organization_id"
73+
)
74+
)
75+
76+
with self.options({"issues.client_error_sampling.project_allowlist": [self.project.id]}):
77+
batch_query = self.handler().batch_query(
78+
groups=groups,
79+
start=self.start,
80+
end=self.end,
81+
environment_id=self.environment.id,
82+
)
83+
# Expect 2 events upsampled by 5x => 10
84+
assert batch_query[event_a.group_id] == 10
85+
86+
def test_batch_query_without_upsampling_counts_raw(self) -> None:
87+
# Same setup as above but without allowlist; expect raw count of 2
88+
event_a = self.store_event(
89+
data={
90+
"event_id": "f" * 32,
91+
"environment": self.environment.name,
92+
"timestamp": before_now(seconds=20).isoformat(),
93+
"fingerprint": ["upsampled-group-raw"],
94+
"contexts": {"error_sampling": {"client_sample_rate": 0.2}},
95+
"exception": {"values": [{"type": "ValueError", "value": "a"}]},
96+
},
97+
project_id=self.project.id,
98+
)
99+
self.store_event(
100+
data={
101+
"event_id": "1" * 32,
102+
"environment": self.environment.name,
103+
"timestamp": before_now(seconds=10).isoformat(),
104+
"fingerprint": ["upsampled-group-raw"],
105+
"contexts": {"error_sampling": {"client_sample_rate": 0.2}},
106+
"exception": {"values": [{"type": "ValueError", "value": "b"}]},
107+
},
108+
project_id=self.project.id,
109+
)
110+
111+
groups = list(
112+
Group.objects.filter(id=event_a.group_id).values(
113+
"id", "type", "project_id", "project__organization_id"
114+
)
115+
)
116+
117+
batch_query = self.handler().batch_query(
118+
groups=groups,
119+
start=self.start,
120+
end=self.end,
121+
environment_id=self.environment.id,
122+
)
123+
assert batch_query[event_a.group_id] == 2
124+
45125
def test_batch_query__tag_conditions__equal(self) -> None:
46126
batch_query = self.handler().batch_query(
47127
groups=self.groups,

0 commit comments

Comments
 (0)