Skip to content

feat(issues): Track weekly report sends and clicks #54744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 15, 2023
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
1 change: 1 addition & 0 deletions src/sentry/analytics/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@
from .user_created import * # noqa: F401,F403
from .user_signup import * # noqa: F401,F403
from .webhook_repository_created import * # noqa: F401,F403
from .weekly_report import * # noqa: F401,F403
15 changes: 15 additions & 0 deletions src/sentry/analytics/events/weekly_report.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from sentry import analytics


class WeeklyReportSent(analytics.Event):
type = "weekly_report.sent"

attributes = (
analytics.Attribute("organization_id"),
analytics.Attribute("user_id"),
analytics.Attribute("notification_uuid"),
analytics.Attribute("user_project_count", type=int),
)


analytics.register(WeeklyReportSent)
9 changes: 9 additions & 0 deletions src/sentry/notifications/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import annotations

import logging
import random
import string
import time
from collections import defaultdict
from dataclasses import dataclass
Expand Down Expand Up @@ -441,6 +443,13 @@ def get_replay_id(event: Event | GroupEvent) -> str | None:
return replay_id


def generate_notification_uuid() -> str:
"""
Generates a random string of 16 characters to be used as a notification uuid
"""
return "".join(random.choices(string.ascii_letters + string.digits, k=16))


@dataclass
class PerformanceProblemContext:
problem: PerformanceProblem
Expand Down
19 changes: 17 additions & 2 deletions src/sentry/tasks/weekly_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from snuba_sdk.orderby import Direction, OrderBy
from snuba_sdk.query import Limit, Query

from sentry import features
from sentry import analytics, features
from sentry.api.serializers.snuba import zerofill
from sentry.constants import DataCategory
from sentry.models import (
Expand All @@ -32,6 +32,7 @@
OrganizationMember,
OrganizationStatus,
)
from sentry.notifications.utils import generate_notification_uuid
from sentry.services.hybrid_cloud.user_option import user_option_service
from sentry.silo import SiloMode
from sentry.snuba.dataset import Dataset
Expand Down Expand Up @@ -739,6 +740,8 @@ def render_template_context(ctx, user_id):
"organizations:session-replay", ctx.organization
) and features.has("organizations:session-replay-weekly-email", ctx.organization)

notification_uuid = generate_notification_uuid()

# Render the first section of the email where we had the table showing the
# number of accepted/dropped errors/transactions for each project.
def trends():
Expand Down Expand Up @@ -793,7 +796,9 @@ def sum_event_counts(project_ctxs):
legend = [
{
"slug": project_ctx.project.slug,
"url": project_ctx.project.get_absolute_url(),
"url": project_ctx.project.get_absolute_url(
params={"referrer": "weekly_report", "notification_uuid": notification_uuid}
),
"color": project_breakdown_colors[i],
"dropped_error_count": project_ctx.dropped_error_count,
"accepted_error_count": project_ctx.accepted_error_count,
Expand Down Expand Up @@ -1031,6 +1036,8 @@ def issue_summary():
"key_performance_issues": key_performance_issues(),
"key_replays": key_replays() if has_replay_section else [],
"issue_summary": issue_summary(),
"user_project_count": len(user_projects),
"notification_uuid": notification_uuid,
}


Expand All @@ -1052,6 +1059,14 @@ def send_email(ctx, user_id, dry_run=False, email_override=None):
)
if dry_run:
return
else:
analytics.record(
"weekly_report.sent",
user_id=user_id,
organization_id=ctx.organization.id,
notification_uuid=template_ctx["notification_uuid"],
user_project_count=template_ctx["user_project_count"],
)
if email_override:
message.send(to=(email_override,))
else:
Expand Down
25 changes: 16 additions & 9 deletions src/sentry/templates/sentry/emails/reports/body.html
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ <h1>
<h4 class="total-count-title">Total Project Errors</h4>
<h1 style="margin: 0;" class="total-count">{{ trends.total_error_count|small_count:1 }}</h1>
{% url 'sentry-organization-issue-list' organization.slug as issue_list %}
<a href="{% org_url organization issue_list query="referrer=weekly-email" %}"
{% querystring referrer="weekly_report" notification_uuid=notification_uuid as query %}
<a href="{% org_url organization issue_list query=query %}"
style="font-size: 12px; margin-bottom: 16px; display: block;">View All Errors</a>

