Skip to content

Commit 36d5f14

Browse files
evanpurkhiservishnupsatish
authored andcommitted
feat(snuba): Support start/end filter in get_adjacent_event_ids{,_snql} (#93272)
Part of [RTC-856: Issue Details: the timeline filter should apply to the event paginator](https://linear.app/getsentry/issue/RTC-856/issue-details-the-timeline-filter-should-apply-to-the-event-paginator)
1 parent a7eff15 commit 36d5f14

File tree

3 files changed

+98
-57
lines changed

3 files changed

+98
-57
lines changed

src/sentry/eventstore/base.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ def get_adjacent_event_ids_snql(
278278
group_id: int,
279279
environments: list[str],
280280
event: Event | GroupEvent,
281+
start: datetime | None = None,
282+
end: datetime | None = None,
281283
conditions: list[Condition] | None = None,
282284
):
283285
raise NotImplementedError

src/sentry/eventstore/snuba/backend.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -460,14 +460,16 @@ def get_adjacent_event_ids_snql(
460460
group_id: int | None,
461461
environments: Sequence[str],
462462
event: Event | GroupEvent,
463+
start: datetime | None = None,
464+
end: datetime | None = None,
463465
conditions: Sequence[Condition] | None = None,
464466
):
465467
"""
466-
Utility function for grabbing an event's adjascent events,
468+
Utility function for grabbing an event's adjacent events,
467469
which are the ones with the closest timestamps before and after.
468470
This function is only used in project_event_details at the moment,
469471
so it's interface is tailored to that. We use SnQL and use the project_id
470-
and toStartOfDay(timestamp) to efficently scan our table
472+
and toStartOfDay(timestamp) to efficiently scan our table
471473
"""
472474
dataset = self._get_dataset_for_event(event)
473475
app_id = "eventstore"
@@ -492,12 +494,15 @@ def make_constant_conditions():
492494
*project_conditions,
493495
]
494496

497+
lower_bound = start or (event.datetime - timedelta(days=100))
498+
upper_bound = end or (event.datetime + timedelta(days=100))
499+
495500
def make_prev_timestamp_conditions(event):
496501
return [
497502
Condition(
498503
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
499504
Op.GTE,
500-
event.datetime - timedelta(days=100),
505+
lower_bound,
501506
),
502507
Condition(
503508
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
@@ -521,7 +526,7 @@ def make_next_timestamp_conditions(event):
521526
Condition(
522527
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
523528
Op.LT,
524-
event.datetime + timedelta(days=100),
529+
upper_bound,
525530
),
526531
Condition(
527532
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
@@ -600,9 +605,9 @@ def get_adjacent_event_ids(self, event, filter):
600605
prev_filter = deepcopy(filter)
601606
prev_filter.conditions = prev_filter.conditions or []
602607
prev_filter.conditions.extend(get_before_event_condition(event))
603-
604-
# We only store 90 days of data, add a few extra days just in case
605-
prev_filter.start = event.datetime - timedelta(days=100)
608+
if not prev_filter.start:
609+
# We only store 90 days of data, add a few extra days just in case
610+
prev_filter.start = event.datetime - timedelta(days=100)
606611
# the previous event can have the same timestamp, add 1 second
607612
# to the end condition since it uses a less than condition
608613
prev_filter.end = event.datetime + timedelta(seconds=1)
@@ -612,7 +617,8 @@ def get_adjacent_event_ids(self, event, filter):
612617
next_filter.conditions = next_filter.conditions or []
613618
next_filter.conditions.extend(get_after_event_condition(event))
614619
next_filter.start = event.datetime
615-
next_filter.end = datetime.utcnow()
620+
if not next_filter.end:
621+
next_filter.end = datetime.utcnow()
616622
next_filter.orderby = ASC_ORDERING
617623

618624
dataset = self._get_dataset_for_event(event)

tests/sentry/eventstore/snuba/test_backend.py

Lines changed: 82 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
class SnubaEventStorageTest(TestCase, SnubaTestCase, PerformanceIssueTestCase):
1616
def setUp(self):
1717
super().setUp()
18-
self.min_ago = before_now(minutes=1).isoformat()
19-
self.two_min_ago = before_now(minutes=2).isoformat()
18+
self.min_ago = before_now(minutes=1)
19+
self.two_min_ago = before_now(minutes=2)
2020
self.project1 = self.create_project()
2121
self.project2 = self.create_project()
2222

@@ -26,7 +26,7 @@ def setUp(self):
2626
"type": "default",
2727
"platform": "python",
2828
"fingerprint": ["group1"],
29-
"timestamp": self.two_min_ago,
29+
"timestamp": self.two_min_ago.isoformat(),
3030
"tags": {"foo": "1"},
3131
},
3232
project_id=self.project1.id,
@@ -37,7 +37,7 @@ def setUp(self):
3737
"type": "default",
3838
"platform": "python",
3939
"fingerprint": ["group1"],
40-
"timestamp": self.min_ago,
40+
"timestamp": self.min_ago.isoformat(),
4141
"tags": {"foo": "1"},
4242
},
4343
project_id=self.project2.id,
@@ -48,7 +48,7 @@ def setUp(self):
4848
"type": "default",
4949
"platform": "python",
5050
"fingerprint": ["group2"],
51-
"timestamp": self.min_ago,
51+
"timestamp": self.min_ago.isoformat(),
5252
"tags": {"foo": "1"},
5353
},
5454
project_id=self.project2.id,
@@ -176,7 +176,7 @@ def test_get_event_by_id_cached(self):
176176
dummy_event = Event(
177177
project_id=self.project2.id,
178178
event_id="1" * 32,
179-
data={"something": "hi", "timestamp": self.min_ago, "type": "error"},
179+
data={"something": "hi", "timestamp": self.min_ago.isoformat(), "type": "error"},
180180
)
181181
mock_event.return_value = dummy_event
182182
event = self.eventstore.get_event_by_id(self.project2.id, "1" * 32)
@@ -185,7 +185,7 @@ def test_get_event_by_id_cached(self):
185185

186186
# Now we store the event properly, so it will exist in Snuba.
187187
self.store_event(
188-
data={"event_id": "1" * 32, "timestamp": self.min_ago, "type": "error"},
188+
data={"event_id": "1" * 32, "timestamp": self.min_ago.isoformat(), "type": "error"},
189189
project_id=self.project2.id,
190190
)
191191

@@ -217,33 +217,47 @@ def test_get_event_beyond_retention(self):
217217
def test_get_adjacent_event_ids(self):
218218
event = self.eventstore.get_event_by_id(self.project2.id, "b" * 32)
219219

220-
_filter = Filter(project_ids=[self.project1.id, self.project2.id])
220+
filter = Filter(project_ids=[self.project1.id, self.project2.id])
221221

222-
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=_filter)
222+
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=filter)
223223

