fix(crons): Initial removal of status from Monitor object

src/sentry/monitors/consumers/check_in.py

@@ -161,10 +161,8 @@ def _process_message(wrapper: Dict) -> None:
                 signal_first_checkin(project, monitor)
 
             if check_in.status == CheckInStatus.ERROR and monitor.status != MonitorStatus.DISABLED:
-                monitor.mark_failed(start_time)
                 monitor_environment.mark_failed(start_time)
             else:
-                monitor.mark_ok(check_in, start_time)
                 monitor_environment.mark_ok(check_in, start_time)
 
             metrics.incr(

src/sentry/monitors/endpoints/base.py

@@ -225,7 +225,7 @@ def try_checkin_lookup(monitor: Monitor, checkin_id: str):
     # which is unfinished (thus still mutable)
     if checkin_id == "latest":
         checkin = (
-            MonitorCheckIn.objects.filter(monitor=monitor)
+            MonitorCheckIn.objects.filter(monitor_environment__monitor=monitor)
             .exclude(status__in=CheckInStatus.FINISHED_VALUES)
             .order_by("-date_added")
             .first()
@@ -240,6 +240,6 @@ def try_checkin_lookup(monitor: Monitor, checkin_id: str):
         raise ParameterValidationError("Invalid check-in UUID")
 
     try:
-        return MonitorCheckIn.objects.get(monitor=monitor, guid=checkin_id)
+        return MonitorCheckIn.objects.get(monitor_environment__monitor=monitor, guid=checkin_id)
     except MonitorCheckIn.DoesNotExist:
         raise ResourceDoesNotExist

src/sentry/monitors/endpoints/monitor_ingest_checkin_details.py

@@ -102,12 +102,10 @@ def put(
             checkin.update(**params)
 
             if checkin.status == CheckInStatus.ERROR:
-                monitor_failed = monitor.mark_failed(current_datetime)
-                monitor_environment.mark_failed(current_datetime)
+                monitor_failed = monitor_environment.mark_failed(current_datetime)
                 if not monitor_failed:
                     return self.respond(serialize(checkin, request.user), status=208)
             else:
-                monitor.mark_ok(checkin, current_datetime)
                 monitor_environment.mark_ok(checkin, current_datetime)
 
         return self.respond(serialize(checkin, request.user))

src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py

@@ -186,14 +186,12 @@ def post(
             signal_first_checkin(project, monitor)
 
             if checkin.status == CheckInStatus.ERROR and monitor.status != MonitorStatus.DISABLED:
-                monitor_failed = monitor.mark_failed(last_checkin=checkin.date_added)
-                monitor_environment.mark_failed(last_checkin=checkin.date_added)
+                monitor_failed = monitor_environment.mark_failed(last_checkin=checkin.date_added)
                 if not monitor_failed:
                     if isinstance(request.auth, ProjectKey):
                         return self.respond(status=200)
                     return self.respond(serialize(checkin, request.user), status=200)
             else:
-                monitor.mark_ok(checkin, checkin.date_added)
                 monitor_environment.mark_ok(checkin, checkin.date_added)
 
         if isinstance(request.auth, ProjectKey):

src/sentry/monitors/endpoints/organization_monitor_checkin_index.py

@@ -56,8 +56,9 @@ def get(self, request: Request, organization, project, monitor) -> Response:
         if start is None or end is None:
             raise ParseError(detail="Invalid date range")
 
+        # TODO(rjo100): switch this to query monitor environment when hack is removed
         queryset = MonitorCheckIn.objects.filter(
-            monitor_id=monitor.id, date_added__gte=start, date_added__lte=end
+            monitor__id=monitor.id, date_added__gte=start, date_added__lte=end
         )
 
         environments = get_environments(request, organization)

src/sentry/monitors/endpoints/organization_monitor_details.py

@@ -99,6 +99,7 @@ def put(self, request: Request, organization, project, monitor) -> Response:
         if "slug" in result:
             params["slug"] = result["slug"]
         if "status" in result:
+            # TODO(rjo100): resets status if monitor is failed, needs fixing with environments
             if result["status"] == MonitorStatus.ACTIVE:
                 if monitor.status not in (MonitorStatus.OK, MonitorStatus.ERROR):
                     params["status"] = MonitorStatus.ACTIVE
@@ -111,6 +112,8 @@ def put(self, request: Request, organization, project, monitor) -> Response:
 
         if params:
             monitor.update(**params)
+            if params.get("status"):
+                MonitorEnvironment.objects.filter(monitor=monitor).update(status=params["status"])
             self.create_audit_entry(
                 request=request,
                 organization=organization,

src/sentry/monitors/models.py

@@ -218,43 +218,6 @@ def get_next_scheduled_checkin(self, last_checkin):
         )
         return next_checkin + timedelta(minutes=int(self.config.get("checkin_margin") or 0))
 
-    def mark_failed(self, last_checkin=None, reason=MonitorFailure.UNKNOWN):
-        if last_checkin is None:
-            next_checkin_base = timezone.now()
-            last_checkin = self.last_checkin or timezone.now()
-        else:
-            next_checkin_base = last_checkin
-
-        new_status = MonitorStatus.ERROR
-        if reason == MonitorFailure.MISSED_CHECKIN:
-            new_status = MonitorStatus.MISSED_CHECKIN
-
-        affected = (
-            type(self)
-            .objects.filter(
-                Q(last_checkin__lte=last_checkin) | Q(last_checkin__isnull=True), id=self.id
-            )
-            .update(
-                next_checkin=self.get_next_scheduled_checkin(next_checkin_base),
-                status=new_status,
-                last_checkin=last_checkin,
-            )
-        )
-        if not affected:
-            return False
-
-        return True
-
-    def mark_ok(self, checkin: MonitorCheckIn, ts: datetime):
-        params = {
-            "last_checkin": ts,
-            "next_checkin": self.get_next_scheduled_checkin(ts),
-        }
-        if checkin.status == CheckInStatus.OK and self.status != MonitorStatus.DISABLED:
-            params["status"] = MonitorStatus.OK
-
-        Monitor.objects.filter(id=self.id).exclude(last_checkin__gt=ts).update(**params)
-
 
 @region_silo_only_model
 class MonitorCheckIn(Model):
@@ -417,7 +380,7 @@ def mark_ok(self, checkin: MonitorCheckIn, ts: datetime):
             "last_checkin": ts,
             "next_checkin": self.monitor.get_next_scheduled_checkin(ts),
         }
-        if checkin.status == CheckInStatus.OK and self.status != MonitorStatus.DISABLED:
+        if checkin.status == CheckInStatus.OK and self.monitor.status != MonitorStatus.DISABLED:
             params["status"] = MonitorStatus.OK
 
         MonitorEnvironment.objects.filter(id=self.id).exclude(last_checkin__gt=ts).update(**params)

src/sentry/monitors/serializers.py

@@ -86,8 +86,6 @@ def serialize(self, obj, attrs, user):
             "name": obj.name,
             "slug": obj.slug,
             "config": config,
-            "lastCheckIn": obj.last_checkin,
-            "nextCheckIn": obj.next_checkin,
             "dateCreated": obj.date_added,
             "project": attrs["project"],
             "environments": attrs["environments"],
@@ -102,8 +100,6 @@ class MonitorSerializerResponse(TypedDict):
     type: str
     config: Any
     dateCreated: datetime
-    lastCheckIn: datetime
-    nextCheckIn: datetime
     project: ProjectSerializerResponse
     environments: MonitorEnvironmentSerializerResponse
 

src/sentry/monitors/tasks.py

@@ -70,7 +70,6 @@ def check_monitors(current_datetime=None):
             monitor_environment=monitor_environment,
             status=CheckInStatus.MISSED,
         )
-        monitor_environment.monitor.mark_failed(reason=MonitorFailure.MISSED_CHECKIN)
         monitor_environment.mark_failed(reason=MonitorFailure.MISSED_CHECKIN)
 
     qs = (
@@ -106,5 +105,4 @@ def check_monitors(current_datetime=None):
             status__in=[CheckInStatus.OK, CheckInStatus.ERROR],
         ).exists()
         if not has_newer_result:
-            monitor_environment.monitor.mark_failed(reason=MonitorFailure.DURATION)
             monitor_environment.mark_failed(reason=MonitorFailure.DURATION)

tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_attachment.py

@@ -5,8 +5,14 @@
 from django.urls import reverse
 from django.utils import timezone
 
-from sentry.models import File
-from sentry.monitors.models import CheckInStatus, Monitor, MonitorCheckIn, MonitorType
+from sentry.models import Environment, File
+from sentry.monitors.models import (
+    CheckInStatus,
+    Monitor,
+    MonitorCheckIn,
+    MonitorEnvironment,
+    MonitorType,
+)
 from sentry.testutils import MonitorIngestTestCase
 from sentry.testutils.silo import region_silo_test
 
@@ -22,16 +28,31 @@ def _create_monitor(self):
         return Monitor.objects.create(
             organization_id=self.organization.id,
             project_id=self.project.id,
-            next_checkin=timezone.now() - timedelta(minutes=1),
             type=MonitorType.CRON_JOB,
             config={"schedule": "* * * * *"},
             date_added=timezone.now() - timedelta(minutes=1),
         )
 
+    def _create_monitor_environment(self, monitor, name="production", **kwargs):
+        environment = Environment.get_or_create(project=self.project, name=name)
+
+        monitorenvironment_defaults = {
+            "status": monitor.status,
+            "next_checkin": timezone.now() - timedelta(minutes=1),
+            **kwargs,
+        }
+
+        return MonitorEnvironment.objects.create(
+            monitor=monitor, environment=environment, **monitorenvironment_defaults
+        )
+
     def test_upload(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
+
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,
@@ -60,8 +81,11 @@ def test_upload(self):
 
     def test_upload_no_file(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
+
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,
@@ -83,8 +107,11 @@ def test_upload_no_file(self):
     )
     def test_upload_file_too_big(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
+
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,
@@ -107,8 +134,11 @@ def test_upload_file_too_big(self):
 
     def test_duplicate_upload(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
+
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,
@@ -151,8 +181,11 @@ def test_duplicate_upload(self):
 
     def test_invalid_file_upload(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
+
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,

tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_details.py

@@ -104,20 +104,19 @@ def test_passing(self):
                 checkin.monitor_environment.environment.name == monitor_environment.environment.name
             )
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.next_checkin > checkin.date_added
-            assert monitor.status == MonitorStatus.OK
-            assert monitor.last_checkin > checkin.date_added
-
             monitor_environment = MonitorEnvironment.objects.get(id=monitor_environment.id)
             assert monitor_environment.next_checkin > checkin.date_added
             assert monitor_environment.status == MonitorStatus.OK
             assert monitor_environment.last_checkin > checkin.date_added
 
     def test_passing_with_slug(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
         checkin = MonitorCheckIn.objects.create(
-            monitor=monitor, project_id=self.project.id, date_added=monitor.date_added
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added,
         )
 
         path = reverse(
@@ -147,11 +146,6 @@ def test_failing(self):
             checkin = MonitorCheckIn.objects.get(id=checkin.id)
             assert checkin.status == CheckInStatus.ERROR
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.next_checkin > checkin.date_added
-            assert monitor.status == MonitorStatus.ERROR
-            assert monitor.last_checkin > checkin.date_added
-
             monitor_environment = MonitorEnvironment.objects.get(id=monitor_environment.id)
             assert monitor_environment.next_checkin > checkin.date_added
             assert monitor_environment.status == MonitorStatus.ERROR
@@ -196,11 +190,6 @@ def test_latest_returns_last_unfinished(self):
             checkin3 = MonitorCheckIn.objects.get(id=checkin3.id)
             assert checkin3.status == CheckInStatus.OK
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.next_checkin > checkin2.date_added
-            assert monitor.status == MonitorStatus.OK
-            assert monitor.last_checkin > checkin2.date_added
-
             monitor_environment = MonitorEnvironment.objects.get(id=monitor_environment.id)
             assert monitor_environment.next_checkin > checkin2.date_added
             assert monitor_environment.status == MonitorStatus.OK

tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_index.py

@@ -75,11 +75,6 @@ def test_passing(self, mock_record):
             checkin = MonitorCheckIn.objects.get(guid=resp.data["id"])
             assert checkin.status == CheckInStatus.OK
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.status == MonitorStatus.OK
-            assert monitor.last_checkin == checkin.date_added
-            assert monitor.next_checkin == monitor.get_next_scheduled_checkin(checkin.date_added)
-
             monitor_environment = MonitorEnvironment.objects.get(id=checkin.monitor_environment.id)
             assert monitor_environment.status == MonitorStatus.OK
             assert monitor_environment.last_checkin == checkin.date_added
@@ -109,11 +104,6 @@ def test_failing(self):
             checkin = MonitorCheckIn.objects.get(guid=resp.data["id"])
             assert checkin.status == CheckInStatus.ERROR
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.status == MonitorStatus.ERROR
-            assert monitor.last_checkin == checkin.date_added
-            assert monitor.next_checkin == monitor.get_next_scheduled_checkin(checkin.date_added)
-
             monitor_environment = MonitorEnvironment.objects.get(id=checkin.monitor_environment.id)
             assert monitor_environment.status == MonitorStatus.ERROR
             assert monitor_environment.last_checkin == checkin.date_added
@@ -132,11 +122,6 @@ def test_disabled(self):
             checkin = MonitorCheckIn.objects.get(guid=resp.data["id"])
             assert checkin.status == CheckInStatus.ERROR
 
-            monitor = Monitor.objects.get(id=monitor.id)
-            assert monitor.status == MonitorStatus.DISABLED
-            assert monitor.last_checkin == checkin.date_added
-            assert monitor.next_checkin == monitor.get_next_scheduled_checkin(checkin.date_added)
-
             monitor_environment = MonitorEnvironment.objects.get(id=checkin.monitor_environment.id)
             assert monitor_environment.status == MonitorStatus.DISABLED
             assert monitor_environment.last_checkin == checkin.date_added

tests/sentry/monitors/endpoints/test_organization_monitor_checkin_attachment.py

@@ -18,8 +18,10 @@ def test_download(self):
         file.putfile(ContentFile(b"some data!"))
 
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,
@@ -32,8 +34,10 @@ def test_download(self):
 
     def test_download_no_file(self):
         monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor)
         checkin = MonitorCheckIn.objects.create(
             monitor=monitor,
+            monitor_environment=monitor_environment,
             project_id=self.project.id,
             date_added=monitor.date_added,
             status=CheckInStatus.IN_PROGRESS,