From 3ed1872e8230a75ee5d454938cf4bd9d0bfd3168 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 a7651a08acdfd2..ce9572ba3e6d2c 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 54a28515a1160c..026d2c02d0eeca 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