Skip to content

Commit

Permalink
fix(replays): Drop and log click events with negative node-ids (#66993)
Browse files Browse the repository at this point in the history
Negative node-ids can't be inserted into our database because of type
issues. Let's drop them for now. We'll also log them so we can figure
out if they're important (and possibly debug an SDK issue).
  • Loading branch information
cmanallen committed Mar 15, 2024
1 parent f6cb475 commit 3ed1872
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 3 deletions.
23 changes: 20 additions & 3 deletions src/sentry/replays/usecases/ingest/dom_index.py
Expand Up @@ -231,6 +231,7 @@ def create_click_event(
replay_id: str,
is_dead: bool,
is_rage: bool,
project_id: int,
) -> ReplayActionsEventPayloadClick | None:
node = payload.get("data", {}).get("node")
if node is None:
Expand All @@ -242,7 +243,7 @@ def create_click_event(
# before truncating the list.
classes = _parse_classes(attributes.get("class", ""))

return {
event: ReplayActionsEventPayloadClick = {
"node_id": node["id"],
"tag": node["tagName"][:32],
"id": attributes.get("id", "")[:64],
Expand All @@ -262,6 +263,18 @@ def create_click_event(
),
}

# This is unsupported and will cause errors on insert! Let's drop these bad clicks
# and logs them to bigquery to see if we can figure out where they're coming from.
if event["node_id"] < 0:
# Log to "slow_click" because its the only bigquery sink
logger.info(
"sentry.replays.slow_click",
extra={"event_type": "negative-click-node-id", "project_id": project_id, "data": event},
)
return None

return event


def _parse_classes(classes: str) -> list[str]:
return list(filter(lambda n: n != "", classes.split(" ")))[:10]
Expand Down Expand Up @@ -383,7 +396,9 @@ def _handle_breadcrumb(
is_rage = (
payload["data"].get("clickCount", 0) or payload["data"].get("clickcount", 0)
) >= 5
click = create_click_event(payload, replay_id, is_dead=True, is_rage=is_rage)
click = create_click_event(
payload, replay_id, is_dead=True, is_rage=is_rage, project_id=project_id
)
if click is not None:
if is_rage:
metrics.incr("replay.rage_click_detected")
Expand Down Expand Up @@ -417,7 +432,9 @@ def _handle_breadcrumb(
log["dom_tree"] = log.pop("message")
logger.info("sentry.replays.slow_click", extra=log)
elif category == "ui.click":
click = create_click_event(payload, replay_id, is_dead=False, is_rage=False)
click = create_click_event(
payload, replay_id, is_dead=False, is_rage=False, project_id=project_id
)
if click is not None:
return click
return None
40 changes: 40 additions & 0 deletions tests/sentry/replays/unit/test_ingest_dom_index.py
Expand Up @@ -1098,3 +1098,43 @@ def test_log_canvas_size():

# No events.
log_canvas_size(1, 1, "a", [])


def test_emit_click_negative_node_id():
"""Test "get_user_actions" function."""
events = [
{
"type": 5,
"timestamp": 1674298825,
"data": {
"tag": "breadcrumb",
"payload": {
"timestamp": 1674298825.403,
"type": "default",
"category": "ui.click",
"message": "div#hello.hello.world",
"data": {
"nodeId": 1,
"node": {
"id": -1,
"tagName": "div",
"attributes": {
"id": "hello",
"class": "hello world",
"aria-label": "test",
"role": "button",
"alt": "1",
"data-testid": "2",
"title": "3",
"data-sentry-component": "SignUpForm",
},
"textContent": "Hello, world!",
},
},
},
},
}
]

user_actions = get_user_actions(1, uuid.uuid4().hex, events, None)
assert len(user_actions) == 0

0 comments on commit 3ed1872

Please sign in to comment.