feat(crons): Move over check_missing task to parallel task (#56579)

Moves over `check_missing` to run in parallel with the newly registered
task.

Author: rjo100

src/sentry/monitors/tasks.py

@@ -204,34 +204,7 @@ def check_missing(current_datetime: datetime):
     )
     metrics.gauge("sentry.monitors.tasks.check_missing.count", qs.count(), sample_rate=1.0)
     for monitor_environment in qs:
-        try:
-            logger.info(
-                "monitor.missed-checkin", extra={"monitor_environment_id": monitor_environment.id}
-            )
-
-            monitor = monitor_environment.monitor
-            expected_time = monitor_environment.next_checkin
-
-            # add missed checkin.
-            #
-            # XXX(epurkhiser): The date_added is backdated so that this missed
-            # check-in correctly reflects the time of when the checkin SHOULD
-            # have happened. It is the same as the expected_time.
-            checkin = MonitorCheckIn.objects.create(
-                project_id=monitor_environment.monitor.project_id,
-                monitor=monitor_environment.monitor,
-                monitor_environment=monitor_environment,
-                status=CheckInStatus.MISSED,
-                date_added=expected_time,
-                expected_time=expected_time,
-                monitor_config=monitor.get_validated_config(),
-            )
-            # TODO(epurkhiser): To properly fix GH-55874 we need to actually
-            # pass a timestamp here. But I'm not 100% sure what that should
-            # look like yet.
-            mark_failed(checkin, ts=None)
-        except Exception:
-            logger.exception("Exception in check_monitors - mark missed", exc_info=True)
+        mark_environment_missing.delay(monitor_environment.id)
 
 
 @instrumented_task(

tests/sentry/monitors/test_tasks.py

@@ -2,6 +2,7 @@
 from unittest import mock
 
 import msgpack
+import pytest
 from arroyo.backends.kafka import KafkaPayload
 from django.test import override_settings
 from django.utils import timezone
@@ -20,6 +21,7 @@
     check_missing,
     check_timeout,
     clock_pulse,
+    mark_environment_missing,
     try_monitor_tasks_trigger,
 )
 from sentry.testutils.cases import TestCase
@@ -48,7 +50,8 @@ def make_ref_time():
 
 
 class MonitorTaskCheckMissingTest(TestCase):
-    def test_missing_checkin(self):
+    @mock.patch("sentry.monitors.tasks.mark_environment_missing")
+    def test_missing_checkin(self, mark_environment_missing_mock):
         org = self.create_organization()
         project = self.create_project(organization=org)
 
@@ -78,6 +81,14 @@ def test_missing_checkin(self):
 
         check_missing(task_run_ts)
 
+        # assert that task is called for the specific environment
+        assert mark_environment_missing_mock.delay.call_count == 1
+        assert mark_environment_missing_mock.delay.mock_calls[0] == mock.call(
+            monitor_environment.id
+        )
+
+        mark_environment_missing(monitor_environment.id)
+
         # Monitor status is updated
         monitor_environment = MonitorEnvironment.objects.get(
             id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
@@ -98,7 +109,8 @@ def test_missing_checkin(self):
         ).replace(second=0, microsecond=0)
         assert missed_checkin.monitor_config == monitor.config
 
-    def test_missing_checkin_with_margin(self):
+    @mock.patch("sentry.monitors.tasks.mark_environment_missing")
+    def test_missing_checkin_with_margin(self, mark_environment_missing_mock):
         org = self.create_organization()
         project = self.create_project(organization=org)
 
@@ -132,6 +144,9 @@ def test_missing_checkin_with_margin(self):
         # No missed check-in generated as we're still within the check-in margin
         check_missing(task_run_ts)
 
+        # assert that task is not called for the specific environment
+        assert mark_environment_missing_mock.delay.call_count == 0
+
         assert not MonitorEnvironment.objects.filter(
             id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
         ).exists()
@@ -142,6 +157,14 @@ def test_missing_checkin_with_margin(self):
         # Missed check-in generated as clock now exceeds expected time plus margin
         check_missing(task_run_ts + timedelta(minutes=4))
 
+        # assert that task is called for the specific environment
+        assert mark_environment_missing_mock.delay.call_count == 1
+        assert mark_environment_missing_mock.delay.mock_calls[0] == mock.call(
+            monitor_environment.id
+        )
+
+        mark_environment_missing(monitor_environment.id)
+
         monitor_environment = MonitorEnvironment.objects.get(
             id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
         )
@@ -157,7 +180,7 @@ def test_missing_checkin_with_margin(self):
         )
 
         # Missed checkins are back-dated to when the checkin was expected to
-        # happpen. In this case the expected_time is equal to the date_added.
+        # happen. In this case the expected_time is equal to the date_added.
         assert missed_check.date_added == (
             monitor_environment.last_checkin + timedelta(minutes=10)
         ).replace(second=0, microsecond=0)
@@ -269,8 +292,8 @@ def test_not_missing_checkin(self):
             monitor_environment=monitor_environment.id, status=CheckInStatus.MISSED
         ).exists()
 
-    @mock.patch("sentry.monitors.tasks.logger")
-    def test_missed_exception_handling(self, logger):
+    @mock.patch("sentry.monitors.tasks.mark_environment_missing")
+    def test_missed_exception_handling(self, mark_environment_missing_mock):
         org = self.create_organization()
         project = self.create_project(organization=org)
 
@@ -287,7 +310,7 @@ def test_missed_exception_handling(self, logger):
                 "schedule": [-2, "minute"],
             },
         )
-        MonitorEnvironment.objects.create(
+        failing_monitor_environment = MonitorEnvironment.objects.create(
             monitor=exception_monitor,
             environment=self.environment,
             next_checkin=ts - timedelta(minutes=1),
@@ -301,7 +324,7 @@ def test_missed_exception_handling(self, logger):
             type=MonitorType.CRON_JOB,
             config={"schedule": "* * * * *"},
         )
-        monitor_environment = MonitorEnvironment.objects.create(
+        successful_monitor_environment = MonitorEnvironment.objects.create(
             monitor=monitor,
             environment=self.environment,
             next_checkin=ts - timedelta(minutes=1),
@@ -311,15 +334,22 @@ def test_missed_exception_handling(self, logger):
 
         check_missing(task_run_ts)
 
-        # Logged the exception
-        assert logger.exception.call_count == 1
+        # assert that task is called for the specific environments
+        assert mark_environment_missing_mock.delay.call_count == 2
+
+        # assert failing monitor raises an error
+        with pytest.raises(ValueError):
+            mark_environment_missing(failing_monitor_environment.id)
+
+        # assert regular monitor works
+        mark_environment_missing(successful_monitor_environment.id)
 
         # We still marked a monitor as missed
         assert MonitorEnvironment.objects.filter(
-            id=monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
+            id=successful_monitor_environment.id, status=MonitorStatus.MISSED_CHECKIN
         ).exists()
         assert MonitorCheckIn.objects.filter(
-            monitor_environment=monitor_environment.id, status=CheckInStatus.MISSED
+            monitor_environment=successful_monitor_environment.id, status=CheckInStatus.MISSED
         ).exists()