feat(api): Incident should store the aggregate that was used to create it (SEN-1135)

Since alert rules can monitor various aggregates, the incident should reflect that in the graphs we
show.

We also probably need a small amount of frontend work to change some names in the incidents ui.

src/sentry/api/endpoints/organization_incident_index.py

@@ -15,6 +15,7 @@ from sentry.incidents.logic import create_incident
 from sentry.incidents.models import Incident, IncidentStatus, IncidentType
 from sentry.models.group import Group
 from sentry.models.project import Project
+from sentry.snuba.models import QueryAggregations
 
 
 class IncidentSerializer(serializers.Serializer):
@@ -22,6 +23,7 @@ class IncidentSerializer(serializers.Serializer):
     groups = ListField(child=serializers.CharField(), required=True, allow_null=False)
     title = serializers.CharField(required=True)
     query = serializers.CharField(required=False, allow_blank=True, allow_null=True)
+    aggregation = serializers.IntegerField(default=QueryAggregations.TOTAL.value)
     dateStarted = serializers.DateTimeField(required=False)
     dateDetected = serializers.DateTimeField(required=False, allow_null=True)
 
@@ -39,6 +41,15 @@ class IncidentSerializer(serializers.Serializer):
             raise serializers.ValidationError("Invalid group id(s)")
         return list(groups)
 
+    def validate_aggregation(self, aggregation):
+        try:
+            return QueryAggregations(aggregation)
+        except ValueError:
+            raise serializers.ValidationError(
+                "Invalid aggregation, valid values are %s"
+                % [item.value for item in QueryAggregations]
+            )
+
 
 class OrganizationIncidentIndexEndpoint(OrganizationEndpoint):
     permission_classes = (IncidentPermission,)
@@ -93,6 +104,7 @@ class OrganizationIncidentIndexEndpoint(OrganizationEndpoint):
                 type=IncidentType.CREATED,
                 title=result["title"],
                 query=result.get("query", ""),
+                aggregation=result["aggregation"],
                 date_started=result.get("dateStarted"),
                 date_detected=result.get("dateDetected"),
                 projects=result["projects"],

src/sentry/api/serializers/models/incident.py

@@ -48,6 +48,7 @@ class IncidentSerializer(Serializer):
             "type": obj.type,
             "title": obj.title,
             "query": obj.query,
+            "aggregation": obj.aggregation,
             "dateStarted": obj.date_started,
             "dateDetected": obj.date_detected,
             "dateAdded": obj.date_added,

src/sentry/incidents/logic.py

@@ -39,6 +39,7 @@ from sentry.snuba.subscriptions import (
     bulk_create_snuba_subscriptions,
     bulk_delete_snuba_subscriptions,
     bulk_update_snuba_subscriptions,
+    query_aggregation_to_snuba,
 )
 from sentry.utils.committers import get_event_file_committers
 from sentry.utils.snuba import bulk_raw_query, raw_query, SnubaQueryParams, SnubaTSResult, zerofill
