Browse Source

refs(metric_alerts): Use `date_started` and `date_detected` correctly. (#19803)

Previously we've treated `date_started` and `date_detected` as the same value, but we should
differentiate between these. `date_started` is the start of the bucket that created an alert.
`date_detected` is the end of that bucket, and is the date that the trigger conditions were met for
the alert. We also have `date_added`, which is the time we actually created it.

This corrects the subscription processor to pass these dates correctly. We also rename the `STARTED`
activity to `DETECTED`, and the old `DETECTED` activity to `CREATED`. This should have no impact on
the FE since we don't return those specific names, but more accurately represents the purpose of
those activities.

Relies on an upcoming frontend change to use `date_started` to render the started dot on the graph.
Dan Fuller 4 years ago
parent
commit
1c5cb45fb3

+ 2 - 2
src/sentry/incidents/logic.py

@@ -101,9 +101,9 @@ def create_incident(
                 )
 
         create_incident_activity(
-            incident, IncidentActivityType.STARTED, user=user, date_added=date_started
+            incident, IncidentActivityType.DETECTED, user=user, date_added=date_detected
         )
-        create_incident_activity(incident, IncidentActivityType.DETECTED, user=user)
+        create_incident_activity(incident, IncidentActivityType.CREATED, user=user)
         analytics.record(
             "incident.created",
             incident_id=incident.id,

+ 2 - 2
src/sentry/incidents/models.py

@@ -246,10 +246,10 @@ class TimeSeriesSnapshot(Model):
 
 
 class IncidentActivityType(Enum):
-    DETECTED = 1
+    CREATED = 1
     STATUS_CHANGE = 2
     COMMENT = 3
-    STARTED = 4
+    DETECTED = 4
 
 
 class IncidentActivity(Model):

+ 1 - 4
src/sentry/incidents/subscription_processor.py

@@ -248,10 +248,7 @@ class SubscriptionProcessor(object):
                     self.alert_rule.name,
                     alert_rule=self.alert_rule,
                     date_started=detected_at,
-                    # TODO: This should probably be either the current time or the
-                    # message time. Current time likely makes most sense, since this is
-                    # when we actually noticed the problem.
-                    date_detected=detected_at,
+                    date_detected=self.last_update,
                     projects=[self.subscription.project],
                 )
             # Now create (or update if it already exists) the incident trigger so that

+ 1 - 1
tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py

@@ -49,7 +49,7 @@ class OrganizationIncidentActivityIndexTest(APITestCase):
         activities = [
             create_incident_activity(
                 incident=incident,
-                activity_type=IncidentActivityType.DETECTED,
+                activity_type=IncidentActivityType.CREATED,
                 user=self.user,
                 comment="hello",
             ),

+ 1 - 1
tests/sentry/incidents/endpoints/test_organization_incident_comment_details.py

@@ -16,7 +16,7 @@ class BaseIncidentCommentDetailsTest(object):
         self.login_as(self.user)
         self.activity = self.create_incident_comment(self.incident, user=self.user)
         self.detected_activity = self.create_incident_activity(
-            self.incident, user=self.user, type=IncidentActivityType.DETECTED.value
+            self.incident, user=self.user, type=IncidentActivityType.CREATED.value
         )
 
         user2 = self.create_user()

+ 11 - 7
tests/sentry/incidents/test_logic.py

@@ -87,6 +87,7 @@ class CreateIncidentTest(TestCase):
         incident_type = IncidentType.ALERT_TRIGGERED
         title = "hello"
         date_started = timezone.now() - timedelta(minutes=5)
+        date_detected = timezone.now() - timedelta(minutes=4)
         alert_rule = self.create_alert_rule()
 
         self.record_event.reset_mock()
@@ -95,6 +96,7 @@ class CreateIncidentTest(TestCase):
             type_=incident_type,
             title=title,
             date_started=date_started,
+            date_detected=date_detected,
             projects=[self.project],
             alert_rule=alert_rule,
         )
@@ -103,20 +105,22 @@ class CreateIncidentTest(TestCase):
         assert incident.type == incident_type.value
         assert incident.title == title
         assert incident.date_started == date_started
-        assert incident.date_detected == date_started
+        assert incident.date_detected == date_detected
         assert incident.alert_rule == alert_rule
         assert IncidentProject.objects.filter(
             incident=incident, project__in=[self.project]
         ).exists()
         assert (
             IncidentActivity.objects.filter(
-                incident=incident, type=IncidentActivityType.STARTED.value, date_added=date_started
+                incident=incident,
+                type=IncidentActivityType.DETECTED.value,
+                date_added=date_detected,
             ).count()
             == 1
         )
         assert (
             IncidentActivity.objects.filter(
-                incident=incident, type=IncidentActivityType.DETECTED.value
+                incident=incident, type=IncidentActivityType.CREATED.value
             ).count()
             == 1
         )
@@ -1283,7 +1287,7 @@ class CreateAlertRuleTriggerActionTest(BaseAlertRuleTriggerActionTest, TestCase)
 
     @patch("sentry.integrations.msteams.utils.get_channel_id", return_value="some_id")
     def test_msteams(self, mock_get_channel_id):
-        integration = Integration.objects.create(external_id="1", provider="msteams",)
+        integration = Integration.objects.create(external_id="1", provider="msteams")
         integration.add_organization(self.organization, self.user)
         type = AlertRuleTriggerAction.Type.MSTEAMS
         target_type = AlertRuleTriggerAction.TargetType.SPECIFIC
@@ -1306,7 +1310,7 @@ class CreateAlertRuleTriggerActionTest(BaseAlertRuleTriggerActionTest, TestCase)
 
     @patch("sentry.integrations.msteams.utils.get_channel_id", return_value=None)
     def test_msteams_not_existing(self, mock_get_channel_id):
-        integration = Integration.objects.create(external_id="1", provider="msteams",)
+        integration = Integration.objects.create(external_id="1", provider="msteams")
         integration.add_organization(self.organization, self.user)
         type = AlertRuleTriggerAction.Type.MSTEAMS
         target_type = AlertRuleTriggerAction.TargetType.SPECIFIC
@@ -1396,7 +1400,7 @@ class UpdateAlertRuleTriggerAction(BaseAlertRuleTriggerActionTest, TestCase):
 
     @patch("sentry.integrations.msteams.utils.get_channel_id", return_value="some_id")
     def test_msteams(self, mock_get_channel_id):
-        integration = Integration.objects.create(external_id="1", provider="msteams",)
+        integration = Integration.objects.create(external_id="1", provider="msteams")
         integration.add_organization(self.organization, self.user)
         type = AlertRuleTriggerAction.Type.MSTEAMS
         target_type = AlertRuleTriggerAction.TargetType.SPECIFIC
@@ -1419,7 +1423,7 @@ class UpdateAlertRuleTriggerAction(BaseAlertRuleTriggerActionTest, TestCase):
 
     @patch("sentry.integrations.msteams.utils.get_channel_id", return_value=None)
     def test_msteams_not_existing(self, mock_get_channel_id):
-        integration = Integration.objects.create(external_id="1", provider="msteams",)
+        integration = Integration.objects.create(external_id="1", provider="msteams")
         integration.add_organization(self.organization, self.user)
         type = AlertRuleTriggerAction.Type.MSTEAMS
         target_type = AlertRuleTriggerAction.TargetType.SPECIFIC

+ 1 - 1
tests/sentry/incidents/test_tasks.py

@@ -69,7 +69,7 @@ class TestSendSubscriberNotifications(BaseIncidentActivityTest, TestCase):
         ).exists()
 
     def test_invalid_types(self):
-        activity_type = IncidentActivityType.DETECTED
+        activity_type = IncidentActivityType.CREATED
         activity = create_incident_activity(self.incident, activity_type)
         send_subscriber_notifications(activity.id)
         self.send_async.assert_not_called()  # NOQA