Skip to content

feat(weekly-report): adding week over week percentage change to total errors and transactions#117037

Merged
amy-chen23 merged 7 commits into
masterfrom
amyc/weekly-report-top-level-metric
Jun 10, 2026
Merged

feat(weekly-report): adding week over week percentage change to total errors and transactions#117037
amy-chen23 merged 7 commits into
masterfrom
amyc/weekly-report-top-level-metric

Conversation

@amy-chen23

@amy-chen23 amy-chen23 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Resolves ID-1587

To test:

  • Start dev environment
  • Go to debug/mail/weekly-reports/
  • Email should render normally -- there shouldn't be any changes to the email content itself

Adds a percentage change indicator next to Total Errors and Total Transactions in the weekly email report, showing how the current week compares to the previous week (e.g., "▲ 50% (vs. last week)"). The metric is displayed as a gray pill badge with a unicode arrow.

Feature Flag
Gated behind feature flag organizations:weekly-report-week-over-week-metric

Previous-week data fetching using cache-first approach

We cache the previous week's counts. If it's a cache miss, then one additional Snuba query per organization fetches last week's accepted error and transaction counts, reusing the existing project_event_counts_for_organization() function with shifted dates. The query is instrumented with @metrics.wraps and sentry_sdk.start_span for cost monitoring.

Percentage display

The percentage is hidden when there's no previous-week data or when the change rounds to 0%. The debug weekly report view also generates randomized previous-week data so the indicator is visible in email previews.

Screenshot (fake data):
Screenshot 2026-06-05 at 3 02 47 PM

@linear-code

linear-code Bot commented Jun 5, 2026

Copy link
Copy Markdown

ID-1587

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 5, 2026
@amy-chen23 amy-chen23 marked this pull request as ready for review June 8, 2026 18:45
@amy-chen23 amy-chen23 requested review from a team as code owners June 8, 2026 18:45
Comment thread src/sentry/snuba/referrer.py Outdated
REPORTS_KEY_TRANSACTIONS = "reports.key_transactions"
REPORTS_OUTCOME_SERIES = "reports.outcome_series"
REPORTS_OUTCOMES = "reports.outcomes"
REPORTS_OUTCOMES_PREVIOUS_WEEK = "reports.outcomes_previous_week"

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.

it might be worth using the same referrer here. the logic is that snuba referrers can correspond to rate limits (as in a particular referrer gets a certain rate limit), so it makes more sense for the entire report to be rate limited together, rather than breaking it up.

}


def _pct_change(current: int, previous: int) -> dict[str, Any] | None:

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.

returning this in float format might be easier. e.g. return -0.12 instead of a string of '12%' and 'is_increase: False'

<td class="project-breakdown-graph-cell {% if has_replay_graph %}errors-wide{% else %}errors{% endif %}" {% if has_replay_graph %}colspan="2"{% endif %}>
<h4 class="total-count-title">Total Project Errors</h4>
<h1 style="margin: 0;" class="total-count">{{ trends.total_error_count|small_count:1 }}</h1>
<h1 style="margin: 0;" class="total-count">

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.

is there a way to feature flag some of these changes? it would be nice to roll this out to e.g. only sentry, or to a small subset of customers before rolling it out globally. not sure how to do it with emails though.

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.

HTML templates can't have feature flag checks, but the logic that creates email template context can have feature flag checks. You could put the result of a feature flag check into a context variable and use it in the template.

@amy-chen23 amy-chen23 marked this pull request as draft June 9, 2026 16:55
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 10, 2026
@github-actions

This comment was marked as resolved.

@amy-chen23 amy-chen23 force-pushed the amyc/weekly-report-top-level-metric branch from 2c01c64 to cf71ff6 Compare June 10, 2026 18:58
@amy-chen23 amy-chen23 marked this pull request as ready for review June 10, 2026 20:44
return render_template_context(ctx, user_id)
context = render_template_context(ctx, user_id)
if context is not None:
context["show_week_over_week_metric"] = (

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.

confirms that feature flag organizations:weekly-report-week-over-week-metric hides frontend HTML changes

from taskbroker_client.worker.workerchild import ProcessingDeadlineExceeded

from sentry import analytics
from sentry import analytics, features

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.

Stale cached prior-week counts

Medium Severity

Week-over-week reads Redis first and only queries Snuba on a miss. Cached metrics are written only for non-empty projects when a report is sent, so a quiet week leaves old totals on the key. The next active week can compare against the wrong baseline and show misleading percentages.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 489d941. 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.

10 day TTL prevents this


# Snuba fallback for cache misses (e.g. new projects, first report run)
prev_start = ctx.start - timedelta(days=7)
prev_end = ctx.end - timedelta(days=7)

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.

not a big deal but could probably use

prev_start = ctx.start - (ctx.end - ctx.start)
prev_end = ctx.start

so the 7-day length doesn't have to be hard-coded anywhere

"error_pct_change": _pct_change(total_error, prev_week_error),
"error_pct_change_display": _format_pct(_pct_change(total_error, prev_week_error)),
"transaction_pct_change": _pct_change(total_transaction, prev_week_transaction),
"transaction_pct_change_display": _format_pct(

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.

nit: is it necessary to pass both a change var and a change_display var? could we just have a single change var which is either the formatted string or None, and then in the template we display it based on whether or not it's None?

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.

yea i agree that we can just have a single formatted change_var string

}


def _pct_change(current: int, previous: int) -> float | None:

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.

up to you but maybe would be good to throw these into utils.py to keep this file cleaner?

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.

since we're combining change and change_display vars, i also combined the 2 percentage change functions. Since all the the formatting logic is in that 1 percentage change function, i'll prob keep it in weekly_reports.py

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 25c7cd0. Configure here.


missed_project_ids = set(project_ids) - set(cached.keys())
if not missed_project_ids:
return

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.

Stale cache skews week comparison

Medium Severity

Previous-week totals are taken from Redis on any cache hit, but keys are not tied to a report period and empty weeks never refresh or clear stored values. A later run can compare this week to counts from an older skipped or quiet week while still labeling the change as vs. last week.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 25c7cd0. 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.

10 day TTL prevents this

@amy-chen23 amy-chen23 merged commit 2381228 into master Jun 10, 2026
85 checks passed
@amy-chen23 amy-chen23 deleted the amyc/weekly-report-top-level-metric branch June 10, 2026 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants