From 0e671ca1791dbb36b451cbfef7125cd60a863fc5 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:17:01 -0700 Subject: [PATCH 01/13] perf(replay): reduce postgres queries in recording consumer using get_from_cache --- .../replays/consumers/recording_buffered.py | 4 +- .../replays/usecases/ingest/__init__.py | 3 +- .../replays/usecases/ingest/dom_index.py | 71 ++++++++----------- 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/sentry/replays/consumers/recording_buffered.py b/src/sentry/replays/consumers/recording_buffered.py index a0844ac38674e6..e2c6d17556d935 100644 --- a/src/sentry/replays/consumers/recording_buffered.py +++ b/src/sentry/replays/consumers/recording_buffered.py @@ -55,6 +55,7 @@ from sentry_kafka_schemas.schema_types.ingest_replay_recordings_v1 import ReplayRecording from sentry.conf.types.kafka_definition import Topic, get_topic_codec +from sentry.models.project import Project from sentry.replays.lib.storage import ( RecordingSegmentStorageMeta, make_recording_filename, @@ -294,8 +295,9 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: else None ) + project = Project.objects.get_from_cache(decoded_message["project_id"]) replay_actions = parse_replay_actions( - decoded_message["project_id"], + project, decoded_message["replay_id"], decoded_message["retention_days"], parsed_recording_data, diff --git a/src/sentry/replays/usecases/ingest/__init__.py b/src/sentry/replays/usecases/ingest/__init__.py index 882aede4e14b00..5b631e12ecb3cc 100644 --- a/src/sentry/replays/usecases/ingest/__init__.py +++ b/src/sentry/replays/usecases/ingest/__init__.py @@ -275,9 +275,10 @@ def recording_post_processor( op="replays.usecases.ingest.parse_and_emit_replay_actions", description="parse_and_emit_replay_actions", ): + project = Project.objects.get_from_cache(id=message.project_id) parse_and_emit_replay_actions( retention_days=message.retention_days, - project_id=message.project_id, + project=project, replay_id=message.replay_id, segment_data=parsed_segment_data, replay_event=parsed_replay_event, diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index 93c0da0c7793a4..4b418f3615a6ec 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -62,7 +62,7 @@ class ReplayActionsEvent(TypedDict): def parse_and_emit_replay_actions( - project_id: int, + project: Project, replay_id: str, retention_days: int, segment_data: list[dict[str, Any]], @@ -70,7 +70,7 @@ def parse_and_emit_replay_actions( ) -> None: with metrics.timer("replays.usecases.ingest.dom_index.parse_and_emit_replay_actions"): message = parse_replay_actions( - project_id, replay_id, retention_days, segment_data, replay_event + project, replay_id, retention_days, segment_data, replay_event ) if message is not None: emit_replay_actions(message) @@ -82,19 +82,19 @@ def emit_replay_actions(action: ReplayActionsEvent) -> None: def parse_replay_actions( - project_id: int, + project: Project, replay_id: str, retention_days: int, segment_data: list[dict[str, Any]], replay_event: dict[str, Any] | None, ) -> ReplayActionsEvent | None: """Parse RRWeb payload to ReplayActionsEvent.""" - actions = get_user_actions(project_id, replay_id, segment_data, replay_event) + actions = get_user_actions(project, replay_id, segment_data, replay_event) if len(actions) == 0: return None payload = create_replay_actions_payload(replay_id, actions) - return create_replay_actions_event(replay_id, project_id, retention_days, payload) + return create_replay_actions_event(replay_id, project.id, retention_days, payload) def create_replay_actions_event( @@ -153,7 +153,7 @@ def log_canvas_size( def get_user_actions( - project_id: int, + project: Project, replay_id: str, events: list[dict[str, Any]], replay_event: dict[str, Any] | None, @@ -185,7 +185,7 @@ def get_user_actions( tag = event.get("data", {}).get("tag") if tag == "breadcrumb": - click = _handle_breadcrumb(event, project_id, replay_id, replay_event) + click = _handle_breadcrumb(event, project, replay_id, replay_event) if click is not None: result.append(click) # look for request / response breadcrumbs and report metrics on them @@ -193,7 +193,7 @@ def get_user_actions( _handle_resource_metric_event(event) # log the SDK options sent from the SDK 1/500 times if tag == "options" and random.randint(0, 499) < 1: - _handle_options_logging_event(project_id, replay_id, event) + _handle_options_logging_event(project.id, replay_id, event) # log large dom mutation breadcrumb events 1/100 times payload = event.get("data", {}).get("payload", {}) @@ -203,7 +203,7 @@ def get_user_actions( and payload.get("category") == "replay.mutations" and random.randint(0, 500) < 1 ): - _handle_mutations_event(project_id, replay_id, event) + _handle_mutations_event(project.id, replay_id, event) return result @@ -287,8 +287,7 @@ def _parse_classes(classes: str) -> list[str]: return list(filter(lambda n: n != "", classes.split(" ")))[:10] -def _should_report_hydration_error_issue(project_id: int) -> bool: - project = Project.objects.get(id=project_id) +def _should_report_hydration_error_issue(project: Project) -> bool: """ The feature is controlled by Sentry admins for release of the feature, while the project option is controlled by the project owner, and is a @@ -300,27 +299,16 @@ def _should_report_hydration_error_issue(project_id: int) -> bool: ) and project.get_option("sentry:replay_hydration_error_issues") -def _should_report_rage_click_issue(project_id: int) -> bool: - project = Project.objects.get(id=project_id) - - def _project_has_feature_enabled() -> bool: - """ - Check if the project has the feature flag enabled, - This is controlled by Sentry admins for release of the feature - """ - return features.has( - "organizations:session-replay-rage-click-issue-creation", - project.organization, - ) - - def _project_has_option_enabled() -> bool: - """ - Check if the project has the option enabled, - This is controlled by the project owner, and is a permanent setting - """ - return project.get_option("sentry:replay_rage_click_issues") - - return all([_project_has_feature_enabled(), _project_has_option_enabled()]) +def _should_report_rage_click_issue(project: Project) -> bool: + """ + The feature is controlled by Sentry admins for release of the feature, + while the project option is controlled by the project owner, and is a + permanent setting + """ + return features.has( + "organizations:session-replay-rage-click-issue-creation", + project.organization, + ) and project.get_option("sentry:replay_rage_click_issues") def _iter_custom_events(events: list[dict[str, Any]]) -> Generator[dict[str, Any]]: @@ -392,7 +380,10 @@ def _handle_mutations_event(project_id: int, replay_id: str, event: dict[str, An def _handle_breadcrumb( - event: dict[str, Any], project_id: int, replay_id: str, replay_event: dict[str, Any] | None + event: dict[str, Any], + project: Project, + replay_id: str, + replay_event: dict[str, Any] | None, ) -> ReplayActionsEventPayloadClick | None: click = None @@ -417,15 +408,15 @@ def _handle_breadcrumb( 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, project_id=project_id + 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") - if _should_report_rage_click_issue(project_id): + if _should_report_rage_click_issue(project): if replay_event is not None: report_rage_click_issue_with_replay_event( - project_id, + project.id, replay_id, payload["timestamp"], payload["message"], @@ -436,7 +427,7 @@ def _handle_breadcrumb( ) # Log the event for tracking. log = event["data"].get("payload", {}).copy() - log["project_id"] = project_id + log["project_id"] = project.id log["replay_id"] = replay_id log["dom_tree"] = log.pop("message") @@ -444,16 +435,16 @@ def _handle_breadcrumb( elif category == "ui.click": click = create_click_event( - payload, replay_id, is_dead=False, is_rage=False, project_id=project_id + payload, replay_id, is_dead=False, is_rage=False, project_id=project.id ) if click is not None: return click elif category == "replay.hydrate-error": metrics.incr("replay.hydration_error_breadcrumb") - if replay_event is not None and _should_report_hydration_error_issue(project_id): + if replay_event is not None and _should_report_hydration_error_issue(project): report_hydration_error_issue_with_replay_event( - project_id, + project.id, replay_id, payload["timestamp"], payload.get("data", {}).get("url"), From 3c66aedfd9762a6f94a54ec796b0298f7868905a Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 13 Sep 2024 14:04:01 -0700 Subject: [PATCH 02/13] Move should_report queries up to get_user_actions --- .../replays/usecases/ingest/dom_index.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index 4b418f3615a6ec..f21d73f4ab4a68 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -177,6 +177,10 @@ def get_user_actions( "textContent": "Helloworld!" } """ + # Feature flag and project option queries + should_report_rage_click_issue = _should_report_rage_click_issue(project) + should_report_hydration_error_issue = _should_report_hydration_error_issue(project) + result: list[ReplayActionsEventPayloadClick] = [] for event in _iter_custom_events(events): if len(result) == 20: @@ -185,7 +189,14 @@ def get_user_actions( tag = event.get("data", {}).get("tag") if tag == "breadcrumb": - click = _handle_breadcrumb(event, project, replay_id, replay_event) + click = _handle_breadcrumb( + event, + project, + replay_id, + replay_event, + should_report_rage_click_issue=should_report_rage_click_issue, + should_report_hydration_error_issue=should_report_hydration_error_issue, + ) if click is not None: result.append(click) # look for request / response breadcrumbs and report metrics on them @@ -384,6 +395,8 @@ def _handle_breadcrumb( project: Project, replay_id: str, replay_event: dict[str, Any] | None, + should_report_rage_click_issue=False, + should_report_hydration_error_issue=False, ) -> ReplayActionsEventPayloadClick | None: click = None @@ -413,7 +426,7 @@ def _handle_breadcrumb( if click is not None: if is_rage: metrics.incr("replay.rage_click_detected") - if _should_report_rage_click_issue(project): + if should_report_rage_click_issue: if replay_event is not None: report_rage_click_issue_with_replay_event( project.id, @@ -442,7 +455,7 @@ def _handle_breadcrumb( elif category == "replay.hydrate-error": metrics.incr("replay.hydration_error_breadcrumb") - if replay_event is not None and _should_report_hydration_error_issue(project): + if replay_event is not None and should_report_hydration_error_issue: report_hydration_error_issue_with_replay_event( project.id, replay_id, From 5ba752c9f9c9e017937e51074b498233d3001f33 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 13 Sep 2024 14:05:37 -0700 Subject: [PATCH 03/13] Shorten var names --- src/sentry/replays/usecases/ingest/dom_index.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index f21d73f4ab4a68..e9a7894093e7b3 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -178,8 +178,8 @@ def get_user_actions( } """ # Feature flag and project option queries - should_report_rage_click_issue = _should_report_rage_click_issue(project) - should_report_hydration_error_issue = _should_report_hydration_error_issue(project) + should_report_rage = _should_report_rage_click_issue(project) + should_report_hydration = _should_report_hydration_error_issue(project) result: list[ReplayActionsEventPayloadClick] = [] for event in _iter_custom_events(events): @@ -194,8 +194,8 @@ def get_user_actions( project, replay_id, replay_event, - should_report_rage_click_issue=should_report_rage_click_issue, - should_report_hydration_error_issue=should_report_hydration_error_issue, + should_report_rage_click_issue=should_report_rage, + should_report_hydration_error_issue=should_report_hydration, ) if click is not None: result.append(click) From f3e6ce6e55600d0a53343fe65dea94b6b8c96384 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:14:58 -0700 Subject: [PATCH 04/13] Update unit test param type --- .../replays/unit/test_ingest_dom_index.py | 76 ++++++++++--------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/tests/sentry/replays/unit/test_ingest_dom_index.py b/tests/sentry/replays/unit/test_ingest_dom_index.py index 026d2c02d0eeca..acc762b2b987ee 100644 --- a/tests/sentry/replays/unit/test_ingest_dom_index.py +++ b/tests/sentry/replays/unit/test_ingest_dom_index.py @@ -3,9 +3,11 @@ import uuid from typing import Any from unittest import mock +from unittest.mock import Mock import pytest +from sentry.models.project import Project from sentry.replays.testutils import mock_replay_event from sentry.replays.usecases.ingest.dom_index import ( _get_testid, @@ -28,7 +30,15 @@ def patch_rage_click_issue_with_replay_event(): yield m -def test_get_user_actions(): +@pytest.fixture(autouse=True) +def mock_project() -> Project: + """Has id=1. Use for unit tests so we can skip @django_db""" + proj = Mock(spec=Project) + proj.id = 1 + return proj + + +def test_get_user_actions(mock_project): """Test "get_user_actions" function.""" events = [ { @@ -64,7 +74,7 @@ def test_get_user_actions(): } ] - user_actions = get_user_actions(1, uuid.uuid4().hex, events, None) + user_actions = get_user_actions(mock_project, uuid.uuid4().hex, events, None) assert len(user_actions) == 1 assert user_actions[0]["node_id"] == 1 assert user_actions[0]["tag"] == "div" @@ -83,7 +93,7 @@ def test_get_user_actions(): assert len(user_actions[0]["event_hash"]) == 36 -def test_get_user_actions_str_payload(): +def test_get_user_actions_str_payload(mock_project): """Test "get_user_actions" function.""" events = [ { @@ -96,11 +106,11 @@ def test_get_user_actions_str_payload(): } ] - user_actions = get_user_actions(1, uuid.uuid4().hex, events, None) + user_actions = get_user_actions(mock_project, uuid.uuid4().hex, events, None) assert len(user_actions) == 0 -def test_get_user_actions_missing_node(): +def test_get_user_actions_missing_node(mock_project): """Test "get_user_actions" function.""" events = [ { @@ -118,11 +128,11 @@ def test_get_user_actions_missing_node(): } ] - user_actions = get_user_actions(1, uuid.uuid4().hex, events, None) + user_actions = get_user_actions(mock_project, uuid.uuid4().hex, events, None) assert len(user_actions) == 0 -def test_get_user_actions_performance_spans(): +def test_get_user_actions_performance_spans(mock_project): """Test that "get_user_actions" doesn't error when collecting rsrc metrics, on various formats of performanceSpan""" # payloads are not realistic examples - only include the fields necessary for testing # TODO: does not test if metrics.distribution() is called downstream, with correct param types. @@ -194,10 +204,10 @@ def test_get_user_actions_performance_spans(): }, }, ] - get_user_actions(1, uuid.uuid4().hex, events, None) + get_user_actions(mock_project, uuid.uuid4().hex, events, None) -def test_parse_replay_actions(): +def test_parse_replay_actions(mock_project): events = [ { "type": 5, @@ -232,7 +242,7 @@ def test_parse_replay_actions(): }, } ] - replay_actions = parse_replay_actions(1, "1", 30, events, None) + replay_actions = parse_replay_actions(mock_project, "1", 30, events, None) assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -381,9 +391,7 @@ def test_parse_replay_dead_click_actions(patch_rage_click_issue_with_replay_even } ): default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions( - default_project.id, "1", 30, events, mock_replay_event() - ) + replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -541,7 +549,7 @@ def test_rage_click_issue_creation_no_component_name( } ): default_project.update_option("sentry:replay_rage_click_issues", True) - parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) + parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) # test that 2 rage click issues are still created assert patch_rage_click_issue_with_replay_event.call_count == 2 @@ -588,7 +596,7 @@ def test_parse_replay_click_actions_not_dead( } ] - replay_actions = parse_replay_actions(default_project.id, "1", 30, events, None) + replay_actions = parse_replay_actions(default_project, "1", 30, events, None) assert patch_rage_click_issue_with_replay_event.delay.call_count == 0 assert replay_actions is None @@ -632,7 +640,7 @@ def test_parse_replay_rage_click_actions(default_project): }, } ] - replay_actions = parse_replay_actions(default_project.id, "1", 30, events, None) + replay_actions = parse_replay_actions(default_project, "1", 30, events, None) assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -672,7 +680,7 @@ def test_encode_as_uuid(): assert isinstance(uuid.UUID(a), uuid.UUID) -def test_parse_request_response_latest(): +def test_parse_request_response_latest(mock_project): events = [ { "type": 5, @@ -711,14 +719,14 @@ def test_parse_request_response_latest(): } ] with mock.patch("sentry.utils.metrics.distribution") as timing: - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert timing.call_args_list == [ mock.call("replays.usecases.ingest.request_body_size", 2949, unit="byte"), mock.call("replays.usecases.ingest.response_body_size", 94, unit="byte"), ] -def test_parse_request_response_no_info(): +def test_parse_request_response_no_info(mock_project): events = [ { "type": 5, @@ -739,11 +747,11 @@ def test_parse_request_response_no_info(): }, }, ] - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) # just make sure we don't raise -def test_parse_request_response_old_format_request_only(): +def test_parse_request_response_old_format_request_only(mock_project): events = [ { "type": 5, @@ -766,13 +774,13 @@ def test_parse_request_response_old_format_request_only(): }, ] with mock.patch("sentry.utils.metrics.distribution") as timing: - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert timing.call_args_list == [ mock.call("replays.usecases.ingest.request_body_size", 1002, unit="byte"), ] -def test_parse_request_response_old_format_response_only(): +def test_parse_request_response_old_format_response_only(mock_project): events = [ { "type": 5, @@ -794,13 +802,13 @@ def test_parse_request_response_old_format_response_only(): }, ] with mock.patch("sentry.utils.metrics.distribution") as timing: - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert timing.call_args_list == [ mock.call("replays.usecases.ingest.response_body_size", 1002, unit="byte"), ] -def test_parse_request_response_old_format_request_and_response(): +def test_parse_request_response_old_format_request_and_response(mock_project): events = [ { "type": 5, @@ -823,7 +831,7 @@ def test_parse_request_response_old_format_request_and_response(): }, ] with mock.patch("sentry.utils.metrics.distribution") as timing: - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert timing.call_args_list == [ mock.call("replays.usecases.ingest.request_body_size", 1002, unit="byte"), mock.call("replays.usecases.ingest.response_body_size", 8001, unit="byte"), @@ -948,9 +956,7 @@ def test_parse_replay_rage_clicks_with_replay_event( } ): default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions( - default_project.id, "1", 30, events, mock_replay_event() - ) + replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -961,7 +967,7 @@ def test_parse_replay_rage_clicks_with_replay_event( assert isinstance(replay_actions["payload"], list) -def test_log_sdk_options(): +def test_log_sdk_options(mock_project): events: list[dict[str, Any]] = [ { "data": { @@ -993,11 +999,11 @@ def test_log_sdk_options(): mock.patch("random.randint") as randint, ): randint.return_value = 0 - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert logger.info.call_args_list == [mock.call("sentry.replays.slow_click", extra=log)] -def test_log_large_dom_mutations(): +def test_log_large_dom_mutations(mock_project): events: list[dict[str, Any]] = [ { "type": 5, @@ -1023,7 +1029,7 @@ def test_log_large_dom_mutations(): mock.patch("random.randint") as randint, ): randint.return_value = 0 - parse_replay_actions(1, "1", 30, events, None) + parse_replay_actions(mock_project, "1", 30, events, None) assert logger.info.call_args_list == [mock.call("Large DOM Mutations List:", extra=log)] @@ -1100,7 +1106,7 @@ def test_log_canvas_size(): log_canvas_size(1, 1, "a", []) -def test_emit_click_negative_node_id(): +def test_emit_click_negative_node_id(mock_project): """Test "get_user_actions" function.""" events = [ { @@ -1136,5 +1142,5 @@ def test_emit_click_negative_node_id(): } ] - user_actions = get_user_actions(1, uuid.uuid4().hex, events, None) + user_actions = get_user_actions(mock_project, uuid.uuid4().hex, events, None) assert len(user_actions) == 0 From 670f08e0304271923051f606375749b973b7b365 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:35:23 -0700 Subject: [PATCH 05/13] Use kwarg in recording_buffered --- src/sentry/replays/consumers/recording_buffered.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/consumers/recording_buffered.py b/src/sentry/replays/consumers/recording_buffered.py index 89733757555c87..f2dad09323557c 100644 --- a/src/sentry/replays/consumers/recording_buffered.py +++ b/src/sentry/replays/consumers/recording_buffered.py @@ -332,7 +332,7 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: else None ) - project = Project.objects.get_from_cache(decoded_message["project_id"]) + project = Project.objects.get_from_cache(id=decoded_message["project_id"]) replay_actions = parse_replay_actions( project, decoded_message["replay_id"], From d98c2dc306a3298ffdc43c0e8c5caeec95c9456e Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:05:16 -0700 Subject: [PATCH 06/13] select_related on organization --- src/sentry/replays/consumers/recording_buffered.py | 4 +++- src/sentry/replays/usecases/ingest/__init__.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sentry/replays/consumers/recording_buffered.py b/src/sentry/replays/consumers/recording_buffered.py index f2dad09323557c..c5e33f56847dc3 100644 --- a/src/sentry/replays/consumers/recording_buffered.py +++ b/src/sentry/replays/consumers/recording_buffered.py @@ -332,7 +332,9 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: else None ) - project = Project.objects.get_from_cache(id=decoded_message["project_id"]) + project = Project.objects.select_related("organization").get_from_cache( + id=decoded_message["project_id"] + ) replay_actions = parse_replay_actions( project, decoded_message["replay_id"], diff --git a/src/sentry/replays/usecases/ingest/__init__.py b/src/sentry/replays/usecases/ingest/__init__.py index 029357deb87a15..00516248a39e89 100644 --- a/src/sentry/replays/usecases/ingest/__init__.py +++ b/src/sentry/replays/usecases/ingest/__init__.py @@ -270,7 +270,9 @@ def recording_post_processor( op="replays.usecases.ingest.parse_and_emit_replay_actions", description="parse_and_emit_replay_actions", ): - project = Project.objects.get_from_cache(id=message.project_id) + project = Project.objects.select_related("organization").get_from_cache( + id=message.project_id + ) parse_and_emit_replay_actions( retention_days=message.retention_days, project=project, From 1f0524f7d01a7446d452eea151df95d825a7fd35 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:25:07 -0700 Subject: [PATCH 07/13] Update should_report docstrings --- src/sentry/replays/usecases/ingest/dom_index.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index 3fe7334fb6c8b7..1eb4c77c429516 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -300,9 +300,8 @@ def _parse_classes(classes: str) -> list[str]: def _should_report_hydration_error_issue(project: Project) -> bool: """ - The feature is controlled by Sentry admins for release of the feature, - while the project option is controlled by the project owner, and is a - permanent setting + Checks the feature that's controlled by Sentry admins for release of the feature, + and the permanent project option, controlled by the project owner. """ return features.has( "organizations:session-replay-hydration-error-issue-creation", @@ -313,9 +312,7 @@ def _should_report_hydration_error_issue(project: Project) -> bool: <<<<<<< aliu/reduce-dom-index-queries def _should_report_rage_click_issue(project: Project) -> bool: """ - The feature is controlled by Sentry admins for release of the feature, - while the project option is controlled by the project owner, and is a - permanent setting + Checks the project option, controlled by a project owner. """ return features.has( "organizations:session-replay-rage-click-issue-creation", From 1bd3ce8c7b2230586c4acd516ed2fe1273165233 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:27:39 -0700 Subject: [PATCH 08/13] Fix merge --- .../replays/usecases/ingest/dom_index.py | 10 +--------- .../replays/unit/test_ingest_dom_index.py | 20 ------------------- 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index 1eb4c77c429516..a8d31ca67a2f48 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -309,20 +309,12 @@ def _should_report_hydration_error_issue(project: Project) -> bool: ) and project.get_option("sentry:replay_hydration_error_issues") -<<<<<<< aliu/reduce-dom-index-queries def _should_report_rage_click_issue(project: Project) -> bool: """ Checks the project option, controlled by a project owner. """ - return features.has( - "organizations:session-replay-rage-click-issue-creation", - project.organization, - ) and project.get_option("sentry:replay_rage_click_issues") -======= -def _should_report_rage_click_issue(project_id: int) -> bool: - project = Project.objects.get(id=project_id) + project = Project.objects.get(id=project.id) return project.get_option("sentry:replay_rage_click_issues") ->>>>>>> master def _iter_custom_events(events: list[dict[str, Any]]) -> Generator[dict[str, Any]]: diff --git a/tests/sentry/replays/unit/test_ingest_dom_index.py b/tests/sentry/replays/unit/test_ingest_dom_index.py index 903fc069998e69..9562e80bdfb70f 100644 --- a/tests/sentry/replays/unit/test_ingest_dom_index.py +++ b/tests/sentry/replays/unit/test_ingest_dom_index.py @@ -384,18 +384,8 @@ def test_parse_replay_dead_click_actions(patch_rage_click_issue_with_replay_even }, ] -<<<<<<< aliu/reduce-dom-index-queries - with Feature( - { - "organizations:session-replay-rage-click-issue-creation": True, - } - ): - default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) -======= default_project.update_option("sentry:replay_rage_click_issues", True) replay_actions = parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) ->>>>>>> master assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -949,18 +939,8 @@ def test_parse_replay_rage_clicks_with_replay_event( }, ] -<<<<<<< aliu/reduce-dom-index-queries - with Feature( - { - "organizations:session-replay-rage-click-issue-creation": True, - } - ): - default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) -======= default_project.update_option("sentry:replay_rage_click_issues", True) replay_actions = parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) ->>>>>>> master assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" From b5b3707130cf3bb4da556bba6944aff8fe78d268 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 20 Sep 2024 11:46:42 -0700 Subject: [PATCH 09/13] Fix test merge --- src/sentry/replays/usecases/ingest/dom_index.py | 1 - tests/sentry/replays/unit/test_ingest_dom_index.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/dom_index.py b/src/sentry/replays/usecases/ingest/dom_index.py index a8d31ca67a2f48..e211891d9c1792 100644 --- a/src/sentry/replays/usecases/ingest/dom_index.py +++ b/src/sentry/replays/usecases/ingest/dom_index.py @@ -313,7 +313,6 @@ def _should_report_rage_click_issue(project: Project) -> bool: """ Checks the project option, controlled by a project owner. """ - project = Project.objects.get(id=project.id) return project.get_option("sentry:replay_rage_click_issues") diff --git a/tests/sentry/replays/unit/test_ingest_dom_index.py b/tests/sentry/replays/unit/test_ingest_dom_index.py index 9562e80bdfb70f..34eaa548fafb32 100644 --- a/tests/sentry/replays/unit/test_ingest_dom_index.py +++ b/tests/sentry/replays/unit/test_ingest_dom_index.py @@ -385,7 +385,7 @@ def test_parse_replay_dead_click_actions(patch_rage_click_issue_with_replay_even ] default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) + replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" @@ -538,7 +538,7 @@ def test_rage_click_issue_creation_no_component_name( ] default_project.update_option("sentry:replay_rage_click_issues", True) - parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) + parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) # test that 2 rage click issues are still created assert patch_rage_click_issue_with_replay_event.call_count == 2 @@ -940,7 +940,7 @@ def test_parse_replay_rage_clicks_with_replay_event( ] default_project.update_option("sentry:replay_rage_click_issues", True) - replay_actions = parse_replay_actions(default_project.id, "1", 30, events, mock_replay_event()) + replay_actions = parse_replay_actions(default_project, "1", 30, events, mock_replay_event()) assert patch_rage_click_issue_with_replay_event.call_count == 2 assert replay_actions is not None assert replay_actions["type"] == "replay_event" From adeebadee66a63bdb612e47f1be8e1b3b358ac0f Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:32:00 -0700 Subject: [PATCH 10/13] Rm select related --- src/sentry/replays/usecases/ingest/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/usecases/ingest/__init__.py b/src/sentry/replays/usecases/ingest/__init__.py index 9ab992e8b4d8d6..6874e4cda2ec2a 100644 --- a/src/sentry/replays/usecases/ingest/__init__.py +++ b/src/sentry/replays/usecases/ingest/__init__.py @@ -262,7 +262,7 @@ def recording_post_processor( op="replays.usecases.ingest.parse_and_emit_replay_actions", description="parse_and_emit_replay_actions", ): - project = Project.objects.select_related("organization").get_from_cache( + project = Project.objects.get_from_cache( id=message.project_id ) parse_and_emit_replay_actions( From fcf0145c2ca9acec1e39575effb1cc511799202d Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 20:32:42 +0000 Subject: [PATCH 11/13] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/replays/usecases/ingest/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sentry/replays/usecases/ingest/__init__.py b/src/sentry/replays/usecases/ingest/__init__.py index 6874e4cda2ec2a..137e832b4df851 100644 --- a/src/sentry/replays/usecases/ingest/__init__.py +++ b/src/sentry/replays/usecases/ingest/__init__.py @@ -262,9 +262,7 @@ def recording_post_processor( op="replays.usecases.ingest.parse_and_emit_replay_actions", description="parse_and_emit_replay_actions", ): - project = Project.objects.get_from_cache( - id=message.project_id - ) + project = Project.objects.get_from_cache(id=message.project_id) parse_and_emit_replay_actions( retention_days=message.retention_days, project=project, From efaa6ed4a27b147bb0f4576b9b425ff1247155d2 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:34:02 -0700 Subject: [PATCH 12/13] Rm select related 2 --- src/sentry/replays/consumers/recording_buffered.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/consumers/recording_buffered.py b/src/sentry/replays/consumers/recording_buffered.py index dbc3d0dabbef9f..59917d086ea70f 100644 --- a/src/sentry/replays/consumers/recording_buffered.py +++ b/src/sentry/replays/consumers/recording_buffered.py @@ -319,7 +319,7 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: else None ) - project = Project.objects.select_related("organization").get_from_cache( + project = Project.objects.get_from_cache( id=decoded_message["project_id"] ) replay_actions = parse_replay_actions( From 103328b29d07c5d41c5eb57921baa61f1926d9b3 Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 20:34:46 +0000 Subject: [PATCH 13/13] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/replays/consumers/recording_buffered.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sentry/replays/consumers/recording_buffered.py b/src/sentry/replays/consumers/recording_buffered.py index 59917d086ea70f..05021dbe51f73b 100644 --- a/src/sentry/replays/consumers/recording_buffered.py +++ b/src/sentry/replays/consumers/recording_buffered.py @@ -319,9 +319,7 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: else None ) - project = Project.objects.get_from_cache( - id=decoded_message["project_id"] - ) + project = Project.objects.get_from_cache(id=decoded_message["project_id"]) replay_actions = parse_replay_actions( project, decoded_message["replay_id"],