Skip to content

Commit

Permalink
fix(replays): Screen replays missing their starting segment (#56112)
Browse files Browse the repository at this point in the history
  • Loading branch information
cmanallen committed Sep 12, 2023
1 parent c4c5b2b commit 3db3d84
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/sentry/replays/usecases/query/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ def make_scalar_search_conditions_query(
period_start: datetime,
period_stop: datetime,
) -> Query:
# NOTE: This query may return replay-ids which do not have a segment_id 0 row. These replays
# will be removed from the final output and could lead to pagination peculiarities. In
# practice, this is not expected to be noticable by the end-user.
#
# To fix this issue remove the ability to search against "varying" columns and apply a
# "segment_id = 0" condition to the WHERE clause.

where = handle_search_filters(scalar_search_config, search_filters)
orderby = handle_ordering(sort)

Expand All @@ -251,9 +258,11 @@ def make_aggregate_search_conditions_query(
period_start: datetime,
period_stop: datetime,
) -> Query:
having: list[Condition] = handle_search_filters(agg_search_config, search_filters)
orderby = handle_ordering(sort)

having: list[Condition] = handle_search_filters(agg_search_config, search_filters)
having.append(Condition(Function("min", parameters=[Column("segment_id")]), Op.EQ, 0))

return Query(
match=Entity("replays"),
select=[Column("replay_id")],
Expand Down Expand Up @@ -299,6 +308,10 @@ def _select_from_fields() -> list[Union[Column, Function]]:
Condition(Column("timestamp"), Op.GTE, period_start - timedelta(hours=1)),
Condition(Column("timestamp"), Op.LT, period_end + timedelta(hours=1)),
],
# NOTE: Refer to this note: "make_scalar_search_conditions_query".
#
# This condition ensures that every replay shown to the user is valid.
having=[Condition(Function("min", parameters=[Column("segment_id")]), Op.EQ, 0)],
groupby=[Column("project_id"), Column("replay_id")],
granularity=Granularity(3600),
)
Expand Down
18 changes: 18 additions & 0 deletions tests/sentry/replays/test_organization_replay_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -1695,3 +1695,21 @@ def test_query_scalar_optimization_multiple_varying(self):
assert response.status_code == 200
response_data = response.json()
assert len(response_data["data"]) == 1

def test_get_replays_missing_segment_0(self):
"""Test fetching replays when the 0th segment is missing."""
project = self.create_project(teams=[self.team])

replay1_id = uuid.uuid4().hex
seq1_timestamp = datetime.datetime.now() - datetime.timedelta(seconds=22)
seq2_timestamp = datetime.datetime.now() - datetime.timedelta(seconds=5)
self.store_replays(mock_replay(seq1_timestamp, project.id, replay1_id, segment_id=2))
self.store_replays(mock_replay(seq2_timestamp, project.id, replay1_id, segment_id=1))

with self.feature(REPLAYS_FEATURES):
response = self.client.get(self.url)
assert response.status_code == 200

response_data = response.json()
assert "data" in response_data
assert len(response_data["data"]) == 0

0 comments on commit 3db3d84

Please sign in to comment.