Skip to content

Commit c66a6a5

Browse files
authored
fix(replay): Add fallback ordering to replay index page (#103328)
fixes REPLAY-830 In the replay index page, when sorting by columns that contain duplicate values (e.g., startedAt, duration, countErrors), the order of rows with identical values is non-deterministic, which results in inconsistent pagination. Let's add a fallback ordering on replay_id. We are already grouping by replay_id so the fallback sort will operate on the aggregated results.
1 parent 0149e9e commit c66a6a5

File tree

2 files changed

+44
-7
lines changed

2 files changed

+44
-7
lines changed

src/sentry/replays/usecases/query/__init__.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,11 @@ def _query_using_scalar_strategy(
344344

345345
try:
346346
where = handle_search_filters(scalar_search_config, search_filters)
347-
orderby = handle_ordering(agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD)
347+
orderby = handle_ordering(
348+
agg_sort_config,
349+
sort or "-" + DEFAULT_SORT_FIELD,
350+
tiebreaker="replay_id", # Ensure stable sort when ordering by column with duplicates
351+
)
348352
except RetryAggregated:
349353
return _query_using_aggregated_strategy(
350354
search_filters,
@@ -378,7 +382,11 @@ def _query_using_aggregated_strategy(
378382
period_start: datetime,
379383
period_stop: datetime,
380384
):
381-
orderby = handle_ordering(agg_sort_config, sort or "-" + DEFAULT_SORT_FIELD)
385+
orderby = handle_ordering(
386+
agg_sort_config,
387+
sort or "-" + DEFAULT_SORT_FIELD,
388+
tiebreaker="replay_id", # Ensure stable sort when ordering by column with duplicates
389+
)
382390

383391
having: list[Condition] = handle_search_filters(agg_search_config, search_filters)
384392
having.append(Condition(Function("min", parameters=[Column("segment_id")]), Op.EQ, 0))
@@ -459,11 +467,16 @@ def execute_query(query: Query, tenant_id: dict[str, int], referrer: str) -> Map
459467
raise
460468

461469

462-
def handle_ordering(config: dict[str, Expression], sort: str) -> list[OrderBy]:
463-
if sort.startswith("-"):
464-
return [OrderBy(_get_sort_column(config, sort[1:]), Direction.DESC)]
465-
else:
466-
return [OrderBy(_get_sort_column(config, sort), Direction.ASC)]
470+
def handle_ordering(
471+
config: dict[str, Expression], sort: str, tiebreaker: str | None = None
472+
) -> list[OrderBy]:
473+
direction = Direction.DESC if sort.startswith("-") else Direction.ASC
474+
bare_sort = sort[1:] if sort.startswith("-") else sort
475+
476+
orderby = [OrderBy(_get_sort_column(config, bare_sort), direction)]
477+
if tiebreaker:
478+
orderby.append(OrderBy(Column(tiebreaker), direction))
479+
return orderby
467480

468481

469482
def _get_sort_column(config: dict[str, Expression], column_name: str) -> Function:

tests/sentry/replays/endpoints/test_organization_replay_index.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,30 @@ def test_get_replays_duration_sorted(self) -> None:
485485
assert response_data["data"][0]["id"] == replay2_id
486486
assert response_data["data"][1]["id"] == replay1_id
487487

488+
def test_get_replays_duration_sorted_tiebreaker(self) -> None:
489+
"""Test that replays with identical durations have deterministic order when ordering by duration."""
490+
project = self.create_project(teams=[self.team])
491+
replay_ids = [uuid.uuid4().hex for _ in range(5)]
492+
493+
timestamp_start = datetime.datetime.now() - datetime.timedelta(seconds=10)
494+
timestamp_end = datetime.datetime.now() - datetime.timedelta(seconds=5)
495+
496+
for replay_id in replay_ids:
497+
self.store_replays(mock_replay(timestamp_start, project.id, replay_id, segment_id=0))
498+
self.store_replays(mock_replay(timestamp_end, project.id, replay_id, segment_id=1))
499+
500+
with self.feature(self.features):
501+
response = self.client.get(self.url + "?orderBy=duration")
502+
assert response.status_code == 200, response
503+
asc_order = [r["id"] for r in response.json()["data"]]
504+
505+
response = self.client.get(self.url + "?orderBy=-duration")
506+
assert response.status_code == 200, response
507+
desc_order = [r["id"] for r in response.json()["data"]]
508+
509+
assert desc_order == list(reversed(asc_order))
510+
assert set(asc_order) == set(replay_ids)
511+
488512
def test_get_replays_pagination(self) -> None:
489513
"""Test replays can be paginated."""
490514
project = self.create_project(teams=[self.team])

0 commit comments

Comments
 (0)