224224
assert prev_event == (str(self.project1.id), "a" * 32)
225225

226226
# Events with the same timestamp are sorted by event_id
227227
assert next_event == (str(self.project2.id), "c" * 32)
228228

229-
# Returns None if no event
230-
prev_event_none, next_event_none = self.eventstore.get_adjacent_event_ids(
231-
None, filter=_filter
229+
# Returns None if the prev event is outside the start window
230+
period_filter = Filter(
231+
project_ids=[self.project1.id, self.project2.id],
232+
start=self.min_ago,
232233
)
234+
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=period_filter)
235+
assert prev_event is None
236+
assert next_event == (str(self.project2.id), "c" * 32)
237+
238+
# Returns None if the next event is outside the end window
239+
period_filter = Filter(
240+
project_ids=[self.project1.id, self.project2.id],
241+
end=self.min_ago,
242+
)
243+
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=period_filter)
244+
assert prev_event == (str(self.project1.id), "a" * 32)
245+
assert next_event is None
233246

234-
assert prev_event_none is None
235-
assert next_event_none is None
247+
# Returns None if no event
248+
prev_event, next_event = self.eventstore.get_adjacent_event_ids(None, filter=filter)
249+
250+
assert prev_event is None
251+
assert next_event is None
236252

237253
# Returns None if the query fails for a known reason
238254
with mock.patch(
239255
"sentry.utils.snuba.bulk_raw_query", side_effect=snuba.QueryOutsideRetentionError()
240256
):
241-
prev_event_none, next_event_none = self.eventstore.get_adjacent_event_ids(
242-
event, filter=_filter
243-
)
257+
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=filter)
244258

245-
assert prev_event_none is None
246-
assert next_event_none is None
259+
assert prev_event is None
260+
assert next_event is None
247261

248262
def test_adjacent_event_ids_same_timestamp(self):
249263
project = self.create_project()
@@ -254,7 +268,7 @@ def test_adjacent_event_ids_same_timestamp(self):
254268
"type": "default",
255269
"platform": "python",
256270
"fingerprint": ["group"],
257-
"timestamp": self.min_ago,
271+
"timestamp": self.min_ago.isoformat(),
258272
},
259273
project_id=project.id,
260274
)
@@ -264,7 +278,7 @@ def test_adjacent_event_ids_same_timestamp(self):
264278
"type": "default",
265279
"platform": "python",
266280
"fingerprint": ["group"],
267-
"timestamp": self.min_ago,
281+
"timestamp": self.min_ago.isoformat(),
268282
},
269283
project_id=project.id,
270284
)
@@ -316,36 +330,33 @@ def test_transaction_get_next_prev_event_id(self):
316330