@@ -59,6 +60,7 @@ def create_incident(
     type,
     title,
     query,
+    aggregation,
     date_started=None,
     date_detected=None,
     # TODO: Probably remove detection_uuid?
@@ -88,6 +90,7 @@ def create_incident(
             type=type.value,
             title=title,
             query=query,
+            aggregation=aggregation.value,
             date_started=date_started,
             date_detected=date_detected,
             alert_rule=alert_rule,
@@ -454,7 +457,13 @@ def get_incident_event_stats(incident, start=None, end=None, data_points=50):
 def bulk_get_incident_event_stats(incidents, query_params_list, data_points=50):
     snuba_params_list = [
         SnubaQueryParams(
-            aggregations=[("count()", "", "count")],
+            aggregations=[
+                (
+                    query_aggregation_to_snuba[QueryAggregations(incident.aggregation)][0],
+                    query_aggregation_to_snuba[QueryAggregations(incident.aggregation)][1],
+                    "count",
+                )
+            ],
             orderby="time",
             groupby=["time"],
             rollup=max(int(incident.duration.total_seconds() / data_points), 1),

src/sentry/incidents/models.py

@@ -106,6 +106,7 @@ class Incident(Model):
     detection_uuid = UUIDField(null=True, db_index=True)
     status = models.PositiveSmallIntegerField(default=IncidentStatus.OPEN.value)
     type = models.PositiveSmallIntegerField(default=IncidentType.CREATED.value)
+    aggregation = models.PositiveSmallIntegerField(default=QueryAggregations.TOTAL.value)
     title = models.TextField()
     # Query used to fetch events related to an incident
     query = models.TextField()

src/sentry/incidents/subscription_processor.py

@@ -175,8 +175,8 @@ class SubscriptionProcessor(object):
                     # TODO: Include more info in name?
                     self.alert_rule.name,
                     alert_rule=self.alert_rule,
-                    # TODO: Incidents need to keep track of which metric to display
                     query=self.subscription.query,
+                    aggregation=QueryAggregations(self.alert_rule.aggregation),
                     date_started=detected_at,
                     date_detected=detected_at,
                     projects=[self.subscription.project],

src/sentry/migrations/0017_incident_aggregation.py

@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Adding indexes to large tables. These indexes should be created concurrently,
+    #   unfortunately we can't run migrations outside of a transaction until Django
+    #   1.10. So until then these should be run manually.
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    dependencies = [("sentry", "0016_delete_alert_rule_deprecated_fields")]
+
+    """
+    Generated SQL:
+    This table has low hundreds of rows, adding a default is fine.
+    ALTER TABLE "sentry_incident" ADD COLUMN "aggregation" smallint DEFAULT 0 NOT NULL CHECK ("aggregation" >= 0);
+    ALTER TABLE "sentry_incident" ALTER COLUMN "aggregation" DROP DEFAULT;
+    """
+
+    operations = [
+        migrations.AddField(
+            model_name="incident",
+            name="aggregation",
+            field=models.PositiveSmallIntegerField(default=0),
+        )
+    ]

tests/acceptance/test_incidents.py

@@ -8,6 +8,7 @@ from sentry.testutils import AcceptanceTestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import iso_format, before_now
 from sentry.incidents.logic import create_incident
 from sentry.incidents.models import IncidentType
+from sentry.snuba.models import QueryAggregations
 
 FEATURE_NAME = "organizations:incidents"
 
@@ -32,6 +33,7 @@ class OrganizationIncidentsListTest(AcceptanceTestCase, SnubaTestCase):
             type=IncidentType.CREATED,
             title="Incident #1",
             query="",
+            aggregation=QueryAggregations.TOTAL,
             date_started=timezone.now(),
             projects=[self.project],
             groups=[self.group],

tests/sentry/api/serializers/test_incident.py

@@ -30,6 +30,7 @@ class IncidentSerializerTest(TestCase):
         assert result["type"] == incident.type
         assert result["title"] == incident.title
         assert result["query"] == incident.query
+        assert result["aggregation"] == incident.aggregation
         assert result["dateStarted"] == incident.date_started
         assert result["dateDetected"] == incident.date_detected
         assert result["dateAdded"] == incident.date_added

tests/sentry/incidents/test_logic.py

@@ -86,6 +86,7 @@ class CreateIncidentTest(TestCase):
         incident_type = IncidentType.CREATED
         title = "hello"
         query = "goodbye"
+        aggregation = QueryAggregations.UNIQUE_USERS
         date_started = timezone.now()
         other_project = self.create_project(fire_project_created=True)
         other_group = self.create_group(project=other_project)
@@ -105,6 +106,7 @@ class CreateIncidentTest(TestCase):
             type=incident_type,
             title=title,
             query=query,
+            aggregation=aggregation,
             date_started=date_started,
             projects=[self.project],
             groups=[self.group, other_group],
@@ -114,6 +116,7 @@ class CreateIncidentTest(TestCase):
         assert incident.status == incident_type.value
         assert incident.title == title
         assert incident.query == query
+        assert incident.aggregation == aggregation.value
         assert incident.date_started == date_started
         assert incident.date_detected == date_started
         assert incident.alert_rule == alert_rule
@@ -196,6 +199,7 @@ class UpdateIncidentStatus(TestCase):
             IncidentType.CREATED,
             "Test",
             "",
+            QueryAggregations.TOTAL,
             timezone.now(),
             projects=[self.project],
         )
@@ -212,6 +216,7 @@ class UpdateIncidentStatus(TestCase):
             IncidentType.CREATED,
             "Test",
             "",
+            QueryAggregations.TOTAL,
             timezone.now(),
             projects=[self.project],
         )
@@ -735,6 +740,7 @@ class BulkGetIncidentStatusTest(TestCase, BaseIncidentsTest):
             IncidentType.CREATED,
             "Closed",
             "",
+            QueryAggregations.TOTAL,
             groups=[self.group],
             date_started=timezone.now() - timedelta(days=30),
         )
@@ -744,6 +750,7 @@ class BulkGetIncidentStatusTest(TestCase, BaseIncidentsTest):
             IncidentType.CREATED,
             "Open",
             "",
+            QueryAggregations.TOTAL,
             groups=[self.group],
             date_started=timezone.now() - timedelta(days=30),
         )