feat(weekly-report): Add "Past Resolved Issues"#117751
Conversation
| REPORTS_KEY_ERRORS = "reports.key_errors" | ||
| REPORTS_KEY_ERRORS_BATCHED = "reports.key_errors.batched" | ||
| REPORTS_KEY_PERFORMANCE_ISSUES = "reports.key_performance_issues" | ||
| REPORTS_PAST_RESOLVED_ISSUES = "reports.past_resolved_issues" |
There was a problem hiding this comment.
i'm open to combining all referrers into one "WEEKLY_REPORT" referrer to remove all the clutter
There was a problem hiding this comment.
I would talk to the EAP team before doing that.
| self._hydrate_key_error_groups(ctx) | ||
| self._hydrate_key_performance_issue_groups(ctx) | ||
| if features.has("organizations:weekly-report-past-issues", self.organization): | ||
| self._append_project_past_resolved_issues(ctx) |
There was a problem hiding this comment.
Flag on still runs key errors
Medium Severity
With organizations:weekly-report-past-issues enabled, the report still builds unresolved key_errors and key_performance_issues and always puts them in the email context, while also populating past_issues. That contradicts replacing those sections with past resolved issues and wastes Snuba work on every flagged org.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0cdad91. Configure here.
There was a problem hiding this comment.
end goal is to replace key_errors and key_performance_issues. however, for now, we'll append this section
| REPORTS_KEY_ERRORS = "reports.key_errors" | ||
| REPORTS_KEY_ERRORS_BATCHED = "reports.key_errors.batched" | ||
| REPORTS_KEY_PERFORMANCE_ISSUES = "reports.key_performance_issues" | ||
| REPORTS_PAST_RESOLVED_ISSUES = "reports.past_resolved_issues" |
There was a problem hiding this comment.
I would talk to the EAP team before doing that.
| error_group_ids = [g.id for g in candidates if g.type is None or g.type < 1000] | ||
| perf_group_ids = [g.id for g in candidates if g.type is not None and 1000 <= g.type < 2000] |
There was a problem hiding this comment.
What are 1000 and 2000? Are there enum or constant values you could reference instead?
There was a problem hiding this comment.
Yup, i'll filter by issue_category == GroupCategory.ERROR and issue_category == GroupCategory.PERFORMANCE instead
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5652107. Configure here.
9563adb to
73111da
Compare
| project_id=project.id, | ||
| status=GroupStatus.RESOLVED, | ||
| resolved_at__gte=ctx.start, | ||
| resolved_at__lt=ctx.end + timedelta(days=1), |
There was a problem hiding this comment.
what's the reason for adding a day of buffer here?
There was a problem hiding this comment.
the previous queries for weekly report have always added the day buffer. my guess is that previously, there might have been issues with late-day events being excluded
| project_ctx.total_substatus_count += item["total"] | ||
|
|
||
|
|
||
| _PAST_ISSUES_CANDIDATE_LIMIT = 50 |
There was a problem hiding this comment.
nit: you don't have to _ prefix these
| Op.EQ, | ||
| GroupStatus.RESOLVED, | ||
| ), | ||
| Condition(Column("level", entity=events_entity), Op.EQ, "error"), |
There was a problem hiding this comment.
hmm i see the old queries do this as well but i think filtering on level: error actually might be wrong / unnecessary
There was a problem hiding this comment.
agree. if we remove the filter, we count events with levels "error", "warning", "info", "debug", and "fatal" which seems more correct. since we want to prioritize issues with the most activity
| for group, count, _has_link in project_ctx.past_resolved_issues | ||
| ] | ||
|
|
||
| # Re-sort with link boost applied, then truncate to top 3 |
There was a problem hiding this comment.
what would be cool is if we had an abstracted search API which allowed for arbitrary postgres and snuba conditions to both be taken into consideration for filtering and sorting, and you just get back the appropriate results based on your query
@roggenkemper we don't have anything like this right?
There was a problem hiding this comment.
sounds interesting -- how much would this overlap with the recommended sort?
| _PAST_ISSUES_LINK_BOOST = 2 | ||
|
|
||
|
|
||
| def project_past_resolved_issues( |
There was a problem hiding this comment.
i forgot - how did we manage the per-org batched requests for key/current errors? are we unable to use the same approach to batch the past issues queries?
There was a problem hiding this comment.
currently we're querying snuba per org per project (like how we queried pre-batch). The problem implementing it here is that we have a max of 50 group IDs to query. If we batch by 100, then we're querying for 5000 group IDs. so there are tradeoffs :(
we're implementing batching this weekend, so maybe we see how much it improves performance before we implement a similar logic here?
| return heapq.nlargest(3, all_key_performance_issues(), lambda d: d["count"]) | ||
|
|
||
| def past_issues(): | ||
| from sentry.tasks.summaries.utils import _PAST_ISSUES_LINK_BOOST |
There was a problem hiding this comment.
nit: can we put at top of file?
| past_issues.append( | ||
| { | ||
| "count": count, | ||
| "group": group, | ||
| "title": display["title"], | ||
| "message": display["message"], | ||
| "has_linked_pr_or_commit": has_link, | ||
| } | ||
| ) | ||
| past_issues.sort(key=lambda x: x["count"], reverse=True) | ||
| context["past_issues"] = past_issues[:3] |
There was a problem hiding this comment.
Bug: The debug view for weekly reports sorts past issues by count, while production uses a relevance score, causing a misleading preview of the issue order.
Severity: LOW
Suggested Fix
Update the debug view's sorting logic to match the production logic. Calculate a _relevance score for each past issue using the formula count * (PAST_ISSUES_LINK_BOOST if has_linked_pr_or_commit else 1) and sort the past_issues list based on this score before slicing the top results.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/web/frontend/debug/debug_weekly_report.py#L227-L241
Potential issue: The debug view for weekly reports at `debug/mail/weekly-reports/` sorts
past resolved issues by their raw `count`. However, the production code that generates
the actual email sorts these issues using a `_relevance` score, calculated as `count *
(PAST_ISSUES_LINK_BOOST if has_linked_pr_or_commit else 1)`. This discrepancy means the
debug preview, which is intended for testing, will show a different and incorrect
ordering of issues compared to the final email whenever an issue's rank is affected by
the link boost, making the preview unreliable.
Also affects:
src/sentry/tasks/summaries/weekly_reports.py:783-798
There was a problem hiding this comment.
preview is for frontend changes, populates with fake info anyway


Resolves ID-1622
To test:
debug/mail/weekly-reports/Summary
eventsdataset) and performance (search_issuesdataset) resolved groups, merge event counts, and enrich withGroupLinkdata via a single org-level batched queryevent_count * 2for issues linked to commits/PRs,event_count * 1otherwiseorganizations:weekly-report-past-issuesfeature flagBackend Changes
src/sentry/tasks/summaries/utils.py— Addproject_past_resolved_issues(), Snuba count helpers for error/perf datasets,fetch_past_resolved_issue_links()for org-level GroupLink enrichmentsrc/sentry/tasks/summaries/organization_report_context_factory.py— Wire new pipeline behind feature flag, skip old key_errors/key_performance_issues when flag is onsrc/sentry/tasks/summaries/weekly_reports.py— Addpast_issues()template context with relevance-boosted rankingFrontend Changes
body.htmlValidation / edge cases