Browse Source

feat(crons): Start using next_checkin_latest (#53994)

Properly uses `next_checkin_latest`
Richard Ortenberg 1 year ago
parent
commit
1ae45d5eae
2 changed files with 69 additions and 4 deletions
  1. 1 1
      src/sentry/monitors/tasks.py
  2. 68 3
      tests/sentry/monitors/test_tasks.py

+ 1 - 1
src/sentry/monitors/tasks.py

@@ -56,7 +56,7 @@ def check_missing(current_datetime=None):
 
     qs = (
         MonitorEnvironment.objects.filter(
-            monitor__type__in=[MonitorType.CRON_JOB], next_checkin__lt=current_datetime
+            monitor__type__in=[MonitorType.CRON_JOB], next_checkin_latest__lt=current_datetime
         )
         .exclude(
             status__in=[

+ 68 - 3
tests/sentry/monitors/test_tasks.py

@@ -29,7 +29,7 @@ class CheckMonitorsTest(TestCase):
         # run very close, let's say for our test that it runs 12 seconds after
         # the minute.
         #
-        # This is testing that the task correctly clamps it's reference time
+        # This is testing that the task correctly clamps its reference time
         # down to the minute.
         task_run_ts = ts.replace(second=12, microsecond=0)
 
@@ -37,7 +37,7 @@ class CheckMonitorsTest(TestCase):
         # same here.
         next_checkin_ts = ts.replace(second=0, microsecond=0)
 
-        return (task_run_ts, next_checkin_ts)
+        return task_run_ts, next_checkin_ts
 
     def test_missing_checkin(self):
         org = self.create_organization()
@@ -56,12 +56,13 @@ class CheckMonitorsTest(TestCase):
                 "checkin_margin": None,
             },
         )
-        # Exepcted checkin was a full minute ago.
+        # Expected check-in was a full minute ago.
         monitor_environment = MonitorEnvironment.objects.create(
             monitor=monitor,
             environment=self.environment,
             last_checkin=next_checkin_ts - timedelta(minutes=2),
             next_checkin=next_checkin_ts - timedelta(minutes=1),
+            next_checkin_latest=next_checkin_ts - timedelta(minutes=1),
             status=MonitorStatus.OK,
         )
 
@@ -83,6 +84,61 @@ class CheckMonitorsTest(TestCase):
         ).replace(second=0, microsecond=0)
         assert missed_check.monitor_config == monitor.config
 
+    def test_missing_checkin_with_margin(self):
+        org = self.create_organization()
+        project = self.create_project(organization=org)
+
+        task_run_ts, next_checkin_ts = self.make_ref_time()
+
+        monitor = Monitor.objects.create(
+            organization_id=org.id,
+            project_id=project.id,
+            type=MonitorType.CRON_JOB,
+            config={
+                "schedule": [10, "minute"],
+                "schedule_type": ScheduleType.INTERVAL,
+                "max_runtime": None,
+                "checkin_margin": 5,
+            },
+        )
+        # Expected check-in was 12 minutes ago.
+        monitor_environment = MonitorEnvironment.objects.create(
+            monitor=monitor,
+            environment=self.environment,
+            last_checkin=next_checkin_ts - timedelta(minutes=12),
+            next_checkin=next_checkin_ts - timedelta(minutes=2),
+            next_checkin_latest=next_checkin_ts + timedelta(minutes=3),
+            status=MonitorStatus.OK,
+        )
+
+        # No missed check-in generated as expected time was still within margin
+        check_missing(task_run_ts)
+
+        assert not MonitorEnvironment.objects.filter(
+            id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
+        ).exists()
+        assert not MonitorCheckIn.objects.filter(
+            monitor_environment=monitor_environment.id, status=CheckInStatus.MISSED
+        )
+
+        # Missed check-in generated as clock now exceeds expected time plus margin
+        check_missing(task_run_ts + timedelta(minutes=4))
+
+        assert MonitorEnvironment.objects.filter(
+            id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
+        ).exists()
+
+        assert MonitorCheckIn.objects.filter(
+            monitor_environment=monitor_environment.id, status=CheckInStatus.MISSED
+        )
+        missed_check = MonitorCheckIn.objects.get(
+            monitor_environment=monitor_environment.id, status=CheckInStatus.MISSED
+        )
+        assert missed_check.expected_time == (
+            monitor_environment.last_checkin + timedelta(minutes=10)
+        ).replace(second=0, microsecond=0)
+        assert missed_check.monitor_config == monitor.config
+
     def assert_state_does_not_change_for_state(self, state):
         org = self.create_organization()
         project = self.create_project(organization=org)
@@ -102,6 +158,7 @@ class CheckMonitorsTest(TestCase):
             monitor=monitor,
             environment=self.environment,
             next_checkin=next_checkin_ts - timedelta(minutes=1),
+            next_checkin_latest=next_checkin_ts - timedelta(minutes=1),
             status=MonitorStatus.ACTIVE,
         )
 
