Browse Source

ref(related_issues): Minor code refactoring before feature work (#69137)

Changes included:

* Only return new data shape in the endpoint
* Add rate limit to endpoint
* More explicit typing
* Return list of group IDs instead of groups
Armen Zambrano G 10 months ago
parent
commit
160e69539a

+ 25 - 12
src/sentry/api/endpoints/issues/related_issues.py

@@ -1,5 +1,3 @@
-from typing import Any
-
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -9,22 +7,37 @@ from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.group import GroupEndpoint
 from sentry.issues.related import find_related_issues
 from sentry.models.group import Group
+from sentry.types.ratelimit import RateLimit, RateLimitCategory
 
 
 @region_silo_endpoint
 class RelatedIssuesEndpoint(GroupEndpoint):
     owner = ApiOwner.ISSUES
     publish_status = {"GET": ApiPublishStatus.EXPERIMENTAL}
+    enforce_rate_limit = True
+    rate_limits = {
+        "GET": {
+            RateLimitCategory.IP: RateLimit(limit=15, window=5),
+            RateLimitCategory.USER: RateLimit(limit=15, window=5),
+            RateLimitCategory.ORGANIZATION: RateLimit(limit=15, window=1),
+        }
+    }
 
+    # We get a Group object since the endpoint is /issues/{issue_id}/related-issues
     def get(self, _: Request, group: Group) -> Response:
+        """
+        Retrieve related issues for an Issue
+        ````````````````````````````````````
+        Related issues can be based on the same root cause or trace connected.
+
+        :pparam string group_id: the ID of the issue
+        """
         related_issues = find_related_issues(group)
-        # Backward compatible for UI
-        response: dict[str, Any] = {
-            related_set["type"]: [int(g.id) for g in related_set["data"]]
-            for related_set in related_issues
-        }
-        response["data"] = [
-            {"type": related_set["type"], "data": [int(g.id) for g in related_set["data"]]}
-            for related_set in related_issues
-        ]
-        return Response(response)
+        return Response(
+            {
+                "data": [
+                    {"type": related_set["type"], "data": related_set["data"]}
+                    for related_set in related_issues
+                ]
+            }
+        )

+ 2 - 4
src/sentry/issues/related/__init__.py

@@ -1,7 +1,5 @@
 """This module exports a function to find related issues. It groups them by type."""
 
-from typing import Any
-
 from sentry.models.group import Group
 
 from .same_root_cause import same_root_cause_analysis
@@ -13,8 +11,8 @@ RELATED_ISSUES_ALGORITHMS = {
 }
 
 
-def find_related_issues(group: Group) -> list[dict[str, Any]]:
-    related_issues = []
+def find_related_issues(group: Group) -> list[dict[str, list[int] | str]]:
+    related_issues: list[dict[str, list[int] | str]] = []
     for key, func in RELATED_ISSUES_ALGORITHMS.items():
         related_issues.append({"type": key, "data": func(group)})
 

+ 6 - 9
src/sentry/issues/related/same_root_cause.py

@@ -3,29 +3,26 @@
 # The first case this module handles is environmental failures.
 #
 # Refer to README in module for more details.
-from typing import Any
-
 from sentry.models.group import Group
 from sentry.utils.query import RangeQuerySetWrapper
 
 
-def same_root_cause_analysis(group: Group) -> list[Group]:
+def same_root_cause_analysis(group: Group) -> list[int]:
     """Analyze and create a group set if the group was caused by the same root cause."""
     # Querying the data field (which is a GzippedDictField) cannot be done via
     # Django's ORM, thus, we do so via compare_groups
     project_groups = RangeQuerySetWrapper(Group.objects.filter(project=group.project_id), limit=100)
-    same_error_type_groups = [g for g in project_groups if compare_groups(g, group)]
+    same_error_type_groups = [g.id for g in project_groups if compare_groups(g, group)]
     return same_error_type_groups or []
 
 
 def compare_groups(groupA: Group, groupB: Group) -> bool:
-    return match_criteria(_extract_values(groupA), _extract_values(groupB))
+    return match_criteria(
+        {"title": groupA.title, "type": groupA.get_event_type()},
+        {"title": groupB.title, "type": groupB.get_event_type()},
+    )
 
 
 def match_criteria(a: dict[str, str | None], b: dict[str, str | None]) -> bool:
     # XXX: In future iterations we will be able to use similar titles rather than an exact match
     return a["type"] == b["type"] and a["title"] == b["title"]
-
-
-def _extract_values(group: Group) -> dict[str, Any]:
-    return {"title": group.title, "type": group.data.get("metadata", {}).get("type")}

+ 3 - 2
tests/sentry/api/endpoints/issues/test_related_issues.py

@@ -44,6 +44,7 @@ class RelatedIssuesTest(APITestCase):
         # For instance, this URL
         # https://us.sentry.io/api/0/organizations/sentry/issues-stats/?groups=4741828952&groups=4489703641&statsPeriod=24h
         assert response.json() == {
-            "same_root_cause": [1, 5],  # Old approach
-            "data": [{"type": "same_root_cause", "data": [1, 5]}],
+            "data": [
+                {"type": "same_root_cause", "data": [1, 5]},
+            ],
         }