Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/sentry/search/eap/replays/attributes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from sentry.search.eap import constants
from sentry.search.eap.columns import (
ResolvedAttribute,
VirtualColumnDefinition,
project_context_constructor,
project_term_resolver,
)
from sentry.search.eap.common_columns import COMMON_COLUMNS
from sentry.utils.validators import is_event_id_or_list

# TODO(wmak): Most of this is copy paste from the other attribute definitions, just trying to get a quick POC
REPLAY_ATTRIBUTE_DEFINITIONS = {
column.public_alias: column
for column in COMMON_COLUMNS
+ [
ResolvedAttribute(
public_alias="id",
internal_name="sentry.item_id",
search_type="string",
validator=is_event_id_or_list,
),
]
}


REPLAY_VIRTUAL_CONTEXTS = {
key: VirtualColumnDefinition(
constructor=project_context_constructor(key),
term_resolver=project_term_resolver,
filter_column="project.id",
)
for key in constants.PROJECT_FIELDS
}
19 changes: 19 additions & 0 deletions src/sentry/search/eap/replays/definitions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType

from sentry.search.eap.columns import ColumnDefinitions
from sentry.search.eap.replays.attributes import (
REPLAY_ATTRIBUTE_DEFINITIONS,
REPLAY_VIRTUAL_CONTEXTS,
)

REPLAY_DEFINITIONS = ColumnDefinitions(
aggregates={},
conditional_aggregates={},
formulas={},
columns=REPLAY_ATTRIBUTE_DEFINITIONS,
contexts=REPLAY_VIRTUAL_CONTEXTS,
trace_item_type=TraceItemType.TRACE_ITEM_TYPE_REPLAY,
filter_aliases={},
column_to_alias=None,
alias_to_column=None,
)
52 changes: 52 additions & 0 deletions src/sentry/snuba/replays.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import logging

import sentry_sdk
from sentry_protos.snuba.v1.request_common_pb2 import PageToken

from sentry.search.eap.replays.definitions import REPLAY_DEFINITIONS
from sentry.search.eap.resolver import SearchResolver
from sentry.search.eap.types import AdditionalQueries, EAPResponse, SearchResolverConfig
from sentry.search.events.types import SAMPLING_MODES, SnubaParams
from sentry.snuba import rpc_dataset_common

logger = logging.getLogger(__name__)


class Replays(rpc_dataset_common.RPCBase):
DEFINITIONS = REPLAY_DEFINITIONS

@classmethod
@sentry_sdk.trace
def run_table_query(
cls,
*,
params: SnubaParams,
query_string: str,
selected_columns: list[str],
orderby: list[str] | None,
offset: int,
limit: int,
referrer: str,
config: SearchResolverConfig,
sampling_mode: SAMPLING_MODES | None = None,
equations: list[str] | None = None,
search_resolver: SearchResolver | None = None,
page_token: PageToken | None = None,
additional_queries: AdditionalQueries | None = None,
) -> EAPResponse:
return cls._run_table_query(
rpc_dataset_common.TableQuery(
query_string=query_string,
selected_columns=selected_columns,
equations=equations,
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,
)
Comment on lines +42 to +52
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

5 changes: 4 additions & 1 deletion src/sentry/snuba/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from sentry.snuba.models import QuerySubscription, SnubaQuery
from sentry.snuba.ourlogs import OurLogs
from sentry.snuba.profile_functions import ProfileFunctions
from sentry.snuba.replays import Replays
from sentry.snuba.spans_rpc import Spans
from sentry.snuba.trace_metrics import TraceMetrics
from sentry.snuba.uptime_results import UptimeResults
Expand All @@ -39,6 +40,7 @@
"issuePlatform": issue_platform,
"profileFunctions": functions,
"profile_functions": ProfileFunctions,
"replays": Replays,
"spans": Spans,
"spansIndexed": spans_indexed,
"spansMetrics": spans_metrics,
Expand All @@ -47,10 +49,11 @@
}
DEPRECATED_LABELS = {"ourlogs"}
RPC_DATASETS = {
OurLogs,
ProfileFunctions,
Replays,
Spans,
TraceMetrics,
OurLogs,
UptimeResults,
}
DATASET_LABELS = {
Expand Down
64 changes: 60 additions & 4 deletions src/sentry/testutils/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -2289,11 +2289,67 @@ def setUp(self):
super().setUp()
assert requests.post(settings.SENTRY_SNUBA + "/tests/replays/drop").status_code == 200

def store_replays(self, replay):
response = requests.post(
settings.SENTRY_SNUBA + "/tests/entities/replays/insert", json=[replay]
def create_replay(
self,
timestamp: datetime,
project: Project | None = None,
replay_id: str | None = None,
trace_id: str | None = None,
) -> TraceItem:
if project is None:
project = self.project
if replay_id is None:
replay_id = uuid4().hex
if trace_id is None:
trace_id = uuid4().hex
rpc_timestamp = Timestamp()
rpc_timestamp.FromDatetime(timestamp)
return TraceItem(
organization_id=project.organization.id,
project_id=project.id,
item_type=TraceItemType.TRACE_ITEM_TYPE_REPLAY,
timestamp=rpc_timestamp,
trace_id=trace_id,
item_id=int(replay_id, 16).to_bytes(
16,
byteorder="little",
signed=False,
),
received=rpc_timestamp,
retention_days=90,
attributes={},
client_sample_rate=1.0,
server_sample_rate=1.0,
)
assert response.status_code == 200

def store_replay(self, replay):
assert (
requests.post(
settings.SENTRY_SNUBA + EAP_ITEMS_INSERT_ENDPOINT,
files={"item_0": replay.SerializeToString()},
).status_code
== 200
)

def store_replays(self, replays, is_eap=False):
"""Trying to deprecate the single replay store functionality here with `store_replay` so when is_eap is True
this function will attempt to store multiple replays"""
if is_eap:
assert (
requests.post(
settings.SENTRY_SNUBA + EAP_ITEMS_INSERT_ENDPOINT,
files={
f"item_{index}": replay.SerializeToString()
for index, replay in enumerate(replays)
},
).status_code
== 200
)
else:
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


def mock_event_links(self, timestamp, project_id, level, replay_id, event_id):
event = self.store_event(
Expand Down
43 changes: 43 additions & 0 deletions tests/snuba/api/endpoints/test_organization_events_replays.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from uuid import uuid4

import pytest

from sentry.testutils.cases import ReplaysSnubaTestCase
from tests.snuba.api.endpoints.test_organization_events import OrganizationEventsEndpointTestBase


class OrganizationEventsReplaysEndpointTest(
OrganizationEventsEndpointTestBase, ReplaysSnubaTestCase
):
dataset = "replays"

def setUp(self) -> None:
super().setUp()
self.features = {
"organizations:session-replay": True,
}

@pytest.mark.querybuilder
def test_simple(self) -> None:
replay_ids = [uuid4().hex, uuid4().hex]
self.store_replays(
[
self.create_replay(self.ten_mins_ago, replay_id=replay_id)
for replay_id in replay_ids
],
is_eap=True,
)
response = self.do_request(
{
"field": ["id"],
"query": "",
"orderby": "-id",
"project": self.project.id,
"dataset": self.dataset,
}
)
assert response.status_code == 200, response.content
data = response.data["data"]
assert len(data) == 2
for replay_id in replay_ids:
assert replay_id in [row["id"] for row in data]
Loading