Skip to content

Conversation

@wmak
Copy link
Member

@wmak wmak commented Nov 26, 2025

  • This adds a replays dataset to the events endpoint since we're going to be adding replays to EAP
  • I need to go through all the attributes on replays still and figure out if any of them will have different public aliases etc. but for now I just want to get the base work up of allowing these queries

- This adds a replays dataset to the events endpoint since we're going
  to be adding replays to EAP
- I need to go through all the attributes on replays still and figure
  out if any of them will have different public aliases etc. but for now
  I just want to get the base work up of allowing these queries
@wmak wmak requested a review from a team as a code owner November 26, 2025 23:39
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 26, 2025
Comment on lines +42 to +52
orderby=orderby,
offset=offset,
limit=limit,
referrer=referrer,
sampling_mode=sampling_mode,
page_token=page_token,
resolver=search_resolver or cls.get_resolver(params, config),
additional_queries=additional_queries,
),
params.debug,
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The Replays dataset, when queried for timeseries data, will raise an unhandled NotImplementedError().
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The Replays class, now exposed via RPC_DATASETS for the events endpoint, lacks implementations for run_timeseries_query(), run_trace_query(), and run_stats_query(). If a user queries organization_events_stats.py with dataset=replays and timeseries parameters, the system will attempt to call run_timeseries_query() on the Replays object, which raises an unhandled NotImplementedError(), resulting in a 500 error.

💡 Suggested Fix

Implement run_timeseries_query(), run_trace_query(), and run_stats_query() methods in the Replays class in src/sentry/snuba/replays.py to handle timeseries, trace, and stats queries, or explicitly prevent these query types for the Replays dataset.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/snuba/replays.py#L1-L52

Potential issue: The `Replays` class, now exposed via `RPC_DATASETS` for the events
endpoint, lacks implementations for `run_timeseries_query()`, `run_trace_query()`, and
`run_stats_query()`. If a user queries `organization_events_stats.py` with
`dataset=replays` and timeseries parameters, the system will attempt to call
`run_timeseries_query()` on the `Replays` object, which raises an unhandled
`NotImplementedError()`, resulting in a 500 error.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3917707

response = requests.post(
settings.SENTRY_SNUBA + "/tests/entities/replays/insert", json=[replays]
)
assert response.status_code == 200
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Non-EAP path wraps replays list creating nested array

The store_replays method parameter was renamed from replay (singular) to replays (plural), but the non-EAP path (is_eap=False) still wraps the input with json=[replays]. The original code expected a single item and wrapped it in a list. Now that the parameter name suggests a list, if someone passes a list like store_replays([item1, item2], is_eap=False), it creates a nested array [[item1, item2]] instead of [item1, item2], which would cause the API call to fail or behave incorrectly. This is inconsistent with the similar ReplaysAcceptanceTestCase.store_replays() which correctly uses json=replays without extra wrapping.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants