From 1f2e0c263fa4157a38a07ef5a8641c5f1fe80030 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 15 Mar 2024 11:30:25 -0500 Subject: [PATCH] fix(replays): Drop and log click events with negative node-ids (#66993) 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). --- .../replays/usecases/ingest/dom_index.py | 23 +++++++++-- .../replays/unit/test_ingest_dom_index.py | 40 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index a7651a08acdfd..ce9572ba3e6d2 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -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: @@ -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], @@ -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] @@ -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") @@ -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 diff --git a/tests/sentry/replays/unit/test_ingest_dom_index.py b/tests/sentry/replays/unit/test_ingest_dom_index.py index 54a28515a1160..026d2c02d0eec 100644 --- a/tests/sentry/replays/unit/test_ingest_dom_index.py +++ b/tests/sentry/replays/unit/test_ingest_dom_index.py @@ -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