@@ -151,6 +208,7 @@ class CheckMonitorsTest(TestCase):
             environment=self.environment,
             last_checkin=last_checkin_ts,
             next_checkin=next_checkin_ts,
+            next_checkin_latest=next_checkin_ts,
             status=MonitorStatus.OK,
         )
         # Last checkin was a minute ago
@@ -196,6 +254,7 @@ class CheckMonitorsTest(TestCase):
             environment=self.environment,
             last_checkin=check_in_24hr_ago - timedelta(hours=24),
             next_checkin=check_in_24hr_ago,
+            next_checkin_latest=check_in_24hr_ago,
             status=MonitorStatus.OK,
         )
         # In progress started 24hr ago
@@ -260,6 +319,7 @@ class CheckMonitorsTest(TestCase):
             # Next checkin is in the future, we just completed our last checkin
             last_checkin=next_checkin_ts,
             next_checkin=next_checkin_ts + timedelta(hours=24),
+            next_checkin_latest=next_checkin_ts + timedelta(hours=24),
             status=MonitorStatus.OK,
         )
         # Checkin 24hr ago
@@ -316,6 +376,7 @@ class CheckMonitorsTest(TestCase):
             environment=self.environment,
             last_checkin=check_in_24hr_ago,
             next_checkin=next_checkin_ts,
+            next_checkin_latest=next_checkin_ts,
             status=MonitorStatus.OK,
         )
         checkin = MonitorCheckIn.objects.create(
@@ -366,6 +427,7 @@ class CheckMonitorsTest(TestCase):
             monitor=exception_monitor,
             environment=self.environment,
             next_checkin=next_checkin_ts - timedelta(minutes=1),
+            next_checkin_latest=next_checkin_ts - timedelta(minutes=1),
             status=MonitorStatus.OK,
         )
 
@@ -379,6 +441,7 @@ class CheckMonitorsTest(TestCase):
             monitor=monitor,
             environment=self.environment,
             next_checkin=next_checkin_ts - timedelta(minutes=1),
+            next_checkin_latest=next_checkin_ts - timedelta(minutes=1),
             status=MonitorStatus.OK,
         )
 
@@ -420,6 +483,7 @@ class CheckMonitorsTest(TestCase):
             environment=self.environment,
             last_checkin=next_checkin_ts,
             next_checkin=next_checkin_ts + timedelta(hours=24),
+            next_checkin_latest=next_checkin_ts + timedelta(hours=24),
             status=MonitorStatus.OK,
         )
         MonitorCheckIn.objects.create(
@@ -445,6 +509,7 @@ class CheckMonitorsTest(TestCase):
             environment=self.environment,
             last_checkin=next_checkin_ts,
             next_checkin=next_checkin_ts + timedelta(hours=24),
+            next_checkin_latest=next_checkin_ts + timedelta(hours=24),
             status=MonitorStatus.OK,
         )
         checkin1 = MonitorCheckIn.objects.create(