Skip to content

Commit

Permalink
feat(replay): add has_viewed to details and index (#68391)
Browse files Browse the repository at this point in the history
Relates to #64924. Splitting
#67972 into smaller PRs.

---------

Co-authored-by: Colton Allen <[email protected]>
  • Loading branch information
aliu39 and cmanallen authored Apr 8, 2024
1 parent 03e16cb commit 2b26437
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/sentry/apidocs/examples/replay_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"environment": "production",
"error_ids": ["7e07485f-12f9-416b-8b14-26260799b51f"],
"finished_at": "2022-07-07T14:15:33.201019",
"has_viewed": True,
"id": "7e07485f-12f9-416b-8b14-26260799b51f",
"is_archived": None,
"os": {"name": "iOS", "version": "16.2"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def get(self, request: Request, organization: Organization, replay_id: str) -> R
start=filter_params["start"],
end=filter_params["end"],
organization=organization,
request_user_id=request.user.id,
)

response = process_raw_response(
Expand Down
1 change: 1 addition & 0 deletions src/sentry/replays/endpoints/project_replay_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def get(self, request: Request, project: Project, replay_id: str) -> Response:
start=filter_params["start"],
end=filter_params["end"],
organization=project.organization,
request_user_id=request.user.id,
)

response = process_raw_response(
Expand Down
1 change: 1 addition & 0 deletions src/sentry/replays/lib/new_query/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Functions in this module coerce external types to internal types. Else they die.
"""

import uuid

from sentry.replays.lib.new_query.errors import CouldNotParseValue
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/replays/post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class ReplayDetailsResponse(TypedDict, total=False):
info_ids: list[str] | None
count_warnings: int | None
count_infos: int | None
has_viewed: bool


def process_raw_response(
Expand Down Expand Up @@ -181,6 +182,8 @@ def generate_normalized_output(
ret_item["info_ids"] = item.pop("info_ids", None)
ret_item["count_infos"] = item.pop("count_infos", None)
ret_item["count_warnings"] = item.pop("count_warnings", None)
# Returns a UInt8 of either 0 or 1. We coerce to a bool.
ret_item["has_viewed"] = bool(item.get("has_viewed", 0))
yield ret_item


Expand Down
35 changes: 33 additions & 2 deletions src/sentry/replays/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def query_replays_collection_raw(
project_ids=project_ids,
period_start=start,
period_stop=end,
request_user_id=actor.id if actor else None,
)


Expand All @@ -79,6 +80,7 @@ def query_replay_instance(
start: datetime,
end: datetime,
organization: Organization | None = None,
request_user_id: int | None = None,
):
"""Query aggregated replay instance."""
if isinstance(project_id, list):
Expand All @@ -93,6 +95,7 @@ def query_replay_instance(
project_ids=project_ids,
period_start=start,
period_end=end,
request_user_id=request_user_id,
),
tenant_id={"organization_id": organization.id} if organization else {},
referrer="replays.query.details_query",
Expand Down Expand Up @@ -556,6 +559,7 @@ def _empty_uuids_lambda():
"info_ids": ["info_ids"],
"count_warnings": ["count_warnings"],
"count_infos": ["count_infos"],
"has_viewed": ["has_viewed"],
}


Expand Down Expand Up @@ -754,9 +758,16 @@ def collect_aliases(fields: list[str]) -> list[str]:
return list(result)


def select_from_fields(fields: list[str]) -> list[Column | Function]:
def select_from_fields(fields: list[str], user_id: int | None) -> list[Column | Function]:
"""Return a list of columns to select."""
return [QUERY_ALIAS_COLUMN_MAP[alias] for alias in collect_aliases(fields)]
selection = []
for alias in collect_aliases(fields):
if alias == "has_viewed":
selection.append(compute_has_viewed(user_id))
else:
selection.append(QUERY_ALIAS_COLUMN_MAP[alias])

return selection


def _extract_children(expression: ParenExpression) -> Generator[SearchFilter, None, None]:
Expand All @@ -765,3 +776,23 @@ def _extract_children(expression: ParenExpression) -> Generator[SearchFilter, No
yield child
elif isinstance(child, ParenExpression):
yield from _extract_children(child)


def compute_has_viewed(viewed_by_id: int | None) -> Function:
if viewed_by_id is None:
# Return the literal "false" if no user-id was specified.
return Function("equals", parameters=[1, 2])

return Function(
"greater",
parameters=[
Function(
"sum",
parameters=[
Function("equals", parameters=[Column("viewed_by_id"), viewed_by_id]),
],
),
0,
],
alias="has_viewed",
)
26 changes: 26 additions & 0 deletions src/sentry/replays/testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def mock_expected_response(
"count_errors": kwargs.pop("count_errors", 0),
"count_warnings": kwargs.pop("count_warnings", 0),
"count_infos": kwargs.pop("count_infos", 0),
"has_viewed": kwargs.pop("has_viewed", False),
}


Expand Down Expand Up @@ -252,6 +253,31 @@ def mock_replay_click(
}


def mock_replay_viewed(
timestamp: float,
project_id: str,
replay_id: str,
viewed_by_id: int,
retention_days: int = 30,
) -> dict[str, Any]:
return {
"type": "replay_event",
"start_time": int(timestamp),
"replay_id": replay_id,
"project_id": project_id,
"retention_days": retention_days,
"payload": list(
json.dumps(
{
"type": "replay_viewed",
"timestamp": timestamp,
"viewed_by_id": viewed_by_id,
}
).encode()
),
}


def mock_segment_init(
timestamp: datetime.datetime,
href: str = "http://localhost/",
Expand Down
9 changes: 6 additions & 3 deletions src/sentry/replays/usecases/query/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def query_using_optimized_search(
project_ids: list[int],
period_start: datetime,
period_stop: datetime,
request_user_id: int | None = None,
):
tenant_id = _make_tenant_id(organization)

Expand Down Expand Up @@ -212,6 +213,7 @@ def query_using_optimized_search(
project_ids=project_ids,
period_start=period_start,
period_end=period_stop,
request_user_id=request_user_id,
),
tenant_id,
referrer="replays.query.browse_query",
Expand Down Expand Up @@ -285,15 +287,16 @@ def make_full_aggregation_query(
project_ids: list[int],
period_start: datetime,
period_end: datetime,
request_user_id: int | None,
) -> Query:
"""Return a query to fetch every replay in the set."""
from sentry.replays.query import QUERY_ALIAS_COLUMN_MAP, select_from_fields
from sentry.replays.query import QUERY_ALIAS_COLUMN_MAP, compute_has_viewed, select_from_fields

def _select_from_fields() -> list[Column | Function]:
if fields:
return select_from_fields(list(set(fields)))
return select_from_fields(list(set(fields)), user_id=request_user_id)
else:
return list(QUERY_ALIAS_COLUMN_MAP.values())
return list(QUERY_ALIAS_COLUMN_MAP.values()) + [compute_has_viewed(request_user_id)]

return Query(
match=Entity("replays"),
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/replays/usecases/query/conditions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"SumOfClickScalar",
"SumOfClickSelectorComposite",
"SumOfDeadClickSelectorComposite",
"SumOfIntegerIdScalar",
"SumOfIPv4Scalar",
"SumOfRageClickSelectorComposite",
"SumOfStringArray",
Expand All @@ -22,7 +23,13 @@


from .activity import AggregateActivityScalar
from .aggregate import SumOfIPv4Scalar, SumOfStringArray, SumOfStringScalar, SumOfUUIDArray
from .aggregate import (
SumOfIntegerIdScalar,
SumOfIPv4Scalar,
SumOfStringArray,
SumOfStringScalar,
SumOfUUIDArray,
)
from .duration import SimpleAggregateDurationScalar
from .error_ids import ErrorIdsArray
from .selector import (
Expand Down
20 changes: 20 additions & 0 deletions src/sentry/replays/usecases/query/conditions/aggregate.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
aggregation of the row-wise condition visitor is 0 or not 0. This works because we're looking
for presence in the set; not for conformity across all rows.
"""

from __future__ import annotations

from uuid import UUID
Expand All @@ -29,6 +30,7 @@

from sentry.replays.lib.new_query.conditions import (
GenericBase,
IntegerScalar,
IPv4Scalar,
StringArray,
StringScalar,
Expand All @@ -38,6 +40,24 @@
from sentry.replays.lib.new_query.utils import contains, does_not_contain


class SumOfIntegerIdScalar(GenericBase):
@staticmethod
def visit_eq(expression: Expression, value: int) -> Condition:
return contains(IntegerScalar.visit_eq(expression, value))

@staticmethod
def visit_neq(expression: Expression, value: int) -> Condition:
return does_not_contain(IntegerScalar.visit_eq(expression, value))

@staticmethod
def visit_in(expression: Expression, value: list[int]) -> Condition:
return contains(IntegerScalar.visit_in(expression, value))

@staticmethod
def visit_not_in(expression: Expression, value: list[int]) -> Condition:
return does_not_contain(IntegerScalar.visit_in(expression, value))


class SumOfIPv4Scalar(GenericBase):
@staticmethod
def visit_eq(expression: Expression, value: str) -> Condition:
Expand Down
14 changes: 9 additions & 5 deletions src/sentry/replays/usecases/query/configs/aggregate.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
must parse to the data type of its source even if its later transformed into another type. This
acts as a validation step as must as a type coercion step.
"""

from __future__ import annotations

from sentry.replays.lib.new_query.conditions import IntegerScalar, UUIDScalar
from sentry.replays.lib.new_query.fields import (
ColumnField,
CountField,
FieldProtocol,
IntegerColumnField,
StringColumnField,
SumField,
SumLengthField,
Expand All @@ -31,6 +33,7 @@
SumOfClickScalar,
SumOfClickSelectorComposite,
SumOfDeadClickSelectorComposite,
SumOfIntegerIdScalar,
SumOfIPv4Scalar,
SumOfRageClickSelectorComposite,
SumOfStringArray,
Expand Down Expand Up @@ -86,12 +89,12 @@ def array_string_field(column_name: str) -> StringColumnField:
"click.textContent": click_field("click_text"),
"click.title": click_field("click_title"),
"count_dead_clicks": sum_field("click_is_dead"),
"count_infos": sum_field("count_info_events"),
"count_warnings": sum_field("count_warning_events"),
"count_errors": sum_field("count_error_events"),
"count_infos": sum_field("count_info_events"),
"count_rage_clicks": sum_field("click_is_rage"),
"count_segments": count_field("segment_id"),
"count_urls": sum_field("count_urls"),
"count_warnings": sum_field("count_warning_events"),
"dead.selector": ComputedField(parse_selector, SumOfDeadClickSelectorComposite),
"device.brand": string_field("device_brand"),
"device.family": string_field("device_family"),
Expand All @@ -101,11 +104,10 @@ def array_string_field(column_name: str) -> StringColumnField:
"duration": ComputedField(parse_int, SimpleAggregateDurationScalar),
"environment": string_field("environment"),
"error_ids": ComputedField(parse_uuid, SumOfErrorIdScalar),
"warning_ids": UUIDColumnField("warning_id", parse_uuid, SumOfUUIDScalar),
"info_ids": ComputedField(parse_uuid, SumOfInfoIdScalar),
# Backwards Compat: We pass a simple string to the UUID column. Older versions of ClickHouse
# do not understand the UUID type.
"id": ColumnField("replay_id", parse_uuid, UUIDScalar),
"info_ids": ComputedField(parse_uuid, SumOfInfoIdScalar),
"os.name": string_field("os_name"),
"os.version": string_field("os_version"),
"platform": string_field("platform"),
Expand All @@ -120,6 +122,8 @@ def array_string_field(column_name: str) -> StringColumnField:
"user.id": string_field("user_id"),
"user.ip_address": StringColumnField("ip_address_v4", parse_str, SumOfIPv4Scalar),
"user.username": string_field("user_name"),
"viewed_by_id": IntegerColumnField("viewed_by_id", parse_int, SumOfIntegerIdScalar),
"warning_ids": UUIDColumnField("warning_id", parse_uuid, SumOfUUIDScalar),
}


Expand All @@ -136,8 +140,8 @@ def array_string_field(column_name: str) -> StringColumnField:
# Fields which have multiple names that represent the same search operation are defined here.
# QQ:JFERG: why dont we have these on the scalar search
search_config["error_id"] = search_config["error_ids"]
search_config["warning_id"] = search_config["warning_ids"]
search_config["info_id"] = search_config["info_ids"]
search_config["warning_id"] = search_config["warning_ids"]


search_config["release"] = search_config["releases"]
Expand Down
Loading

0 comments on commit 2b26437

Please sign in to comment.