Skip to content

Commit 908fc04

Browse files
committed
feat(occurrences): Preprocess redirects
We do not update EAP in the event of a group merge. Instead, we track the merge in our "Group Redirect" table. This PR preprocesses EAP queries to account for the redirects, so users can always query and get all their relevant groups.
1 parent d6e2310 commit 908fc04

File tree

2 files changed

+82
-1
lines changed

2 files changed

+82
-1
lines changed

src/sentry/snuba/occurrences_rpc.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import logging
22
from enum import Enum
3+
from re import finditer
34
from typing import Any
45

56
import sentry_sdk
@@ -12,6 +13,7 @@
1213
from sentry.search.eap.resolver import SearchResolver
1314
from sentry.search.eap.types import AdditionalQueries, EAPResponse, SearchResolverConfig
1415
from sentry.search.events.types import SAMPLING_MODES, SnubaParams
16+
from sentry.services.eventstore.query_preprocessing import get_all_merged_group_ids
1517
from sentry.snuba import rpc_dataset_common
1618
from sentry.utils.snuba import process_value
1719

@@ -261,3 +263,40 @@ def _build_category_filter(cls, category: OccurrenceCategory | None) -> TraceIte
261263
)
262264

263265
return None
266+
267+
@staticmethod
268+
def _update_eap_query_string_with_merged_group_ids(qs: str) -> str:
269+
matches = finditer(r"group_id:(\d+|\[(\d(, )?)+\])", qs)
270+
pieces = []
271+
i = 0
272+
for m in matches:
273+
match_start, match_end = m.span()
274+
if match_start != i:
275+
pieces.append(qs[i:match_start])
276+
277+
match = qs[match_start:match_end]
278+
if "[" in match:
279+
group_ids = match.split("[")[1][:-1].split(", ")
280+
all_group_ids = get_all_merged_group_ids({int(gid) for gid in group_ids})
281+
282+
# Only need to update in the "new matches found" case.
283+
if len(all_group_ids) > len(group_ids):
284+
pieces.append(f"group_id:[{', '.join(str(gid) for gid in all_group_ids)}]")
285+
else:
286+
pieces.append(match)
287+
288+
else:
289+
group_id = match.split(":")[1]
290+
all_group_ids = get_all_merged_group_ids({int(group_id)})
291+
292+
# Only need to update in the "new matches found" case.
293+
if len(all_group_ids) > 1:
294+
pieces.append(f"group_id:[{', '.join(str(gid) for gid in all_group_ids)}]")
295+
else:
296+
pieces.append(match)
297+
298+
i = match_end
299+
300+
pieces.append(qs[i:])
301+
302+
return "".join(pieces)

tests/sentry/snuba/test_occurrences_rpc.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from datetime import datetime, timedelta, timezone
1+
from datetime import UTC, datetime, timedelta, timezone
22

33
import pytest
44
from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import (
@@ -19,6 +19,7 @@
1919
)
2020

2121
from sentry.exceptions import InvalidSearchQuery
22+
from sentry.models.groupredirect import GroupRedirect
2223
from sentry.search.eap.occurrences.definitions import OCCURRENCE_DEFINITIONS
2324
from sentry.search.eap.resolver import SearchResolver
2425
from sentry.search.eap.types import SearchResolverConfig
@@ -225,6 +226,47 @@ def test_type_negation_filter_query(self) -> None:
225226
)
226227
assert having is None
227228

229+
def test_update_eap_query_string_with_merged_group_ids(self) -> None:
230+
self.g1 = self.create_group(id=1)
231+
self.g2 = self.create_group(id=2)
232+
self.g3 = self.create_group(id=3)
233+
self.g4 = self.create_group(id=4)
234+
235+
self.gr31 = GroupRedirect.objects.create(
236+
id=10001,
237+
organization_id=self.g1.project.organization_id,
238+
group_id=self.g1.id,
239+
previous_group_id=self.g3.id,
240+
date_added=datetime.now(UTC) - timedelta(hours=4),
241+
)
242+
self.gr21 = GroupRedirect.objects.create(
243+
id=10002,
244+
organization_id=self.g1.project.organization_id,
245+
group_id=self.g1.id,
246+
previous_group_id=self.g2.id,
247+
date_added=datetime.now(UTC) - timedelta(hours=1),
248+
)
249+
250+
assert Occurrences._update_eap_query_string_with_merged_group_ids("foo:bar") == "foo:bar"
251+
assert (
252+
Occurrences._update_eap_query_string_with_merged_group_ids("group_id:1")
253+
== "group_id:[1, 2, 3]"
254+
)
255+
assert (
256+
Occurrences._update_eap_query_string_with_merged_group_ids("group_id:3")
257+
== "group_id:[1, 3]"
258+
)
259+
assert (
260+
Occurrences._update_eap_query_string_with_merged_group_ids("group_id:[3, 4]")
261+
== "group_id:[1, 3, 4]"
262+
)
263+
assert (
264+
Occurrences._update_eap_query_string_with_merged_group_ids(
265+
"group_id:1 foo:bar group_id:[3, 4]"
266+
)
267+
== "group_id:[1, 2, 3] foo:bar group_id:[1, 3, 4]"
268+
)
269+
228270

229271
class OccurrencesTimeseriesTest(TestCase):
230272
def setUp(self) -> None:

0 commit comments

Comments
 (0)