From 891c3c1366570516c6ef9e80765a0e8ec40eaa76 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Wed, 15 Jan 2025 19:27:55 +0000 Subject: [PATCH] Revert "feat(autofix): Add profile context to Autofix payload (#83311)" This reverts commit 6812cc3024ad2bcc36b8090943c2d4395e3ea868. Co-authored-by: roaga <47861399+roaga@users.noreply.github.com> --- src/sentry/api/endpoints/group_ai_autofix.py | 178 +-------------- .../api/endpoints/test_group_ai_autofix.py | 205 +----------------- 2 files changed, 9 insertions(+), 374 deletions(-) diff --git a/src/sentry/api/endpoints/group_ai_autofix.py b/src/sentry/api/endpoints/group_ai_autofix.py index b0d22a4f078073..21bda66878a38d 100644 --- a/src/sentry/api/endpoints/group_ai_autofix.py +++ b/src/sentry/api/endpoints/group_ai_autofix.py @@ -17,11 +17,8 @@ from sentry.api.bases.group import GroupEndpoint from sentry.api.serializers import EventSerializer, serialize from sentry.autofix.utils import get_autofix_repos_from_project_code_mappings, get_autofix_state -from sentry.eventstore.models import Event, GroupEvent from sentry.integrations.utils.code_mapping import get_sorted_code_mapping_configs from sentry.models.group import Group -from sentry.models.project import Project -from sentry.profiles.utils import get_from_profiling_service from sentry.seer.signed_seer_api import get_seer_salted_url, sign_with_seer_secret from sentry.tasks.autofix import check_autofix_status from sentry.types.ratelimit import RateLimit, RateLimitCategory @@ -54,162 +51,14 @@ class GroupAutofixEndpoint(GroupEndpoint): def _get_serialized_event( self, event_id: str, group: Group, user: User | RpcUser | AnonymousUser - ) -> tuple[dict[str, Any] | None, Event | GroupEvent | None]: + ) -> dict[str, Any] | None: event = eventstore.backend.get_event_by_id(group.project.id, event_id, group_id=group.id) if not event: - return None, None + return None serialized_event = serialize(event, user, EventSerializer()) - return serialized_event, event - - def _get_profile_for_event(self, event: Event | GroupEvent, project: Project) -> dict[str, Any]: - context = event.data.get("contexts", {}) - profile_id = context.get("profile", {}).get("profile_id") # transaction profile - - profile_matches_event = True - if not profile_id: - # find most recent profile for this transaction instead - profile_matches_event = False - transaction_name = event.data.get("transaction", "") - if not transaction_name: - return {} - - event_filter = eventstore.Filter( - project_ids=[project.id], - conditions=[ - ["transaction", "=", transaction_name], - ["profile.id", "IS NOT NULL", None], - ], - ) - results = eventstore.backend.get_events( - filter=event_filter, - referrer="api.group_ai_autofix", - tenant_ids={"organization_id": project.organization_id}, - limit=1, - ) - if results: - context = results[0].data.get("contexts", {}) - profile_id = context.get("profile", {}).get("profile_id") - - response = get_from_profiling_service( - "GET", - f"/organizations/{project.organization_id}/projects/{project.id}/profiles/{profile_id}", - params={"format": "sample"}, - ) - - if response.status == 200: - profile = orjson.loads(response.data) - execution_tree = self._convert_profile_to_execution_tree(profile) - result = { - "profile_matches_issue": profile_matches_event, - "execution_tree": execution_tree, - } - return result - else: - return {} - - def _convert_profile_to_execution_tree(self, profile_data: dict) -> list[dict]: - """ - Converts profile data into a hierarchical representation of code execution, - including only items from the MainThread and app frames. - """ - profile = profile_data["profile"] - frames = profile["frames"] - stacks = profile["stacks"] - samples = profile["samples"] - - thread_metadata = profile.get("thread_metadata", {}) - main_thread_id = None - for key, value in thread_metadata.items(): - if value["name"] == "MainThread": - main_thread_id = key - break - - def create_frame_node(frame_index: int) -> dict: - """Create a node representation for a single frame""" - frame = frames[frame_index] - return { - "function": frame.get("function", ""), - "module": frame.get("module", ""), - "filename": frame.get("filename", ""), - "lineno": frame.get("lineno", 0), - "in_app": frame.get("in_app", False), - "children": [], - } - - def find_or_create_child(parent: dict, frame_data: dict) -> dict: - """Find existing child node or create new one""" - for child in parent["children"]: - if ( - child["function"] == frame_data["function"] - and child["module"] == frame_data["module"] - and child["filename"] == frame_data["filename"] - ): - return child - - parent["children"].append(frame_data) - return frame_data - - def merge_stack_into_tree(tree: list[dict], stack_frames: list[dict]): - """Merge a stack trace into the tree""" - if not stack_frames: - return - - # Find or create root node - root = None - for existing_root in tree: - if ( - existing_root["function"] == stack_frames[0]["function"] - and existing_root["module"] == stack_frames[0]["module"] - and existing_root["filename"] == stack_frames[0]["filename"] - ): - root = existing_root - break - - if root is None: - root = stack_frames[0] - tree.append(root) - - # Merge remaining frames - current = root - for frame in stack_frames[1:]: - current = find_or_create_child(current, frame) - - def process_stack(stack_index: int) -> list[dict]: - """Process a stack and return its frame hierarchy, filtering out non-app frames""" - frame_indices = stacks[stack_index] - - if not frame_indices: - return [] - - # Create nodes for app frames only, maintaining order - nodes = [] - for idx in reversed(frame_indices): - frame = frames[idx] - if frame.get("in_app", False) and not ( - frame.get("filename", "").startswith("<") - and frame.get("filename", "").endswith(">") - ): - nodes.append(create_frame_node(idx)) - - return nodes - - # Process all samples to build execution tree - execution_tree: list[dict] = [] - - for sample in samples: - stack_id = sample["stack_id"] - thread_id = sample["thread_id"] - - if str(thread_id) != str(main_thread_id): - continue - - stack_frames = process_stack(stack_id) - if stack_frames: - merge_stack_into_tree(execution_tree, stack_frames) - - return execution_tree + return serialized_event def _make_error_metadata(self, autofix: dict, reason: str): return { @@ -235,7 +84,6 @@ def _call_autofix( group: Group, repos: list[dict], serialized_event: dict[str, Any], - profile: dict[str, Any] | None, instruction: str, timeout_secs: int, pr_to_comment_on_url: str | None = None, @@ -252,7 +100,6 @@ def _call_autofix( "short_id": group.qualified_short_id, "events": [serialized_event], }, - "profile": profile, "instruction": instruction, "timeout_secs": timeout_secs, "last_updated": datetime.now().isoformat(), @@ -294,7 +141,7 @@ def post(self, request: Request, group: Group) -> Response: # This event_id is the event that the user is looking at when they click the "Fix" button event_id = data.get("event_id", None) if event_id is None: - event: Event | GroupEvent | None = group.get_recommended_event_for_environments() + event = group.get_recommended_event_for_environments() if not event: event = group.get_latest_event() @@ -316,7 +163,7 @@ def post(self, request: Request, group: Group) -> Response: return self._respond_with_error("AI Autofix is not enabled for this project.", 403) # For now we only send the event that the user is looking at, in the near future we want to send multiple events. - serialized_event, event = self._get_serialized_event(event_id, group, request.user) + serialized_event = self._get_serialized_event(event_id, group, request.user) if serialized_event is None: return self._respond_with_error("Cannot fix issues without an event.", 400) @@ -332,27 +179,12 @@ def post(self, request: Request, group: Group) -> Response: 400, ) - # find best profile for this event - try: - profile = self._get_profile_for_event(event, group.project) if event else None - except Exception as e: - logger.exception( - "Failed to get profile for event", - extra={ - "group_id": group.id, - "created_at": created_at, - "exception": e, - }, - ) - profile = None - try: run_id = self._call_autofix( request.user, group, repos, serialized_event, - profile, data.get("instruction", data.get("additional_context", "")), TIMEOUT_SECONDS, data.get("pr_to_comment_on_url", None), # support optional PR id for copilot diff --git a/tests/sentry/api/endpoints/test_group_ai_autofix.py b/tests/sentry/api/endpoints/test_group_ai_autofix.py index f96fa84ea8318a..568c5e13dbc92f 100644 --- a/tests/sentry/api/endpoints/test_group_ai_autofix.py +++ b/tests/sentry/api/endpoints/test_group_ai_autofix.py @@ -1,7 +1,7 @@ from datetime import datetime from unittest.mock import ANY, Mock, patch -from sentry.api.endpoints.group_ai_autofix import TIMEOUT_SECONDS, GroupAutofixEndpoint +from sentry.api.endpoints.group_ai_autofix import TIMEOUT_SECONDS from sentry.autofix.utils import AutofixState, AutofixStatus from sentry.models.group import Group from sentry.testutils.cases import APITestCase, SnubaTestCase @@ -87,20 +87,9 @@ def __init__(self): assert len(response.data["autofix"]["repositories"]) == 1 assert response.data["autofix"]["repositories"][0]["default_branch"] == "main" - @patch("sentry.api.endpoints.group_ai_autofix.get_from_profiling_service") - @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._get_profile_for_event") @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._call_autofix") @patch("sentry.tasks.autofix.check_autofix_status.apply_async") - def test_ai_autofix_post_endpoint( - self, mock_check_autofix_status, mock_call, mock_get_profile, mock_profiling_service - ): - # Mock profile data - mock_get_profile.return_value = {"profile_data": "test"} - mock_profiling_service.return_value.status = 200 - mock_profiling_service.return_value.data = ( - b'{"profile": {"frames": [], "stacks": [], "samples": [], "thread_metadata": {}}}' - ) - + def test_ai_autofix_post_endpoint(self, mock_check_autofix_status, mock_call): release = self.create_release(project=self.project, version="1.0.0") repo = self.create_repo( @@ -146,7 +135,6 @@ def test_ai_autofix_post_endpoint( } ], ANY, - {"profile_data": "test"}, "Yes", TIMEOUT_SECONDS, None, @@ -163,24 +151,9 @@ def test_ai_autofix_post_endpoint( mock_check_autofix_status.assert_called_once_with(args=[123], countdown=900) - @patch("sentry.api.endpoints.group_ai_autofix.get_from_profiling_service") - @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._get_profile_for_event") @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._call_autofix") @patch("sentry.tasks.autofix.check_autofix_status.apply_async") - def test_ai_autofix_post_without_event_id( - self, - mock_check_autofix_status, - mock_call, - mock_get_profile, - mock_profiling_service, - ): - # Mock profile data - mock_get_profile.return_value = {"profile_data": "test"} - mock_profiling_service.return_value.status = 200 - mock_profiling_service.return_value.data = ( - b'{"profile": {"frames": [], "stacks": [], "samples": [], "thread_metadata": {}}}' - ) - + def test_ai_autofix_post_without_event_id(self, mock_check_autofix_status, mock_call): release = self.create_release(project=self.project, version="1.0.0") repo = self.create_repo( @@ -224,7 +197,6 @@ def test_ai_autofix_post_without_event_id( } ], ANY, - {"profile_data": "test"}, "Yes", TIMEOUT_SECONDS, None, @@ -242,25 +214,11 @@ def test_ai_autofix_post_without_event_id( mock_check_autofix_status.assert_called_once_with(args=[123], countdown=900) @patch("sentry.models.Group.get_recommended_event_for_environments", return_value=None) - @patch("sentry.api.endpoints.group_ai_autofix.get_from_profiling_service") - @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._get_profile_for_event") @patch("sentry.api.endpoints.group_ai_autofix.GroupAutofixEndpoint._call_autofix") @patch("sentry.tasks.autofix.check_autofix_status.apply_async") def test_ai_autofix_post_without_event_id_no_recommended_event( - self, - mock_check_autofix_status, - mock_call, - mock_get_profile, - mock_profiling_service, - mock_event, + self, mock_check_autofix_status, mock_call, mock_event ): - # Mock profile data - mock_get_profile.return_value = {"profile_data": "test"} - mock_profiling_service.return_value.status = 200 - mock_profiling_service.return_value.data = ( - b'{"profile": {"frames": [], "stacks": [], "samples": [], "thread_metadata": {}}}' - ) - release = self.create_release(project=self.project, version="1.0.0") repo = self.create_repo( @@ -304,7 +262,6 @@ def test_ai_autofix_post_without_event_id_no_recommended_event( } ], ANY, - {"profile_data": "test"}, "Yes", TIMEOUT_SECONDS, None, @@ -445,157 +402,3 @@ def test_ai_autofix_without_stacktrace(self, mock_call): assert response.status_code == 400 # Expecting a Bad Request response for invalid repo assert response.data["detail"] == error_msg - - @patch("sentry.api.endpoints.group_ai_autofix.get_from_profiling_service") - def test_get_profile_for_event_with_profile_id(self, mock_profiling_service): - mock_profiling_service.return_value.status = 200 - mock_profiling_service.return_value.data = ( - b'{"profile": {"frames": [], "stacks": [], "samples": [], "thread_metadata": {}}}' - ) - - # Create event with profile ID - data = load_data("python", timestamp=before_now(minutes=1)) - data["contexts"] = { - "profile": { - "profile_id": "12345678901234567890123456789012", - } - } - event = self.store_event(data=data, project_id=self.project.id) - - profile = GroupAutofixEndpoint()._get_profile_for_event(event, self.project) - - assert profile["profile_matches_issue"] is True - assert "execution_tree" in profile - mock_profiling_service.assert_called_once_with( - "GET", - f"/organizations/{self.project.organization_id}/projects/{self.project.id}/profiles/12345678901234567890123456789012", - params={"format": "sample"}, - ) - - @patch("sentry.api.endpoints.group_ai_autofix.get_from_profiling_service") - def test_get_profile_for_event_without_profile_id(self, mock_profiling_service): - mock_profiling_service.return_value.status = 200 - mock_profiling_service.return_value.data = ( - b'{"profile": {"frames": [], "stacks": [], "samples": [], "thread_metadata": {}}}' - ) - - # Create event with transaction but no profile ID - data = load_data("python", timestamp=before_now(minutes=1)) - data["transaction"] = "test_transaction" - event = self.store_event(data=data, project_id=self.project.id) - - # Create a transaction event with profile ID that matches the transaction name - transaction_data = { - "transaction": "test_transaction", - "contexts": {"profile": {"profile_id": "12345678901234567890123456789012"}}, - } - self.store_event(data=transaction_data, project_id=self.project.id) - - profile = GroupAutofixEndpoint()._get_profile_for_event(event, self.project) - - assert profile["profile_matches_issue"] is False - assert "execution_tree" in profile - - def test_convert_profile_to_execution_tree(self): - profile_data = { - "profile": { - "frames": [ - { - "function": "main", - "module": "app.main", - "filename": "main.py", - "lineno": 10, - "in_app": True, - }, - { - "function": "helper", - "module": "app.utils", - "filename": "utils.py", - "lineno": 20, - "in_app": True, - }, - { - "function": "external", - "module": "external.lib", - "filename": "lib.py", - "lineno": 30, - "in_app": False, - }, - ], - "stacks": [ - [2, 1, 0] - ], # One stack with three frames. In a call stack, the first function is the last frame - "samples": [{"stack_id": 0, "thread_id": "1"}], - "thread_metadata": {"1": {"name": "MainThread"}}, - } - } - - execution_tree = GroupAutofixEndpoint()._convert_profile_to_execution_tree(profile_data) - - # Should only include in_app frames from MainThread - assert len(execution_tree) == 1 # One root node - root = execution_tree[0] - assert root["function"] == "main" - assert root["module"] == "app.main" - assert root["filename"] == "main.py" - assert root["lineno"] == 10 - assert len(root["children"]) == 1 - - child = root["children"][0] - assert child["function"] == "helper" - assert child["module"] == "app.utils" - assert child["filename"] == "utils.py" - assert child["lineno"] == 20 - assert len(child["children"]) == 0 # No children for the last in_app frame - - def test_convert_profile_to_execution_tree_non_main_thread(self): - """Test that non-MainThread samples are excluded from execution tree""" - profile_data = { - "profile": { - "frames": [ - { - "function": "worker", - "module": "app.worker", - "filename": "worker.py", - "lineno": 10, - "in_app": True, - } - ], - "stacks": [[0]], - "samples": [{"stack_id": 0, "thread_id": "2"}], - "thread_metadata": {"2": {"name": "WorkerThread"}}, - } - } - - execution_tree = GroupAutofixEndpoint()._convert_profile_to_execution_tree(profile_data) - - # Should be empty since no MainThread samples - assert len(execution_tree) == 0 - - def test_convert_profile_to_execution_tree_merges_duplicate_frames(self): - """Test that duplicate frames in different samples are merged correctly""" - profile_data = { - "profile": { - "frames": [ - { - "function": "main", - "module": "app.main", - "filename": "main.py", - "lineno": 10, - "in_app": True, - } - ], - "stacks": [[0], [0]], # Two stacks with the same frame - "samples": [ - {"stack_id": 0, "thread_id": "1"}, - {"stack_id": 1, "thread_id": "1"}, - ], - "thread_metadata": {"1": {"name": "MainThread"}}, - } - } - - execution_tree = GroupAutofixEndpoint()._convert_profile_to_execution_tree(profile_data) - - # Should only have one node even though frame appears in multiple samples - assert len(execution_tree) == 1 - assert execution_tree[0]["function"] == "main"