Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,8 @@ def register_temporary_features(manager: FeatureManager) -> None:

# Use batched Snuba queries for weekly report key errors instead of per-project queries
manager.add("organizations:weekly-report-batched-key-errors", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Show combined resolved "past issues" section instead of separate key errors / performance issues
manager.add("organizations:weekly-report-past-issues", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable week-over-week percentage change metric in weekly email reports
manager.add("organizations:weekly-report-week-over-week-metric", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable logging to debug workflow engine process workflows
Expand Down
1 change: 1 addition & 0 deletions src/sentry/snuba/referrer.py
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ class Referrer(StrEnum):
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"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm open to combining all referrers into one "WEEKLY_REPORT" referrer to remove all the clutter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would talk to the EAP team before doing that.

REPORTS_KEY_TRANSACTIONS_THIS_WEEK = "reports.key_transactions.this_week"
REPORTS_KEY_TRANSACTIONS_LAST_WEEK = "reports.key_transactions.last_week"
REPORTS_OUTCOME_SERIES = "reports.outcome_series"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@
ProjectContext,
fetch_key_error_groups,
fetch_key_performance_issue_groups,
fetch_past_resolved_issue_links,
org_key_errors,
organization_project_issue_substatus_summaries,
project_event_counts_for_organization,
project_key_errors,
project_key_performance_issues,
project_key_transactions_last_week,
project_key_transactions_this_week,
project_past_resolved_issues,
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
)
from sentry.tasks.summaries.weekly_report_cache import read_project_metrics
from sentry.utils import metrics
Expand Down Expand Up @@ -214,6 +216,21 @@ def _hydrate_key_performance_issue_groups(self, ctx: OrganizationReportContext)
with sentry_sdk.start_span(op="weekly_reports.fetch_key_performance_issue_groups"):
fetch_key_performance_issue_groups(ctx)

@metrics.wraps("weekly_report.create_context.project_past_resolved_issues")
def _append_project_past_resolved_issues(self, ctx: OrganizationReportContext) -> None:
with sentry_sdk.start_span(op="weekly_reports.project_past_resolved_issues"):
for project in ctx.organization.project_set.all():
if project.id not in ctx.projects_context_map:
continue
project_ctx = ctx.projects_context_map[project.id]
resolved = project_past_resolved_issues(
ctx, project, referrer=Referrer.REPORTS_PAST_RESOLVED_ISSUES.value
)
if resolved:
project_ctx.past_resolved_issues = resolved

fetch_past_resolved_issue_links(ctx)

def create_context(self) -> OrganizationReportContext:
ctx = OrganizationReportContext(self.timestamp, self.duration, self.organization)

Expand All @@ -234,5 +251,7 @@ def create_context(self) -> OrganizationReportContext:
self._append_project_key_errors(ctx)
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0cdad91. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end goal is to replace key_errors and key_performance_issues. however, for now, we'll append this section


return ctx
192 changes: 192 additions & 0 deletions src/sentry/tasks/summaries/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@
from snuba_sdk.relationships import Relationship

from sentry.constants import DataCategory
from sentry.issues.grouptype import (
PERFORMANCE_ISSUE_CATEGORIES,
GroupCategory,
InvalidGroupTypeError,
)
from sentry.models.group import Group, GroupStatus
from sentry.models.grouphistory import GroupHistory
from sentry.models.grouplink import GroupLink
from sentry.models.organization import Organization
from sentry.models.organizationmember import OrganizationMember
from sentry.models.project import Project
Expand Down Expand Up @@ -87,6 +93,8 @@ def __init__(self, project):
self.key_transactions = []
# Array of (Group, count)
self.key_performance_issues = []
# Array of (Group, event_count, has_linked_pr_or_commit)
self.past_resolved_issues: list[tuple[Group, int, bool]] = []

self.key_replay_events = []

Expand All @@ -109,6 +117,7 @@ def check_if_project_is_empty(self):
not self.key_errors_by_group
and not self.key_transactions
and not self.key_performance_issues
and not self.past_resolved_issues
and not self.accepted_error_count
and not self.accepted_transaction_count
)
Expand Down Expand Up @@ -737,3 +746,186 @@ def organization_project_issue_substatus_summaries(ctx: OrganizationReportContex
if item["substatus"] == GroupSubStatus.REGRESSED:
project_ctx.regression_substatus_count = item["total"]
project_ctx.total_substatus_count += item["total"]


PAST_ISSUES_CANDIDATE_LIMIT = 50
PAST_ISSUES_LINK_BOOST = 2


def project_past_resolved_issues(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

ctx: OrganizationReportContext, project: Project, referrer: str
) -> list[tuple[Group, int, bool]]:
if not project.first_event:
return []

with sentry_sdk.start_span(op="weekly_reports.project_past_resolved_issues"):
candidates = list(
Group.objects.filter(
project_id=project.id,
status=GroupStatus.RESOLVED,
resolved_at__gte=ctx.start,
resolved_at__lt=ctx.end + timedelta(days=1),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for adding a day of buffer here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

).order_by("-times_seen")[:PAST_ISSUES_CANDIDATE_LIMIT]
)

if not candidates:
return []

# Filter out groups with unregistered type IDs (deprecated/removed issue types)
valid_candidates = []
for g in candidates:
if g.type is None:
valid_candidates.append(g)
continue
try:
g.issue_category
except InvalidGroupTypeError:
continue
valid_candidates.append(g)

group_id_to_group = {g.id: g for g in valid_candidates}

# Legacy groups may have a None .type which crashes issue_category; treat as error group
error_group_ids = [
g.id
for g in valid_candidates
if g.type is None or g.issue_category == GroupCategory.ERROR
]
perf_group_ids = [
g.id
for g in valid_candidates
if g.type is not None
and (
g.issue_category == GroupCategory.PERFORMANCE
or g.issue_category in PERFORMANCE_ISSUE_CATEGORIES
)
]
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
sentry[bot] marked this conversation as resolved.

event_counts: dict[int, int] = {}

if error_group_ids:
error_counts = _past_resolved_error_counts(ctx, project, error_group_ids, referrer)
event_counts.update(error_counts)

if perf_group_ids:
perf_counts = _past_resolved_perf_counts(ctx, project, perf_group_ids, referrer)
event_counts.update(perf_counts)

# has_link is initially False; updated by fetch_past_resolved_issue_links at org level
scored = []
for group_id, count in event_counts.items():
group = group_id_to_group.get(group_id)
if group is None:
continue
scored.append((group, count, False))

scored.sort(key=lambda x: x[1], reverse=True)
return scored


def _past_resolved_error_counts(
ctx: OrganizationReportContext,
project: Project,
group_ids: list[int],
referrer: str,
) -> dict[int, int]:
events_entity = Entity("events", alias="events")
group_attributes_entity = Entity("group_attributes", alias="group_attributes")
query = Query(
match=Join([Relationship(events_entity, "attributes", group_attributes_entity)]),
select=[Column("group_id", entity=events_entity), Function("count", [])],
where=[
Condition(Column("timestamp", entity=events_entity), Op.GTE, ctx.start),
Condition(
Column("timestamp", entity=events_entity),
Op.LT,
ctx.end + timedelta(days=1),
),
Condition(Column("project_id", entity=events_entity), Op.EQ, project.id),
Condition(Column("project_id", entity=group_attributes_entity), Op.EQ, project.id),
Condition(
Column("group_id", entity=events_entity),
Op.IN,
group_ids,
),
Condition(
Column("group_status", entity=group_attributes_entity),
Op.EQ,
GroupStatus.RESOLVED,
),
],
groupby=[Column("group_id", entity=events_entity)],
orderby=[OrderBy(Function("count", []), Direction.DESC)],
limit=Limit(len(group_ids)),
)

request = Request(
dataset=Dataset.Events.value,
app_id="reports",
query=query,
tenant_ids={"organization_id": ctx.organization.id},
)
rows = raw_snql_query(request, referrer=referrer)["data"]
return {row["events.group_id"]: row["count()"] for row in rows}


def _past_resolved_perf_counts(
ctx: OrganizationReportContext,
project: Project,
group_ids: list[int],
referrer: str,
) -> dict[int, int]:
query = Query(
match=Entity("search_issues"),
select=[Column("group_id"), Function("count", [])],
where=[
Condition(Column("group_id"), Op.IN, group_ids),
Condition(Column("timestamp"), Op.GTE, ctx.start),
Condition(Column("timestamp"), Op.LT, ctx.end + timedelta(days=1)),
Condition(Column("project_id"), Op.EQ, project.id),
],
groupby=[Column("group_id")],
orderby=[OrderBy(Function("count", []), Direction.DESC)],
limit=Limit(len(group_ids)),
)
request = Request(
dataset=Dataset.IssuePlatform.value,
app_id="reports",
query=query,
tenant_ids={"organization_id": ctx.organization.id},
)
rows = raw_snql_query(request, referrer=referrer)["data"]
return {row["group_id"]: row["count()"] for row in rows}


def fetch_past_resolved_issue_links(ctx: OrganizationReportContext) -> None:
all_group_ids: list[int] = []
for project_ctx in ctx.projects_context_map.values():
all_group_ids.extend(
group.id for group, _count, _has_link in project_ctx.past_resolved_issues
)

if not all_group_ids:
return

groups_with_links = set(
GroupLink.objects.filter(
group_id__in=all_group_ids,
linked_type__in=[GroupLink.LinkedType.commit, GroupLink.LinkedType.pull_request],
relationship=GroupLink.Relationship.resolves,
).values_list("group_id", flat=True)
Comment thread
cursor[bot] marked this conversation as resolved.
)

for project_ctx in ctx.projects_context_map.values():
project_ctx.past_resolved_issues = [
(group, count, group.id in groups_with_links)
for group, count, _has_link in project_ctx.past_resolved_issues
]

# Re-sort with link boost applied, then truncate to top 3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont think so

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds interesting -- how much would this overlap with the recommended sort?

for project_ctx in ctx.projects_context_map.values():
project_ctx.past_resolved_issues.sort(
key=lambda x: x[1] * (PAST_ISSUES_LINK_BOOST if x[2] else 1),
reverse=True,
)
project_ctx.past_resolved_issues = project_ctx.past_resolved_issues[:3]
23 changes: 22 additions & 1 deletion src/sentry/tasks/summaries/weekly_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from sentry.tasks.summaries.organization_report_context_factory import (
OrganizationReportContextFactory,
)
from sentry.tasks.summaries.utils import ONE_DAY, OrganizationReportContext
from sentry.tasks.summaries.utils import ONE_DAY, PAST_ISSUES_LINK_BOOST, OrganizationReportContext
from sentry.tasks.summaries.weekly_report_cache import cache_project_metrics
from sentry.taskworker.namespaces import reports_tasks
from sentry.types.group import GroupSubStatus
Expand Down Expand Up @@ -780,6 +780,23 @@ def all_key_performance_issues():

return heapq.nlargest(3, all_key_performance_issues(), lambda d: d["count"])

def past_issues():
def all_past_issues():
for project_ctx in user_projects:
for group, count, has_linked_pr_or_commit in project_ctx.past_resolved_issues:
display = get_group_display(group)
yield {
"count": count,
"group": group,
"title": display["title"],
"message": display["message"],
"has_linked_pr_or_commit": has_linked_pr_or_commit,
"_relevance": count
* (PAST_ISSUES_LINK_BOOST if has_linked_pr_or_commit else 1),
}

return heapq.nlargest(3, all_past_issues(), lambda d: d["_relevance"])
Comment thread
sentry[bot] marked this conversation as resolved.

def issue_summary():
new_substatus_count = 0
escalating_substatus_count = 0
Expand All @@ -800,6 +817,8 @@ def issue_summary():
"total_substatus_count": total_substatus_count,
}

show_past_issues = features.has("organizations:weekly-report-past-issues", ctx.organization)

return {
"organization": ctx.organization,
"start": date_format(local_start),
Expand All @@ -808,6 +827,8 @@ def issue_summary():
"key_errors": key_errors(),
"key_transactions": key_transactions(),
"key_performance_issues": key_performance_issues(),
"past_issues": past_issues() if show_past_issues else [],
"show_past_issues": show_past_issues,
"issue_summary": issue_summary(),
"user_project_count": len(user_projects),
"notification_uuid": notification_uuid,
Expand Down
19 changes: 19 additions & 0 deletions src/sentry/templates/sentry/emails/reports/body.html
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,25 @@ <h4>Most frequent transactions</h4>
{% endfor %}
</div>
{% endif %}
{% if past_issues|length > 0 and not enhanced_privacy %}
<div id="past-resolved-issues">
<h4>Resolved this week</h4>
{% for a in past_issues %}
<div style="display: flex; flex-direction: row; margin-bottom: 8px; align-items: flex-start;">
<div style="width: 10%; font-size: 17px;">{{a.count|small_count:1}}</div>
<div style="width: 65%; min-width: 0;">
{% url 'sentry-organization-issue-detail' issue_id=a.group.id organization_slug=organization.slug as issue_detail %}
{% querystring referrer="weekly_report" notification_uuid=notification_uuid as query %}
<a title="{{a.title}}" style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; width: auto; max-width: 100%; font-size: 17px; line-height: 1.2; margin-bottom: 2px;"
href="{% org_url organization issue_detail query=query %}">{{a.title}}</a>
<div title="{{a.message}}" style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; width: auto; max-width: 100%; max-height: 38px; line-height: 1.2; font-size: 14px; color: #80708F; margin-bottom: 2px;">{{a.message}}</div>
<div style="font-size: 12px; color: #80708F;">{{a.group.project.name}}</div>
</div>
<span style="background-color: #E7E1EC; border-radius: 8px; font-size: 12px; align-self: center; padding: 2px 10px; margin-left: auto; text-align: center;">Resolved</span>
</div>
{% endfor %}
</div>
{% endif %}

</div>
{% endblock %}
Loading
Loading