<table class="graph">
Expand Down Expand Up @@ -274,7 +275,8 @@ <h1 style="margin: 0;" class="total-count">{{ trends.total_error_count|small_cou
<h4 class="total-count-title">Total Project Transactions</h4>
<h1 style="margin: 0;" class="total-count">{{ trends.total_transaction_count|small_count:1 }}</h1>
{% url 'sentry-organization-perfomance' organization.slug as performance_landing %}
<a href="{% org_url organization performance_landing query='referrer=weekly_email_view_all' %}"
{% querystring referrer="weekly_report_view_all" notification_uuid=notification_uuid as query %}
<a href="{% org_url organization performance_landing query=query %}"
style="font-size: 12px; margin-bottom: 16px; display: block;">View All Transactions</a>
<table class="graph">
<tr>
Expand Down Expand Up @@ -310,7 +312,8 @@ <h1 style="margin: 0;" class="total-count">{{ trends.total_transaction_count|sma
<h1 style="font-weight: bold; font-size: 17px;">Something slow?</h1>
<p style="font-size: 11px;">Trace those 10-second page loads to poor-performing API calls.</p>
{% url 'sentry-organization-performance' organization.slug as performance_landing %}
<a href="{% org_url organization performance_landing query='referrer=weekly_email_upsell' %}"
{% querystring referrer="weekly_report_upsell" notification_uuid=notification_uuid as query %}
<a href="{% org_url organization performance_landing query=query %}"
class="btn" style="margin-top: 8px;">Set Up Performance</a>
</div>
</td>
Expand All @@ -321,7 +324,8 @@ <h1 style="font-weight: bold; font-size: 17px;">Something slow?</h1>
<h4 class="total-count-title">Total Project Replays</h4>
<h1 style="margin: 0;" class="total-count">{{ trends.total_replay_count|small_count:1 }}</h1>
{% url 'sentry-organization-replays' organization.slug as replay_landing %}
<a href="{% org_url organization replay_landing query='referrer=weekly_email_view_all' %}"
{% querystring referrer="weekly_report_view_all" notification_uuid=notification_uuid as query %}
<a href="{% org_url organization replay_landing query=query %}"
style="font-size: 12px; margin-bottom: 16px; display: block;">View All Replays</a>
<table class="graph">
<tr>
Expand Down Expand Up @@ -360,7 +364,8 @@ <h1 style="margin: 0;" class="total-count">{{ trends.total_replay_count|small_co
<h1 style="font-weight: bold; font-size: 17px;">Tricky bug?</h1>
<p style="font-size: 11px;">Rewind and replay every step of a user’s journey before and after they encounter an issue.</p>
{% url 'sentry-organization-replays' organization.slug as replay_landing %}
<a href="{% org_url organization replay_landing query='referrer=weekly_email_upsell' %}" class="btn"
{% querystring referrer="weekly_report_upsell" notification_uuid=notification_uuid as query %}
<a href="{% org_url organization replay_landing query=querystring %}" class="btn"
style="margin-top: 8px;">Set Up Session Replay</a>
</div>
</td>
Expand Down Expand Up @@ -479,8 +484,9 @@ <h4>Issues with the most errors</h4>
<div style="width: 10%; font-size: 17px;">{{a.count|small_count:1}}</div>
<div style="width: 65%;">
{% 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 style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; font-size: 17px; height: 24px;"
href="{% org_url organization issue_detail query='referrer=weekly-email' %}">{{a.group.message}}</a>
href="{% org_url organization issue_detail query=query %}">{{a.group.message}}</a>
<div style="font-size: 12px; color: #80708F;">{{a.group.project.name}}</div>
</div>
{% if a.group_substatus and a.group_substatus_color %}
Expand All @@ -500,8 +506,9 @@ <h4>Most frequent performance issues</h4>
<div style="width: 10%; font-size: 17px;">{{a.count|small_count:1}}</div>
<div style="width: 65%;">
{% 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 style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; font-size: 17px; height: 24px;"
href="{% org_url organization issue_detail query="referrer=weekly-email" %}">{{a.group.message}}</a>
href="{% org_url organization issue_detail query=query %}">{{a.group.message}}</a>
<div style="font-size: 12px; color: #80708F;">{{a.group.get_type_display}}</div>
</div>
<span style="background-color: {{a.status_color}}; border-radius: 8px; font-size: 12px; align-self: center; padding: 2px 10px; margin-left: auto; height: 100%;">{{a.status}}</span>
Expand All @@ -516,8 +523,8 @@ <h4>Most frequent transactions</h4>
<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%;">
{% querystring project=a.project.id transaction=a.name referrer="weekly_report" as query %}
{% url 'sentry-organization-performance-summary' organization.slug as performance_summary %}
{% querystring project=a.project.id transaction=a.name referrer="weekly_report" notification_uuid=notification_uuid as query %}
<a style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; font-size: 17px;"
href="{% org_url organization performance_summary query=query %}">{{a.name}}</a>
<div style="font-size: 12px; color: #80708F;">{{a.project.name}}</div>
Expand All @@ -541,8 +548,8 @@ <h4>Most erroneous replays</h4>
<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%;">
{% querystring referrer="weekly_report" as query %}
{% url 'sentry-organization-replay-details' organization.slug a.replay.id as replay_details %}
{% querystring referrer="weekly_report" notification_uuid=notification_uuid as query %}
<a style="display: block; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; font-size: 17px;"
href="{% org_url organization replay_details query=query %}">{{a.id}}</a>
<div style="font-size: 12px; color: #80708F;">{{a.project.name}}</div>
Expand Down
29 changes: 19 additions & 10 deletions tests/sentry/tasks/test_weekly_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

@region_silo_test(stable=True)
class WeeklyReportsTest(OutcomesSnubaTest, SnubaTestCase):
@with_feature("organizations:weekly-email-refresh")
@freeze_time(before_now(days=2).replace(hour=0, minute=0, second=0, microsecond=0))
def test_integration(self):
with unguarded_write(using=router.db_for_write(Project)):
Expand Down Expand Up @@ -89,7 +88,6 @@ def test_with_empty_string_user_option(self):
assert self.organization.name in message.subject

@with_feature("organizations:customer-domains")
@with_feature("organizations:weekly-email-refresh")
Copy link
Member Author

Choose a reason for hiding this comment

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

went looking for this flag, seems to be gone

@freeze_time(before_now(days=2).replace(hour=0, minute=0, second=0, microsecond=0))
def test_message_links_customer_domains(self):
with unguarded_write(using=router.db_for_write(Project)):
Expand All @@ -114,9 +112,10 @@ def test_message_links_customer_domains(self):
assert isinstance(message, EmailMultiAlternatives)
assert self.organization.name in message.subject
html = message.alternatives[0][0]

assert isinstance(html, str)
assert (
f"http://{self.organization.slug}.testserver/issues/?referrer=weekly-email" in html
f"http://{self.organization.slug}.testserver/issues/?referrer=weekly_report" in html
)

@mock.patch("sentry.tasks.weekly_reports.send_email")
Expand Down Expand Up @@ -249,16 +248,16 @@ def test_organization_project_issue_substatus_summaries(self):
assert project_ctx.regression_substatus_count == 0
assert project_ctx.total_substatus_count == 2

@mock.patch("sentry.analytics.record")
@mock.patch("sentry.tasks.weekly_reports.MessageBuilder")
def test_message_builder_simple(self, message_builder):
def test_message_builder_simple(self, message_builder, record):
now = django_timezone.now()

two_days_ago = now - timedelta(days=2)
three_days_ago = now - timedelta(days=3)

self.create_member(
teams=[self.team], user=self.create_user(), organization=self.organization
)
user = self.create_user()
self.create_member(teams=[self.team], user=user, organization=self.organization)

event1 = self.store_event(
data={
Expand Down Expand Up @@ -345,6 +344,16 @@ def test_message_builder_simple(self, message_builder):
assert context["trends"]["total_transaction_count"] == 10
assert "Weekly Report for" in message_params["subject"]

assert isinstance(context["notification_uuid"], str)

record.assert_called_with(
"weekly_report.sent",
user_id=user.id,
organization_id=self.organization.id,
notification_uuid=mock.ANY,
user_project_count=1,
)

@mock.patch("sentry.tasks.weekly_reports.MessageBuilder")
@with_feature("organizations:escalating-issues")
def test_message_builder_substatus_simple(self, message_builder):
Expand Down Expand Up @@ -459,7 +468,7 @@ def test_message_builder_advanced(self, message_builder):

assert ctx["trends"]["legend"][0] == {
"slug": "bar",
"url": f"http://testserver/organizations/baz/issues/?project={self.project.id}",
"url": f"http://testserver/organizations/baz/issues/?referrer=weekly_report&notification_uuid={ctx['notification_uuid']}&project={self.project.id}",
"color": "#422C6E",
"dropped_error_count": 2,
"accepted_error_count": 1,
Expand Down Expand Up @@ -498,7 +507,7 @@ def test_empty_report(self, mock_send_email):
assert mock_send_email.call_count == 0

@with_feature("organizations:session-replay")
@with_feature("organizations:session-replay-weekly-email")
@with_feature("organizations:session-replay-weekly_report")
@mock.patch("sentry.tasks.weekly_reports.MessageBuilder")
def test_message_builder_replays(self, message_builder):

Expand Down Expand Up @@ -529,7 +538,7 @@ def test_message_builder_replays(self, message_builder):

assert ctx["trends"]["legend"][0] == {
"slug": "bar",
"url": f"http://testserver/organizations/baz/issues/?project={self.project.id}",
"url": f"http://testserver/organizations/baz/issues/?referrer=weekly_report&notification_uuid={ctx['notification_uuid']}&project={self.project.id}",
"color": "#422C6E",
"dropped_error_count": 0,
"accepted_error_count": 0,
Expand Down