feat(alerts): Update alert trigger email (#19540)

src/sentry/incidents/action_handlers.py

@@ -41,11 +41,6 @@ class ActionHandler(object):
     [AlertRuleTriggerAction.TargetType.USER, AlertRuleTriggerAction.TargetType.TEAM],
 )
 class EmailActionHandler(ActionHandler):
-    query_aggregates_display = {
-        "count()": "Total Events",
-        "count_unique(tags[sentry:user])": "Total Unique Users",
-    }
-
     def get_targets(self):
         target = self.action.target
         if not target:
@@ -75,7 +70,9 @@ class EmailActionHandler(ActionHandler):
         self.email_users(TriggerStatus.RESOLVED)
 
     def email_users(self, status):
-        email_context = self.generate_email_context(status)
+        email_context = generate_incident_trigger_email_context(
+            self.project, self.incident, self.action.alert_rule_trigger, status
+        )
         for user_id, email in self.get_targets():
             self.build_message(email_context, status, user_id).send_async(to=[email])
 
@@ -91,55 +88,6 @@ class EmailActionHandler(ActionHandler):
             context=context,
         )
 
-    def generate_email_context(self, status):
-        trigger = self.action.alert_rule_trigger
-        alert_rule = trigger.alert_rule
-        snuba_query = alert_rule.snuba_query
-        is_active = status == TriggerStatus.ACTIVE
-        is_threshold_type_above = trigger.threshold_type == AlertRuleThresholdType.ABOVE.value
-
-        # if alert threshold and threshold type is above then show '>'
-        # if resolve threshold and threshold type is *BELOW* then show '>'
-        # we can simplify this to be the below statement
-        show_greater_than_string = is_active == is_threshold_type_above
-        environment_string = snuba_query.environment.name if snuba_query.environment else "All"
-        aggregate = alert_rule.snuba_query.aggregate
-        return {
-            "link": absolute_uri(
-                reverse(
-                    "sentry-metric-alert",
-                    kwargs={
-                        "organization_slug": self.incident.organization.slug,
-                        "incident_id": self.incident.identifier,
-                    },
-                )
-            ),
-            "rule_link": absolute_uri(
-                reverse(
-                    "sentry-alert-rule",
-                    kwargs={
-                        "organization_slug": self.incident.organization.slug,
-                        "project_slug": self.project.slug,
-                        "alert_rule_id": self.action.alert_rule_trigger.alert_rule_id,
-                    },
-                )
-            ),
-            "incident_name": self.incident.title,
-            "environment": environment_string,
-            "time_window": format_duration(snuba_query.time_window / 60),
-            "triggered_at": trigger.date_added,
-            "aggregate": self.query_aggregates_display.get(aggregate, aggregate),
-            "query": snuba_query.query,
-            "threshold": trigger.alert_threshold if is_active else trigger.resolve_threshold,
-            # if alert threshold and threshold type is above then show '>'
-            # if resolve threshold and threshold type is *BELOW* then show '>'
-            "threshold_direction_string": ">" if show_greater_than_string else "<",
-            "status": INCIDENT_STATUS[IncidentStatus(self.incident.status)],
-            "is_critical": self.incident.status == IncidentStatus.CRITICAL,
-            "is_warning": self.incident.status == IncidentStatus.WARNING,
-            "unsubscribe_link": None,
-        }
-
 
 @AlertRuleTriggerAction.register_type(
     "slack",
@@ -179,3 +127,63 @@ def format_duration(minutes):
 
     seconds = minutes / 60
     return "{} second{}".format(seconds, pluralize(seconds))
+
+
+INCIDENT_STATUS_KEY = {
+    IncidentStatus.OPEN: "open",
+    IncidentStatus.CLOSED: "resolved",
+    IncidentStatus.CRITICAL: "critical",
+    IncidentStatus.WARNING: "warning",
+}
+
+
+def generate_incident_trigger_email_context(project, incident, alert_rule_trigger, status):
+    trigger = alert_rule_trigger
+    alert_rule = trigger.alert_rule
+    snuba_query = alert_rule.snuba_query
+    is_active = status == TriggerStatus.ACTIVE
+    is_threshold_type_above = trigger.threshold_type == AlertRuleThresholdType.ABOVE.value
+
+    # if alert threshold and threshold type is above then show '>'
+    # if resolve threshold and threshold type is *BELOW* then show '>'
+    # we can simplify this to be the below statement
+    show_greater_than_string = is_active == is_threshold_type_above
+    environment_string = snuba_query.environment.name if snuba_query.environment else "All"
+    aggregate = alert_rule.snuba_query.aggregate
+    return {
+        "link": absolute_uri(
+            reverse(
+                "sentry-metric-alert",
+                kwargs={
+                    "organization_slug": incident.organization.slug,
+                    "incident_id": incident.identifier,
+                },
+            )
+        ),
+        "rule_link": absolute_uri(
+            reverse(
+                "sentry-alert-rule",
+                kwargs={
+                    "organization_slug": incident.organization.slug,
+                    "project_slug": project.slug,
+                    "alert_rule_id": trigger.alert_rule_id,
+                },
+            )
+        ),
+        "project_slug": project.slug,
+        "incident_name": incident.title,
+        "environment": environment_string,
+        "time_window": format_duration(snuba_query.time_window / 60),
+        "triggered_at": trigger.date_added,
+        "aggregate": aggregate,
+        "query": snuba_query.query,
+        "threshold": trigger.alert_threshold if is_active else trigger.resolve_threshold,
+        # if alert threshold and threshold type is above then show '>'
+        # if resolve threshold and threshold type is *BELOW* then show '>'
+        "threshold_direction_string": ">" if show_greater_than_string else "<",
+        "status": INCIDENT_STATUS[IncidentStatus(incident.status)],
+        "status_key": INCIDENT_STATUS_KEY[IncidentStatus(incident.status)],
+        "is_critical": incident.status == IncidentStatus.CRITICAL,
+        "is_warning": incident.status == IncidentStatus.WARNING,
+        "unsubscribe_link": None,
+    }

src/sentry/templates/sentry/emails/email-styles.html

@@ -275,6 +275,7 @@
   .inner table th {
     min-width: 60px;
     color: #968ba0;
+    padding: 10px 0 0 0;
   }
   .interface {
     margin-bottom: 30px;
@@ -753,8 +754,11 @@
     font-weight: bold;
   }
 
-  h3.alert-title {
-    margin-bottom: 0;
+  .alert-title {
+    margin-bottom: 24px;
+  }
+  .alert-title h3 {
+    margin: 0;
   }
 
   .alert-header {
@@ -764,21 +768,68 @@
     text-transform: uppercase;
   }
 
-  table.alert-rule-table thead th {
+  .alert-badge {
+    display: inline-block;
+    border-radius: 3px;
+    text-transform: uppercase;
+    font-size: 12px;
+    color: #fff;
+    background: #C6BECF;
+    padding: 4px 8px;
+    margin-right: 12px;
+  }
+
+  .alert-info-container {
+    border-radius: 3px;
+    padding: 16px;
+  }
+
+  .alert-info-container table {
+    margin: 0;
+  }
+
+  .alert-info-container h4 {
+    margin: 0 0 12px;
+    display: flex;
+    align-items: center;
+  }
+
+  .alert-container-open {
+    background: #FAF9FB ;
+    border: 1px solid #D3CCDA;
+  }
+
+  .alert-container-resolved {
+    background: #F5FFFB;
+    border: 1px solid #4DC771;
+  }
+  .alert-container-resolved .alert-badge {
+    background: #4DC771;
+  }
+
+  .alert-container-critical {
+    background: #FFF5F7;
+    border: 1px solid #FA4747;
+  }
+  .alert-container-critical .alert-badge {
+    background: #FA4747;
+  }
+
+  .alert-container-warning {
+    background: #FFFDF5;
+    border: 1px solid #FFC227;
+  }
+  .alert-container-warning .alert-badge {
+    background: #FFC227;
+  }
+
+  table.alert-rule-table th {
     font-size: 12px;
     color: #9585A3;
     font-weight: bold;
     text-transform: uppercase;
   }
 
-  .alert-query-box {
-    background-color: #E7E1EC;
-    border-radius: 4px;
-    padding: 8px;
-    color: #645574;
-    margin-top: 12px;
-  }
-
   .alert-critical {
     fill: #e03e2f;
     font-weight: bold;

src/sentry/templates/sentry/emails/incidents/trigger.html

@@ -5,87 +5,76 @@
 {% load sentry_assets %}
 
 {% block activity %}
-    <table>
-      <tr>
-        <td>
-        <h3 class="alert-title">
-            <a class="alert-title-link" href="{{ link }}">{{ incident_name }}</a>
-        </h3>
-        </td>
-        <td>
-          <div class="alert-header">
-          Status
-          </div>
-        </td>
-      </tr>
-      <tr>
-        <td>
-          Created on {{ triggered_at }}
-        </td>
-        <td>
-          {% if is_critical %}
-            <img class="alert-critical" src="{% absolute_asset_url 'sentry' 'images/email/icon-circle-exclamation.png' %}" width="16px" height="16px" alt="Critical" />
-          {% elif is_warning %}
-            <img class="alert-warning" src="{% absolute_asset_url 'sentry' 'images/email/icon-warning.png' %}" width="16px" height="16px" alt="Warning" />
-          {% endif %}
-
-          <span class="alert-status">{{ status }}</span>
-        </td>
-      </tr>
-    </table>
-
-
-    <hr />
+  <div class="alert-title">
+    <h3>
+      <a class="alert-title-link" href="{{ link }}">{{ incident_name }}</a>
+    </h3>
+    <p>
+      Triggered on {{ triggered_at }}
+    </p>
+  </div>
 
   {% if enhanced_privacy %}
     <div class="notice">
       Details about this alert are not shown in this email since enhanced privacy
       controls are enabled. For more details about this alert, <a href="{{ link }}">view on Sentry.</a>
     </div>
-
   {% else %}
+    <div class="alert-info-container alert-container-{{ status_key }}">
+      <h4>
+        <span class="alert-badge">{{ status }} alert</span>
+      </h4>
       <table class="alert-rule-table">
         <thead>
+          <th>
+            Threshold
+          </th>
           <th>
             Metric
           </th>
           <th>
             Environment
           </th>
+        </thead>
+
+        <tbody>
+        <tr>
+          <td>
+            {{ threshold_direction_string }} {{ threshold }}
+          </td>
+          <td>
+            {{ aggregate }}
+          </td>
+          <td>
+            {{ environment }}
+          </td>
+        </tbody>
+
+        <thead>
+          <th>
+            Time Interval
+          </th>
           <th>
-            Threshold
+            Filter
           </th>
           <th>
-            Time Interval
+            Project
           </th>
         </thead>
 
         <tbody>
         <tr>
           <td>
-            {{ aggregate }}
+            {{ time_window }}
           </td>
           <td>
-            {{ environment }}
-          </td>
-        <td>
-          {{ threshold_direction_string }} {{ threshold }}
+            {{ query }}
           </td>
           <td>
-            {{ time_window }}
+            {{ project_slug }}
           </td>
-
-
         </tbody>
       </table>
-
-      <div>
-        <div class="alert-header">
-          Query
-        </div>
-        <div class="alert-query-box">
-          {{ query }}
-        </div>
-      </div>
+    </div>
   {% endif %}
 {% endblock %}

src/sentry/templates/sentry/emails/incidents/trigger.txt

@@ -10,6 +10,7 @@ controls are enabled. For more details about this alert alert, view on Sentry:
 Alert: {{ link }}
 Rule: {{ rule_link }}
 Status: {{ status }}
+Project: {{ project_slug }}
 
 Metric: {{ aggregate }}
 Environment: {{ environment }}

src/sentry/web/debug_urls.py

@@ -5,7 +5,6 @@ from django.views.generic import TemplateView
 
 import sentry.web.frontend.debug.mail
 
-from sentry.web.frontend.debug.debug_alert_rule_trigger_email import DebugAlertRuleTriggerEmailView
 from sentry.web.frontend.debug.debug_assigned_email import (
     DebugAssignedEmailView,
     DebugSelfAssignedEmailView,
@@ -14,6 +13,7 @@ from sentry.web.frontend.debug.debug_assigned_email import (
 from sentry.web.frontend.debug.debug_trigger_error import DebugTriggerErrorView
 from sentry.web.frontend.debug.debug_error_embed import DebugErrorPageEmbedView
 from sentry.web.frontend.debug.debug_incident_activity_email import DebugIncidentActivityEmailView
+from sentry.web.frontend.debug.debug_incident_trigger_email import DebugIncidentTriggerEmailView
 from sentry.web.frontend.debug.debug_invalid_identity_email import DebugInvalidIdentityEmailView
 from sentry.web.frontend.debug.debug_organization_invite_request import (
     DebugOrganizationInviteRequestEmailView,
@@ -111,8 +111,8 @@ urlpatterns = [
     url(r"^debug/mail/sso-linked/$", DebugSsoLinkedEmailView.as_view()),
     url(r"^debug/mail/sso-unlinked/$", DebugSsoUnlinkedEmailView.as_view()),
     url(r"^debug/mail/sso-unlinked/no-password$", DebugSsoUnlinkedNoPasswordEmailView.as_view()),
-    url(r"^debug/mail/alert-rule-trigger$", DebugAlertRuleTriggerEmailView.as_view()),
     url(r"^debug/mail/incident-activity$", DebugIncidentActivityEmailView.as_view()),
+    url(r"^debug/mail/incident-trigger$", DebugIncidentTriggerEmailView.as_view()),
     url(r"^debug/mail/setup-2fa/$", DebugSetup2faEmailView.as_view()),
     url(r"^debug/embed/error-page/$", DebugErrorPageEmbedView.as_view()),
     url(r"^debug/trigger-error/$", DebugTriggerErrorView.as_view()),

src/sentry/web/frontend/debug/debug_alert_rule_trigger_email.py

@@ -1,45 +0,0 @@
-from __future__ import absolute_import, print_function
-
-from django.views.generic import View
-
-from sentry.incidents.models import (
-    Incident,
-    AlertRule,
-    AlertRuleTrigger,
-    AlertRuleTriggerAction,
-    TriggerStatus,
-    IncidentStatus,
-)
-from sentry.models.project import Project
-from sentry.models.organization import Organization
-from sentry.incidents.action_handlers import EmailActionHandler
-
-from .mail import MailPreview
-
-
-class DebugAlertRuleTriggerEmailView(View):
-    def get(self, request):
-        organization = Organization(slug="myorg")
-        project = Project(id=30, slug="myproj")
-
-        incident = Incident(
-            identifier=123,
-            organization=organization,
-            title="Something broke",
-            status=IncidentStatus.CRITICAL,
-        )
-        alert_rule = AlertRule(
-            id=1, organization=organization, aggregation=1, query="is:unresolved", time_window=60
-        )
-        alert_rule_trigger = AlertRuleTrigger(
-            id=5, alert_rule=alert_rule, alert_threshold=100, resolve_threshold=50
-        )
-        action = AlertRuleTriggerAction(id=10, alert_rule_trigger=alert_rule_trigger)
-
-        handler = EmailActionHandler(action, incident, project)
-        email = handler.build_message(
-            handler.generate_email_context(TriggerStatus.ACTIVE), TriggerStatus.ACTIVE, 1
-        )
-        return MailPreview(
-            html_template=email.html_template, text_template=email.template, context=email.context
-        ).render(request)

src/sentry/web/frontend/debug/debug_incident_trigger_email.py

@@ -0,0 +1,47 @@
+from __future__ import absolute_import, print_function
+
+from django.views.generic import View
+
+from sentry.models import Organization, Project
+from sentry.snuba.models import SnubaQuery
+from sentry.incidents.action_handlers import generate_incident_trigger_email_context
+from sentry.incidents.models import (
+    Incident,
+    AlertRule,
+    AlertRuleTrigger,
+    TriggerStatus,
+    IncidentStatus,
+)
+
+
+from .mail import MailPreview
+
+
+class DebugIncidentTriggerEmailView(View):
+    def get(self, request):
+        organization = Organization(slug="myorg")
+        project = Project(slug="myproject", organization=organization)
+
+        query = SnubaQuery(
+            time_window=60, query="transaction:/some/transaction", aggregate="count()"
+        )
+        alert_rule = AlertRule(id=1, organization=organization, name="My Alert", snuba_query=query)
+        incident = Incident(
+            id=2,
+            identifier=123,
+            organization=organization,
+            title="Something broke",
+            alert_rule=alert_rule,
+            status=IncidentStatus.CRITICAL,
+        )
+        trigger = AlertRuleTrigger(alert_rule=alert_rule)
+
+        context = generate_incident_trigger_email_context(
+            project, incident, trigger, TriggerStatus.ACTIVE
+        )
+
+        return MailPreview(
+            text_template=u"sentry/emails/incidents/trigger.txt",
+            html_template=u"sentry/emails/incidents/trigger.html",
+            context=context,
+        ).render(request)

tests/sentry/incidents/test_action_handlers.py

@@ -11,7 +11,12 @@ from exam import fixture
 from freezegun import freeze_time
 from six.moves.urllib.parse import parse_qs
 
-from sentry.incidents.action_handlers import EmailActionHandler, SlackActionHandler
+from sentry.incidents.action_handlers import (
+    EmailActionHandler,
+    SlackActionHandler,
+    generate_incident_trigger_email_context,
+    INCIDENT_STATUS_KEY,
+)
 from sentry.incidents.logic import update_incident_status
 from sentry.incidents.models import (
     AlertRuleTriggerAction,
@@ -85,7 +90,6 @@ class EmailActionHandlerGenerateEmailContextTest(TestCase):
         status = TriggerStatus.ACTIVE
         action = self.create_alert_rule_trigger_action()
         incident = self.create_incident()
-        handler = EmailActionHandler(action, incident, self.project)
         aggregate = action.alert_rule_trigger.alert_rule.snuba_query.aggregate
         expected = {
             "link": absolute_uri(
@@ -108,19 +112,23 @@ class EmailActionHandlerGenerateEmailContextTest(TestCase):
                 )
             ),
             "incident_name": incident.title,
-            "aggregate": handler.query_aggregates_display.get(aggregate, aggregate),
+            "aggregate": aggregate,
             "query": action.alert_rule_trigger.alert_rule.snuba_query.query,
             "threshold": action.alert_rule_trigger.alert_threshold,
             "status": INCIDENT_STATUS[IncidentStatus(incident.status)],
+            "status_key": INCIDENT_STATUS_KEY[IncidentStatus(incident.status)],
             "environment": "All",
             "is_critical": False,
             "is_warning": False,
             "threshold_direction_string": ">",
             "time_window": "10 minutes",
             "triggered_at": timezone.now(),
+            "project_slug": self.project.slug,
             "unsubscribe_link": None,
         }
-        assert expected == handler.generate_email_context(status)
+        assert expected == generate_incident_trigger_email_context(
+            self.project, incident, action.alert_rule_trigger, status
+        )
 
     def test_environment(self):
         status = TriggerStatus.ACTIVE
@@ -132,8 +140,9 @@ class EmailActionHandlerGenerateEmailContextTest(TestCase):
         alert_rule_trigger = self.create_alert_rule_trigger(alert_rule=alert_rule)
         action = self.create_alert_rule_trigger_action(alert_rule_trigger=alert_rule_trigger)
         incident = self.create_incident()
-        handler = EmailActionHandler(action, incident, self.project)
-        assert "prod" == handler.generate_email_context(status).get("environment")
+        assert "prod" == generate_incident_trigger_email_context(
+            self.project, incident, action.alert_rule_trigger, status
+        ).get("environment")
 
 
 @freeze_time()

tests/snuba/incidents/test_tasks.py

@@ -12,7 +12,10 @@ from django.test.utils import override_settings
 from exam import fixture
 from freezegun import freeze_time
 
-from sentry.incidents.action_handlers import EmailActionHandler
+from sentry.incidents.action_handlers import (
+    EmailActionHandler,
+    generate_incident_trigger_email_context,
+)
 from sentry.incidents.logic import create_alert_rule_trigger, create_alert_rule_trigger_action
 from sentry.incidents.models import (
     AlertRuleThresholdType,
@@ -126,7 +129,14 @@ class HandleSnubaQueryUpdateTest(TestCase):
         assert len(mail.outbox) == 1
         handler = EmailActionHandler(self.action, active_incident().get(), self.project)
         message = handler.build_message(
-            handler.generate_email_context(TriggerStatus.ACTIVE), TriggerStatus.ACTIVE, self.user.id
+            generate_incident_trigger_email_context(
+                handler.project,
+                handler.incident,
+                handler.action.alert_rule_trigger,
+                TriggerStatus.ACTIVE,
+            ),
+            TriggerStatus.ACTIVE,
+            self.user.id,
         )
 
         out = mail.outbox[0]