Browse Source

ref: fix reassignment of key_errors (#75186)

depending on the part of the code `key_errors` was either
`list[tuple[int, int]]` or `list[tuple[Group, None, int]]` -- this is
now explicitly separated as `key_errors_by_id: list[tuple[int, int]]`
and `key_errors_by_group: list[tuple[Group, int]]`

<!-- Describe your PR here. -->
anthony sottile 7 months ago
parent
commit
1ae38c5289

+ 3 - 3
src/sentry/integrations/slack/message_builder/notifications/daily_summary.py

@@ -149,10 +149,10 @@ class SlackDailySummaryMessageBuilder(SlackNotificationsMessageBuilder):
 
             # Add Top 3 Error/Performance Issues
             top_issue_fields = []
-            if context.key_errors:
+            if context.key_errors_by_group:
                 top_errors_text = "*Today's Top 3 Error Issues*\n"
-                for error in context.key_errors:
-                    linked_title = self.linkify_error_title(error[0])
+                for group, _ in context.key_errors_by_group:
+                    linked_title = self.linkify_error_title(group)
                     top_errors_text += f"• {linked_title}\n"
                 top_issue_fields.append(self.make_field(top_errors_text))
 

+ 3 - 1
src/sentry/tasks/summaries/daily_summary.py

@@ -189,7 +189,9 @@ def build_summary_data(
                 ctx=ctx, project=project, referrer=Referrer.DAILY_SUMMARY_KEY_ERRORS.value
             )
             if key_errors:
-                project_ctx.key_errors = [(e["events.group_id"], e["count()"]) for e in key_errors]
+                project_ctx.key_errors_by_id = [
+                    (e["events.group_id"], e["count()"]) for e in key_errors
+                ]
 
             # Today's Top 3 Performance Issues
             key_performance_issues = project_key_performance_issues(

+ 23 - 33
src/sentry/tasks/summaries/utils.py

@@ -75,8 +75,8 @@ class ProjectContext:
     def __init__(self, project):
         self.project = project
 
-        # Array of (group_id, group_history, count)
-        self.key_errors = []
+        self.key_errors_by_id: list[tuple[int, int]] = []
+        self.key_errors_by_group: list[tuple[Group, int]] = []
         # Array of (transaction_name, count_this_week, p95_this_week, count_last_week, p95_last_week)
         self.key_transactions = []
         # Array of (Group, count)
@@ -94,7 +94,7 @@ class ProjectContext:
     def __repr__(self):
         return "\n".join(
             [
-                f"{self.key_errors}, ",
+                f"{self.key_errors_by_group}, ",
                 f"Errors: [Accepted {self.accepted_error_count}, Dropped {self.dropped_error_count}]",
                 f"Transactions: [Accepted {self.accepted_transaction_count} Dropped {self.dropped_transaction_count}]",
                 f"Replays: [Accepted {self.accepted_replay_count} Dropped {self.dropped_replay_count}]",
@@ -103,7 +103,7 @@ class ProjectContext:
 
     def check_if_project_is_empty(self):
         return (
-            not self.key_errors
+            not self.key_errors_by_group
             and not self.key_transactions
             and not self.key_performance_issues
             and not self.accepted_error_count
@@ -116,26 +116,21 @@ class ProjectContext:
 
 
 class DailySummaryProjectContext:
-    total_today = 0
-    comparison_period_total = 0
-    comparison_period_avg = 0
-    key_errors: list[tuple[Group, int]] = []
-    key_performance_issues: list[tuple[Group, int]] = []
-    escalated_today: list[Group] = []
-    regressed_today: list[Group] = []
-    new_in_release: dict[int, list[Group]] = {}
-
     def __init__(self, project: Project):
+        self.total_today = 0
+        self.comparison_period_total = 0
+        self.comparison_period_avg = 0
         self.project = project
-        self.key_errors = []
-        self.key_performance_issues = []
-        self.escalated_today = []
-        self.regressed_today = []
-        self.new_in_release = {}
+        self.key_errors_by_id: list[tuple[int, int]] = []
+        self.key_errors_by_group: list[tuple[Group, int]] = []
+        self.key_performance_issues: list[tuple[Group, int]] = []
+        self.escalated_today: list[Group] = []
+        self.regressed_today: list[Group] = []
+        self.new_in_release: dict[int, list[Group]] = {}
 
     def check_if_project_is_empty(self):
         return (
-            not self.key_errors
+            not self.key_errors_by_group
             and not self.key_performance_issues
             and not self.total_today
             and not self.comparison_period_total
@@ -217,7 +212,7 @@ def project_key_errors(
         )
         query_result = raw_snql_query(request, referrer=referrer)
         key_errors = query_result["data"]
-        # Set project_ctx.key_errors to be an array of (group_id, count) for now.
+        # Set project_ctx.key_errors_by_id to be an array of (group_id, count) for now.
         # We will query the group history later on in `fetch_key_error_groups`, batched in a per-organization basis
         return key_errors
 
@@ -346,7 +341,7 @@ def fetch_key_error_groups(ctx: OrganizationReportContext) -> None:
     # Organization pass. Depends on project_key_errors.
     all_key_error_group_ids = []
     for project_ctx in ctx.projects_context_map.values():
-        all_key_error_group_ids.extend([group_id for group_id, count in project_ctx.key_errors])
+        all_key_error_group_ids.extend([group_id for group_id, _ in project_ctx.key_errors_by_id])
 
     if len(all_key_error_group_ids) == 0:
         return
@@ -358,19 +353,14 @@ def fetch_key_error_groups(ctx: OrganizationReportContext) -> None:
     for project_ctx in ctx.projects_context_map.values():
         # note Snuba might have groups that have since been deleted
         # we should just ignore those
-        project_ctx.key_errors = list(
-            filter(
-                lambda x: x[0] is not None,
-                [
-                    (
-                        group_id_to_group.get(group_id),
-                        None,
-                        count,
-                    )
-                    for group_id, count in project_ctx.key_errors
-                ],
+        project_ctx.key_errors_by_group = [
+            (group, count)
+            for group, count in (
+                (group_id_to_group.get(group_id), count)
+                for group_id, count in project_ctx.key_errors_by_id
             )
-        )
+            if group is not None
+        ]
 
 
 def fetch_key_performance_issue_groups(ctx: OrganizationReportContext):

+ 6 - 10
src/sentry/tasks/summaries/weekly_reports.py

@@ -177,7 +177,9 @@ def prepare_organization_report(
 
             project_ctx = cast(ProjectContext, ctx.projects_context_map[project.id])
             if key_errors:
-                project_ctx.key_errors = [(e["events.group_id"], e["count()"]) for e in key_errors]
+                project_ctx.key_errors_by_id = [
+                    (e["events.group_id"], e["count()"]) for e in key_errors
+                ]
 
                 if ctx.organization.slug == "sentry":
                     logger.info(
@@ -636,7 +638,7 @@ def render_template_context(ctx, user_id: int | None) -> dict[str, Any] | None:
                             "project_id": project_ctx.project.id,
                         },
                     )
-                for group, group_history, count in project_ctx.key_errors:
+                for group, count in project_ctx.key_errors_by_group:
                     if ctx.organization.slug == "sentry":
                         logger.info(
                             "render_template_context.all_key_errors.found_error",
@@ -656,14 +658,8 @@ def render_template_context(ctx, user_id: int | None) -> dict[str, Any] | None:
                     yield {
                         "count": count,
                         "group": group,
-                        "status": (
-                            group_history.get_status_display() if group_history else "Unresolved"
-                        ),
-                        "status_color": (
-                            group_status_to_color[group_history.status]
-                            if group_history
-                            else group_status_to_color[GroupHistoryStatus.NEW]
-                        ),
+                        "status": "Unresolved",
+                        "status_color": (group_status_to_color[GroupHistoryStatus.NEW]),
                         "group_substatus": substatus,
                         "group_substatus_color": substatus_color,
                         "group_substatus_border_color": substatus_border_color,

+ 2 - 2
src/sentry/web/frontend/debug/debug_weekly_report.py

@@ -82,8 +82,8 @@ class DebugWeeklyReportView(MailPreviewView):
             project_context.dropped_replay_count = int(
                 random.weibullvariate(5, 1) * random.paretovariate(0.2)
             )
-            project_context.key_errors = [
-                (g, None, random.randint(0, 1000)) for g in Group.objects.all()[:3]
+            project_context.key_errors_by_group = [
+                (g, random.randint(0, 1000)) for g in Group.objects.all()[:3]
             ]
 
             project_context.new_substatus_count = random.randint(5, 200)

+ 14 - 14
tests/sentry/tasks/test_daily_summary.py

@@ -261,13 +261,13 @@ class DailySummaryTest(
         )
         assert project_context_map.total_today == 17  # total outcomes from today
         assert project_context_map.comparison_period_avg == 1
-        assert len(project_context_map.key_errors) == 3
-        assert (self.group1, None, 3) in project_context_map.key_errors
-        assert (self.group2, None, 2) in project_context_map.key_errors
-        assert (self.group3, None, 10) in project_context_map.key_errors
+        assert len(project_context_map.key_errors_by_group) == 3
+        assert (self.group1, 3) in project_context_map.key_errors_by_group
+        assert (self.group2, 2) in project_context_map.key_errors_by_group
+        assert (self.group3, 10) in project_context_map.key_errors_by_group
         assert len(project_context_map.key_performance_issues) == 2
-        assert (self.perf_event.group, None, 1) in project_context_map.key_performance_issues
-        assert (self.perf_event2.group, None, 1) in project_context_map.key_performance_issues
+        assert (self.perf_event.group, 1) in project_context_map.key_performance_issues
+        assert (self.perf_event2.group, 1) in project_context_map.key_performance_issues
         assert project_context_map.escalated_today == [self.group3]
         assert project_context_map.regressed_today == [self.group2]
         assert len(project_context_map.new_in_release) == 2
@@ -281,7 +281,7 @@ class DailySummaryTest(
         )
         assert project_context_map2.total_today == 2
         assert project_context_map2.comparison_period_avg == 0
-        assert project_context_map2.key_errors == [(self.group4, None, 2)]
+        assert project_context_map2.key_errors_by_group == [(self.group4, 2)]
         assert project_context_map2.key_performance_issues == []
         assert project_context_map2.escalated_today == []
         assert project_context_map2.regressed_today == []
@@ -328,9 +328,9 @@ class DailySummaryTest(
         )
         assert project_context_map.total_today == 9  # total outcomes from today
         assert project_context_map.comparison_period_avg == 0
-        assert len(project_context_map.key_errors) == 2
-        assert (group1, None, 3) in project_context_map.key_errors
-        assert (group2, None, 3) in project_context_map.key_errors
+        assert len(project_context_map.key_errors_by_group) == 2
+        assert (group1, 3) in project_context_map.key_errors_by_group
+        assert (group2, 3) in project_context_map.key_errors_by_group
 
     def test_build_summary_data_filter_to_error_level(self):
         """Test that non-error level issues are filtered out of the results"""
@@ -371,10 +371,10 @@ class DailySummaryTest(
         )
         assert project_context_map.total_today == 9  # total outcomes from today
         assert project_context_map.comparison_period_avg == 0
-        assert len(project_context_map.key_errors) == 2
-        assert (group1, None, 3) not in project_context_map.key_errors
-        assert (group2, None, 3) in project_context_map.key_errors
-        assert (group3, None, 3) in project_context_map.key_errors
+        assert len(project_context_map.key_errors_by_group) == 2
+        assert (group1, 3) not in project_context_map.key_errors_by_group
+        assert (group2, 3) in project_context_map.key_errors_by_group
+        assert (group3, 3) in project_context_map.key_errors_by_group
 
     def test_build_summary_data_dedupes_groups(self):
         """