317331
def test_get_adjacent_event_ids_snql(self):
318332
project = self.create_project()
333+
data = {
334+
"type": "default",
335+
"platform": "python",
336+
"fingerprint": ["group"],
337+
}
338+
self.store_event(
339+
data={**data, "event_id": "a" * 32, "timestamp": before_now(minutes=10).isoformat()},
340+
project_id=project.id,
341+
)
319342
event1 = self.store_event(
320-
data={
321-
"event_id": "a" * 32,
322-
"type": "default",
323-
"platform": "python",
324-
"fingerprint": ["group"],
325-
"timestamp": self.two_min_ago,
326-
},
343+
data={**data, "event_id": "b" * 32, "timestamp": before_now(minutes=4).isoformat()},
327344
project_id=project.id,
328345
)
329346
event2 = self.store_event(
330-
data={
331-
"event_id": "b" * 32,
332-
"type": "default",
333-
"platform": "python",
334-
"fingerprint": ["group"],
335-
"timestamp": self.min_ago,
336-
},
347+
data={**data, "event_id": "c" * 32, "timestamp": before_now(minutes=3).isoformat()},
337348
project_id=project.id,
338349
)
339350
event3 = self.store_event(
340-
data={
341-
"event_id": "c" * 32,
342-
"type": "default",
343-
"platform": "python",
344-
"fingerprint": ["group"],
345-
"timestamp": before_now(minutes=0).isoformat(),
346-
},
351+
data={**data, "event_id": "d" * 32, "timestamp": before_now(minutes=2).isoformat()},
352+
project_id=project.id,
353+
)
354+
self.store_event(
355+
data={**data, "event_id": "e" * 32, "timestamp": before_now(minutes=0).isoformat()},
347356
project_id=project.id,
348357
)
358+
359+
# Finds next and previous IDs
349360
prev_ids, next_ids = self.eventstore.get_adjacent_event_ids_snql(
350361
organization_id=event2.organization.id,
351362
project_id=event2.project_id,
@@ -357,6 +368,28 @@ def test_get_adjacent_event_ids_snql(self):
357368
assert prev_ids == (str(event1.project_id), event1.event_id)
358369
assert next_ids == (str(event3.project_id), event3.event_id)
359370

371+
# Filter previous events that are outside of the start window
372+
prev_ids, _ = self.eventstore.get_adjacent_event_ids_snql(
373+
organization_id=event1.organization.id,
374+
project_id=event1.project_id,
375+
group_id=event1.group_id,
376+
environments=[],
377+
event=event1,
378+
start=before_now(minutes=5),
379+
)
380+
assert prev_ids is None
381+
382+
# Filter next events that are outside of the end window
383+
_, next_ids = self.eventstore.get_adjacent_event_ids_snql(
384+
organization_id=event3.organization.id,
385+
project_id=event3.project_id,
386+
group_id=event3.group_id,
387+
environments=[],
388+
event=event3,
389+
end=before_now(minutes=1),
390+
)
391+
assert next_ids is None
392+
360393
def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
361394
project = self.create_project()
362395
event1 = self.store_event(
@@ -365,7 +398,7 @@ def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
365398
"type": "default",
366399
"platform": "python",
367400
"fingerprint": ["group"],
368-
"timestamp": self.two_min_ago,
401+
"timestamp": self.two_min_ago.isoformat(),
369402
},
370403
project_id=project.id,
371404
)
@@ -375,7 +408,7 @@ def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
375408
"type": "default",
376409
"platform": "python",
377410
"fingerprint": ["group"],
378-
"timestamp": self.min_ago,
411+
"timestamp": self.min_ago.isoformat(),
379412
},
380413
project_id=project.id,
381414
)
@@ -408,7 +441,7 @@ def test_adjacent_event_ids_same_timestamp_snql(self):
408441
"type": "default",
409442
"platform": "python",
410443
"fingerprint": ["group"],
411-
"timestamp": self.min_ago,
444+
"timestamp": self.min_ago.isoformat(),
412445
},
413446
project_id=project.id,
414447
)
@@ -418,7 +451,7 @@ def test_adjacent_event_ids_same_timestamp_snql(self):
418451
"type": "default",
419452
"platform": "python",
420453
"fingerprint": ["group"],
421-
"timestamp": self.min_ago,
454+
"timestamp": self.min_ago.isoformat(),
422455
},
423456
project_id=project.id,
424457
)
@@ -458,7 +491,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
458491
"type": "default",
459492
"platform": "python",
460493
"fingerprint": ["group"],
461-
"timestamp": self.min_ago,
494+
"timestamp": self.min_ago.isoformat(),
462495
"tags": {"organization.slug": "sentry"},
463496
},
464497
project_id=project.id,
@@ -469,7 +502,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
469502
"type": "default",
470503
"platform": "python",
471504
"fingerprint": ["group"],
472-
"timestamp": self.min_ago,
505+
"timestamp": self.min_ago.isoformat(),
473506
},
474507
project_id=project.id,
475508
)
@@ -479,7 +512,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
479512
"type": "default",
480513
"platform": "python",
481514
"fingerprint": ["group"],
482-
"timestamp": self.min_ago,
515+
"timestamp": self.min_ago.isoformat(),
483516
"tags": {"organization.slug": "sentry"},
484517
},
485518
project_id=project.id,

0 commit comments

Comments
 (0)