Revert "chore(similarity): Do not send > 30 system frames to seer (#81259)"

getsentry-bot · jangjodi · getsentry-bot · commit 104352cb3ce7 · 2024-11-25T23:22:05.000Z
This reverts commit 2e3a412. Co-authored-by: jangjodi <116035587+jangjodi@users.noreply.github.com>
diff --git a/src/sentry/grouping/ingest/seer.py b/src/sentry/grouping/ingest/seer.py
@@ -17,10 +17,9 @@
 from sentry.seer.similarity.similar_issues import get_similarity_data_from_seer
 from sentry.seer.similarity.types import SimilarIssuesEmbeddingsRequest
 from sentry.seer.similarity.utils import (
-    ReferrerOptions,
     event_content_is_seer_eligible,
     filter_null_from_string,
-    get_stacktrace_string_with_metrics,
+    get_stacktrace_string,
     killswitch_enabled,
 )
 from sentry.utils import metrics
@@ -183,9 +182,7 @@ def _circuit_breaker_broken(event: Event, project: Project) -> bool:
 
 
 def _has_empty_stacktrace_string(event: Event, variants: Mapping[str, BaseVariant]) -> bool:
-    stacktrace_string = get_stacktrace_string_with_metrics(
-        get_grouping_info_from_variants(variants), event.platform, ReferrerOptions.INGEST
-    )
+    stacktrace_string = get_stacktrace_string(get_grouping_info_from_variants(variants))
     if stacktrace_string == "":
         metrics.incr(
             "grouping.similarity.did_call_seer",
@@ -220,10 +217,7 @@ def get_seer_similar_issues(
         "hash": event_hash,
         "project_id": event.project.id,
         "stacktrace": event.data.get(
-            "stacktrace_string",
-            get_stacktrace_string_with_metrics(
-                get_grouping_info_from_variants(variants), event.platform, ReferrerOptions.INGEST
-            ),
+            "stacktrace_string", get_stacktrace_string(get_grouping_info_from_variants(variants))
         ),
         "exception_type": filter_null_from_string(exception_type) if exception_type else None,
         "k": num_neighbors,
diff --git a/src/sentry/issues/endpoints/group_similar_issues_embeddings.py b/src/sentry/issues/endpoints/group_similar_issues_embeddings.py
@@ -18,7 +18,6 @@
 from sentry.seer.similarity.similar_issues import get_similarity_data_from_seer
 from sentry.seer.similarity.types import SeerSimilarIssueData, SimilarIssuesEmbeddingsRequest
 from sentry.seer.similarity.utils import (
-    TooManyOnlySystemFramesException,
     event_content_has_stacktrace,
     get_stacktrace_string,
     killswitch_enabled,
@@ -83,12 +82,9 @@ def get(self, request: Request, group) -> Response:
         stacktrace_string = ""
         if latest_event and event_content_has_stacktrace(latest_event):
             grouping_info = get_grouping_info(None, project=group.project, event=latest_event)
-            try:
-                stacktrace_string = get_stacktrace_string(grouping_info)
-            except TooManyOnlySystemFramesException:
-                stacktrace_string = ""
+            stacktrace_string = get_stacktrace_string(grouping_info)
 
-        if not stacktrace_string or not latest_event:
+        if stacktrace_string == "" or not latest_event:
             return Response([])  # No exception, stacktrace or in-app frames, or event
 
         similar_issues_params: SimilarIssuesEmbeddingsRequest = {
diff --git a/src/sentry/seer/similarity/utils.py b/src/sentry/seer/similarity/utils.py
@@ -1,5 +1,4 @@
 import logging
-from enum import StrEnum
 from typing import Any, TypeVar
 
 from sentry import options
@@ -152,15 +151,6 @@
 ]
 
 
-class ReferrerOptions(StrEnum):
-    INGEST = "ingest"
-    BACKFILL = "backfill"
-
-
-class TooManyOnlySystemFramesException(Exception):
-    pass
-
-
 def _get_value_if_exists(exception_value: dict[str, Any]) -> str:
     return exception_value["values"][0] if exception_value.get("values") else ""
 
@@ -187,7 +177,6 @@ def get_stacktrace_string(data: dict[str, Any]) -> str:
 
     frame_count = 0
     html_frame_count = 0  # for a temporary metric
-    is_frames_truncated = False
     stacktrace_str = ""
     found_non_snipped_context_line = False
 
@@ -196,15 +185,12 @@ def get_stacktrace_string(data: dict[str, Any]) -> str:
     def _process_frames(frames: list[dict[str, Any]]) -> list[str]:
         nonlocal frame_count
         nonlocal html_frame_count
-        nonlocal is_frames_truncated
         nonlocal found_non_snipped_context_line
         frame_strings = []
 
         contributing_frames = [
             frame for frame in frames if frame.get("id") == "frame" and frame.get("contributes")
         ]
-        if len(contributing_frames) + frame_count > MAX_FRAME_COUNT:
-            is_frames_truncated = True
         contributing_frames = _discard_excess_frames(
             contributing_frames, MAX_FRAME_COUNT, frame_count
         )
@@ -269,8 +255,6 @@ def _process_frames(frames: list[dict[str, Any]]) -> list[str]:
                     exc_value = _get_value_if_exists(exception_value)
                 elif exception_value.get("id") == "stacktrace" and frame_count < MAX_FRAME_COUNT:
                     frame_strings = _process_frames(exception_value["values"])
-        if is_frames_truncated and not app_hash:
-            raise TooManyOnlySystemFramesException
         # Only exceptions have the type and value properties, so we don't need to handle the threads
         # case here
         header = f"{exc_type}: {exc_value}\n" if exception["id"] == "exception" else ""
@@ -306,31 +290,6 @@ def _process_frames(frames: list[dict[str, Any]]) -> list[str]:
     return stacktrace_str.strip()
 
 
-def get_stacktrace_string_with_metrics(
-    data: dict[str, Any], platform: str | None, referrer: ReferrerOptions
-) -> str | None:
-    try:
-        stacktrace_string = get_stacktrace_string(data)
-    except TooManyOnlySystemFramesException:
-        platform = platform if platform else "unknown"
-        metrics.incr(
-            "grouping.similarity.over_threshold_only_system_frames",
-            sample_rate=options.get("seer.similarity.metrics_sample_rate"),
-            tags={"platform": platform, "referrer": referrer},
-        )
-        if referrer == ReferrerOptions.INGEST:
-            metrics.incr(
-                "grouping.similarity.did_call_seer",
-                sample_rate=options.get("seer.similarity.metrics_sample_rate"),
-                tags={
-                    "call_made": False,
-                    "blocker": "over-threshold-only-system-frames",
-                },
-            )
-        stacktrace_string = None
-    return stacktrace_string
-
-
 def event_content_has_stacktrace(event: Event) -> bool:
     # If an event has no stacktrace, there's no data for Seer to analyze, so no point in making the
     # API call. If we ever start analyzing message-only events, we'll need to add `event.title in
diff --git a/src/sentry/tasks/embeddings_grouping/utils.py b/src/sentry/tasks/embeddings_grouping/utils.py
@@ -32,10 +32,9 @@
     SimilarHashNotFoundError,
 )
 from sentry.seer.similarity.utils import (
-    ReferrerOptions,
     event_content_has_stacktrace,
     filter_null_from_string,
-    get_stacktrace_string_with_metrics,
+    get_stacktrace_string,
 )
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.referrer import Referrer
@@ -356,10 +355,8 @@ def get_events_from_nodestore(
         event._project_cache = project
         if event and event.data and event_content_has_stacktrace(event):
             grouping_info = get_grouping_info(None, project=project, event=event)
-            stacktrace_string = get_stacktrace_string_with_metrics(
-                grouping_info, event.platform, ReferrerOptions.BACKFILL
-            )
-            if not stacktrace_string:
+            stacktrace_string = get_stacktrace_string(grouping_info)
+            if stacktrace_string == "":
                 invalid_event_group_ids.append(group_id)
                 continue
             primary_hash = event.get_primary_hash()
diff --git a/tests/sentry/grouping/ingest/test_seer.py b/tests/sentry/grouping/ingest/test_seer.py
@@ -8,7 +8,6 @@
 from sentry.grouping.ingest.seer import get_seer_similar_issues, should_call_seer_for_grouping
 from sentry.models.grouphash import GroupHash
 from sentry.seer.similarity.types import SeerSimilarIssueData
-from sentry.seer.similarity.utils import MAX_FRAME_COUNT
 from sentry.testutils.cases import TestCase
 from sentry.testutils.helpers.eventprocessing import save_new_event
 from sentry.testutils.helpers.options import override_options
@@ -307,51 +306,3 @@ def test_returns_no_grouphash_and_empty_metadata_if_no_similar_group_found(self)
                 expected_metadata,
                 None,
             )
-
-    @patch("sentry.seer.similarity.utils.metrics")
-    def test_too_many_only_system_frames(self, mock_metrics: Mock) -> None:
-        type = "FailedToFetchError"
-        value = "Charlie didn't bring the ball back"
-        context_line = f"raise {type}('{value}')"
-        new_event = Event(
-            project_id=self.project.id,
-            event_id="22312012112120120908201304152013",
-            data={
-                "title": f"{type}('{value}')",
-                "exception": {
-                    "values": [
-                        {
-                            "type": type,
-                            "value": value,
-                            "stacktrace": {
-                                "frames": [
-                                    {
-                                        "function": f"play_fetch_{i}",
-                                        "filename": f"dogpark{i}.py",
-                                        "context_line": context_line,
-                                    }
-                                    for i in range(MAX_FRAME_COUNT + 1)
-                                ]
-                            },
-                        }
-                    ]
-                },
-                "platform": "python",
-            },
-        )
-        get_seer_similar_issues(new_event, new_event.get_grouping_variants())
-
-        sample_rate = options.get("seer.similarity.metrics_sample_rate")
-        mock_metrics.incr.assert_any_call(
-            "grouping.similarity.over_threshold_only_system_frames",
-            sample_rate=sample_rate,
-            tags={"platform": "python", "referrer": "ingest"},
-        )
-        mock_metrics.incr.assert_any_call(
-            "grouping.similarity.did_call_seer",
-            sample_rate=1.0,
-            tags={
-                "call_made": False,
-                "blocker": "over-threshold-only-system-frames",
-            },
-        )
diff --git a/tests/sentry/seer/similarity/test_utils.py b/tests/sentry/seer/similarity/test_utils.py
@@ -3,14 +3,10 @@
 from typing import Any, Literal, cast
 from uuid import uuid1
 
-import pytest
-
 from sentry.eventstore.models import Event
 from sentry.seer.similarity.utils import (
     BASE64_ENCODED_PREFIXES,
-    MAX_FRAME_COUNT,
     SEER_ELIGIBLE_PLATFORMS,
-    TooManyOnlySystemFramesException,
     _is_snipped_context_line,
     event_content_is_seer_eligible,
     filter_null_from_string,
@@ -674,18 +670,18 @@ def test_chained_too_many_frames_minified_js_frame_limit(self):
             )
 
     def test_chained_too_many_exceptions(self):
-        """Test that we restrict number of chained exceptions to MAX_FRAME_COUNT."""
+        """Test that we restrict number of chained exceptions to 30."""
         data_chained_exception = copy.deepcopy(self.CHAINED_APP_DATA)
         data_chained_exception["app"]["component"]["values"][0]["values"] = [
             self.create_exception(
                 exception_type_str="Exception",
                 exception_value=f"exception {i} message!",
                 frames=self.create_frames(num_frames=1, context_line_factory=lambda i: f"line {i}"),
             )
-            for i in range(1, MAX_FRAME_COUNT + 2)
+            for i in range(1, 32)
         ]
         stacktrace_str = get_stacktrace_string(data_chained_exception)
-        for i in range(2, MAX_FRAME_COUNT + 2):
+        for i in range(2, 32):
             assert f"exception {i} message!" in stacktrace_str
         assert "exception 1 message!" not in stacktrace_str
 
@@ -714,35 +710,9 @@ def test_no_app_no_system(self):
         stacktrace_str = get_stacktrace_string(data)
         assert stacktrace_str == ""
 
-    def test_too_many_system_frames_single_exception(self):
-        data_system = copy.deepcopy(self.BASE_APP_DATA)
-        data_system["system"] = data_system.pop("app")
-        data_system["system"]["component"]["values"][0]["values"][0][
-            "values"
-        ] += self.create_frames(MAX_FRAME_COUNT + 1, True)
-
-        with pytest.raises(TooManyOnlySystemFramesException):
-            get_stacktrace_string(data_system)
-
-    def test_too_many_system_frames_chained_exception(self):
-        data_system = copy.deepcopy(self.CHAINED_APP_DATA)
-        data_system["system"] = data_system.pop("app")
-        # Split MAX_FRAME_COUNT across the two exceptions
-        data_system["system"]["component"]["values"][0]["values"][0]["values"][0][
-            "values"
-        ] += self.create_frames(MAX_FRAME_COUNT // 2, True)
-        data_system["system"]["component"]["values"][0]["values"][1]["values"][0][
-            "values"
-        ] += self.create_frames(MAX_FRAME_COUNT // 2, True)
-
-        with pytest.raises(TooManyOnlySystemFramesException):
-            get_stacktrace_string(data_system)
+    def test_over_30_contributing_frames(self):
+        """Check that when there are over 30 contributing frames, the last 30 are included."""
 
-    def test_too_many_in_app_contributing_frames(self):
-        """
-        Check that when there are over MAX_FRAME_COUNT contributing frames, the last MAX_FRAME_COUNT
-        are included.
-        """
         data_frames = copy.deepcopy(self.BASE_APP_DATA)
         # Create 30 contributing frames, 1-20 -> last 10 should be included
         data_frames["app"]["component"]["values"][0]["values"][0]["values"] = self.create_frames(
@@ -769,7 +739,7 @@ def test_too_many_in_app_contributing_frames(self):
         for i in range(41, 61):
             num_frames += 1
             assert ("test = " + str(i) + "!") in stacktrace_str
-        assert num_frames == MAX_FRAME_COUNT
+        assert num_frames == 30
 
     def test_too_many_frames_minified_js_frame_limit(self):
         """Test that we restrict fully-minified stacktraces to 20 frames, and all other stacktraces to 30 frames."""
diff --git a/tests/sentry/tasks/test_backfill_seer_grouping_records.py b/tests/sentry/tasks/test_backfill_seer_grouping_records.py
