Browse Source

ref(crons): Switch MonitorObjectStatus back to ObjectStatus (#61586)

This reclaims the monitor status to be a simple `ObjectStatus`

We will continue translating the following requests:

- `PUT {'status': 'muted'}` sets `is_muted` true, but does not update
status
- `PUT {'status': 'active'}` sets `is_muted` to false and updates the
status to active.

Follows after https://github.com/getsentry/sentry/pull/61583
Evan Purkhiser 1 year ago
parent
commit
9ad875c2b4

+ 2 - 3
src/sentry/monitors/consumers/monitor_consumer.py

@@ -16,7 +16,7 @@ from django.db import router, transaction
 from sentry_sdk.tracing import Span, Transaction
 
 from sentry import ratelimits
-from sentry.constants import DataCategory
+from sentry.constants import DataCategory, ObjectStatus
 from sentry.killswitches import killswitch_matches_context
 from sentry.models.project import Project
 from sentry.monitors.logic.mark_failed import mark_failed
@@ -29,7 +29,6 @@ from sentry.monitors.models import (
     MonitorEnvironmentLimitsExceeded,
     MonitorEnvironmentValidationFailed,
     MonitorLimitsExceeded,
-    MonitorObjectStatus,
     MonitorType,
 )
 from sentry.monitors.tasks import try_monitor_tasks_trigger
@@ -91,7 +90,7 @@ def _ensure_monitor_with_config(
             defaults={
                 "project_id": project.id,
                 "name": monitor_slug,
-                "status": MonitorObjectStatus.ACTIVE,
+                "status": ObjectStatus.ACTIVE,
                 "type": MonitorType.CRON_JOB,
                 "config": validated_config,
             },

+ 4 - 3
src/sentry/monitors/endpoints/base.py

@@ -15,10 +15,11 @@ from sentry.api.base import Endpoint
 from sentry.api.bases.organization import OrganizationPermission
 from sentry.api.bases.project import ProjectPermission
 from sentry.api.exceptions import ParameterValidationError, ResourceDoesNotExist
+from sentry.constants import ObjectStatus
 from sentry.models.organization import Organization
 from sentry.models.project import Project
 from sentry.models.projectkey import ProjectKey
-from sentry.monitors.models import CheckInStatus, Monitor, MonitorCheckIn, MonitorObjectStatus
+from sentry.monitors.models import CheckInStatus, Monitor, MonitorCheckIn
 from sentry.utils.sdk import bind_organization_context, configure_scope
 
 
@@ -71,7 +72,7 @@ class MonitorEndpoint(Endpoint):
             raise ResourceDoesNotExist
 
         project = Project.objects.get_from_cache(id=monitor.project_id)
-        if project.status != MonitorObjectStatus.ACTIVE:
+        if project.status != ObjectStatus.ACTIVE:
             raise ResourceDoesNotExist
 
         self.check_object_permissions(request, project)
@@ -200,7 +201,7 @@ class MonitorIngestEndpoint(Endpoint):
         else:
             project = Project.objects.get_from_cache(id=monitor.project_id)
 
-        if project.status != MonitorObjectStatus.ACTIVE:
+        if project.status != ObjectStatus.ACTIVE:
             raise ResourceDoesNotExist
 
         # Validate that the authenticated project matches the monitor. This is

+ 3 - 3
src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py

@@ -16,6 +16,7 @@ from sentry.api.base import region_silo_endpoint
 from sentry.api.serializers import serialize
 from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED
 from sentry.apidocs.parameters import GlobalParams, MonitorParams
+from sentry.constants import ObjectStatus
 from sentry.models.project import Project
 from sentry.models.projectkey import ProjectKey
 from sentry.monitors.logic.mark_failed import mark_failed
@@ -28,7 +29,6 @@ from sentry.monitors.models import (
     MonitorEnvironmentLimitsExceeded,
     MonitorEnvironmentValidationFailed,
     MonitorLimitsExceeded,
-    MonitorObjectStatus,
 )
 from sentry.monitors.serializers import MonitorCheckInSerializer
 from sentry.monitors.utils import get_timeout_at, signal_first_checkin, signal_monitor_created
@@ -100,8 +100,8 @@ class MonitorIngestCheckInIndexEndpoint(MonitorIngestEndpoint):
         Note: If a DSN is utilized for authentication, the response will be limited in details.
         """
         if monitor and monitor.status in [
-            MonitorObjectStatus.PENDING_DELETION,
-            MonitorObjectStatus.DELETION_IN_PROGRESS,
+            ObjectStatus.PENDING_DELETION,
+            ObjectStatus.DELETION_IN_PROGRESS,
         ]:
             return self.respond(status=404)
 

+ 9 - 12
src/sentry/monitors/endpoints/organization_monitor_details.py

@@ -23,6 +23,7 @@ from sentry.apidocs.constants import (
     RESPONSE_UNAUTHORIZED,
 )
 from sentry.apidocs.parameters import GlobalParams, MonitorParams
+from sentry.constants import ObjectStatus
 from sentry.models.rule import Rule, RuleActivity, RuleActivityType
 from sentry.models.scheduledeletion import RegionScheduledDeletion
 from sentry.monitors.endpoints.base import MonitorEndpoint
@@ -31,7 +32,6 @@ from sentry.monitors.models import (
     Monitor,
     MonitorCheckIn,
     MonitorEnvironment,
-    MonitorObjectStatus,
     MonitorStatus,
 )
 from sentry.monitors.serializers import MonitorSerializer
@@ -131,9 +131,6 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
             params["slug"] = result["slug"]
         if "status" in result:
             params["status"] = result["status"]
-
-            # TODO(epurkhiser): Remove dual-writing once status no longer tracks muted
-            params["is_muted"] = result["status"] == MonitorObjectStatus.MUTED
         if "is_muted" in result:
             params["is_muted"] = result["is_muted"]
         if "config" in result:
@@ -216,8 +213,8 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
                     )
                     .exclude(
                         monitor__status__in=[
-                            MonitorObjectStatus.PENDING_DELETION,
-                            MonitorObjectStatus.DELETION_IN_PROGRESS,
+                            ObjectStatus.PENDING_DELETION,
+                            ObjectStatus.DELETION_IN_PROGRESS,
                         ]
                     )
                     .exclude(
@@ -231,8 +228,8 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
             else:
                 monitor_objects = Monitor.objects.filter(id=monitor.id).exclude(
                     status__in=[
-                        MonitorObjectStatus.PENDING_DELETION,
-                        MonitorObjectStatus.DELETION_IN_PROGRESS,
+                        ObjectStatus.PENDING_DELETION,
+                        ObjectStatus.DELETION_IN_PROGRESS,
                     ]
                 )
                 event = audit_log.get_event_id("MONITOR_REMOVE")
@@ -248,14 +245,14 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
                         )
                         .exclude(
                             status__in=[
-                                MonitorObjectStatus.PENDING_DELETION,
-                                MonitorObjectStatus.DELETION_IN_PROGRESS,
+                                ObjectStatus.PENDING_DELETION,
+                                ObjectStatus.DELETION_IN_PROGRESS,
                             ]
                         )
                         .first()
                     )
                     if rule:
-                        rule.update(status=MonitorObjectStatus.PENDING_DELETION)
+                        rule.update(status=ObjectStatus.PENDING_DELETION)
                         RuleActivity.objects.create(
                             rule=rule, user_id=request.user.id, type=RuleActivityType.DELETED.value
                         )
@@ -274,7 +271,7 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
             # create copy of queryset as update will remove objects
             monitor_objects_list = list(monitor_objects)
             if not monitor_objects or not monitor_objects.update(
-                status=MonitorObjectStatus.PENDING_DELETION
+                status=ObjectStatus.PENDING_DELETION
             ):
                 return self.respond(status=404)
 

+ 3 - 3
src/sentry/monitors/endpoints/organization_monitor_index.py

@@ -19,6 +19,7 @@ from sentry.apidocs.constants import (
 )
 from sentry.apidocs.parameters import GlobalParams, OrganizationParams
 from sentry.apidocs.utils import inline_sentry_response_serializer
+from sentry.constants import ObjectStatus
 from sentry.db.models.query import in_iexact
 from sentry.models.environment import Environment
 from sentry.models.organization import Organization
@@ -26,7 +27,6 @@ from sentry.monitors.models import (
     Monitor,
     MonitorEnvironment,
     MonitorLimitsExceeded,
-    MonitorObjectStatus,
     MonitorStatus,
     MonitorType,
 )
@@ -102,8 +102,8 @@ class OrganizationMonitorIndexEndpoint(OrganizationEndpoint):
             organization_id=organization.id, project_id__in=filter_params["project_id"]
         ).exclude(
             status__in=[
-                MonitorObjectStatus.PENDING_DELETION,
-                MonitorObjectStatus.DELETION_IN_PROGRESS,
+                ObjectStatus.PENDING_DELETION,
+                ObjectStatus.DELETION_IN_PROGRESS,
             ]
         )
         query = request.GET.get("query")

+ 2 - 18
src/sentry/monitors/models.py

@@ -74,22 +74,6 @@ class MonitorEnvironmentValidationFailed(Exception):
     pass
 
 
-class MonitorObjectStatus:
-    ACTIVE = 0
-    MUTED = 1
-    PENDING_DELETION = 2
-    DELETION_IN_PROGRESS = 3
-
-    @classmethod
-    def as_choices(cls) -> Sequence[Tuple[int, str]]:
-        return (
-            (cls.ACTIVE, "active"),
-            (cls.MUTED, "muted"),
-            (cls.PENDING_DELETION, "pending_deletion"),
-            (cls.DELETION_IN_PROGRESS, "deletion_in_progress"),
-        )
-
-
 class MonitorStatus:
     """
     The monitor status is an extension of the ObjectStatus constants. In this
@@ -97,7 +81,7 @@ class MonitorStatus:
     represented.
 
     [!!]: This is NOT used for the status of the Monitor model itself. That is
-          a MonitorObjectStatus.
+          a ObjectStatus.
     """
 
     ACTIVE = 0
@@ -206,7 +190,7 @@ class Monitor(Model):
     # TODO(epurkhiser): Muted is moving to its own boolean column, this should
     # become object status again
     status = BoundedPositiveIntegerField(
-        default=MonitorObjectStatus.ACTIVE, choices=MonitorObjectStatus.as_choices()
+        default=ObjectStatus.ACTIVE, choices=ObjectStatus.as_choices()
     )
     """
     Active status of the monitor. This is similar to most other ObjectStatus's

+ 5 - 14
src/sentry/monitors/tasks.py

@@ -12,6 +12,7 @@ from arroyo.backends.kafka import KafkaPayload, KafkaProducer, build_kafka_confi
 from confluent_kafka.admin import AdminClient, PartitionMetadata
 from django.conf import settings
 
+from sentry.constants import ObjectStatus
 from sentry.monitors.logic.mark_failed import mark_failed
 from sentry.monitors.schedule import get_prev_schedule
 from sentry.monitors.types import ClockPulseMessage
@@ -25,14 +26,7 @@ from sentry.utils.kafka_config import (
     get_topic_definition,
 )
 
-from .models import (
-    CheckInStatus,
-    MonitorCheckIn,
-    MonitorEnvironment,
-    MonitorObjectStatus,
-    MonitorStatus,
-    MonitorType,
-)
+from .models import CheckInStatus, MonitorCheckIn, MonitorEnvironment, MonitorStatus, MonitorType
 
 logger = logging.getLogger("sentry")
 
@@ -244,12 +238,9 @@ def check_missing(current_datetime: datetime):
         )
         .exclude(
             monitor__status__in=[
-                # TODO(epurkhiser): This will change to DISABLED when this
-                # moves back to regual ObjectStatus. This means we will create
-                # missed check-ins for muted monitors.
-                MonitorObjectStatus.MUTED,
-                MonitorObjectStatus.PENDING_DELETION,
-                MonitorObjectStatus.DELETION_IN_PROGRESS,
+                ObjectStatus.DISABLED,
+                ObjectStatus.PENDING_DELETION,
+                ObjectStatus.DELETION_IN_PROGRESS,
             ],
         )[:MONITOR_LIMIT]
     )

+ 28 - 12
src/sentry/monitors/validators.py

@@ -11,21 +11,20 @@ from sentry.api.fields.empty_integer import EmptyIntegerField
 from sentry.api.fields.sentry_slug import SentrySlugField
 from sentry.api.serializers.rest_framework import CamelSnakeSerializer
 from sentry.api.serializers.rest_framework.project import ProjectField
+from sentry.constants import ObjectStatus
 from sentry.db.models import BoundedPositiveIntegerField
 from sentry.monitors.constants import MAX_SLUG_LENGTH, MAX_THRESHOLD, MAX_TIMEOUT
-from sentry.monitors.models import (
-    CheckInStatus,
-    Monitor,
-    MonitorObjectStatus,
-    MonitorType,
-    ScheduleType,
-)
+from sentry.monitors.models import CheckInStatus, Monitor, MonitorType, ScheduleType
 
 MONITOR_TYPES = {"cron_job": MonitorType.CRON_JOB}
 
 MONITOR_STATUSES = {
-    "active": MonitorObjectStatus.ACTIVE,
-    "muted": MonitorObjectStatus.MUTED,
+    "active": ObjectStatus.ACTIVE,
+    "disabled": ObjectStatus.DISABLED,
+    # TODO(epurkhiser): Remove once we no longer accept muted as a status. We
+    # use ACTIVE here since we do not want to actually change the status during
+    # muting. The validator will set is_muted to true when this status is set.
+    "muted": ObjectStatus.ACTIVE,
 }
 
 SCHEDULE_TYPES = {
@@ -247,7 +246,7 @@ class MonitorValidator(CamelSnakeSerializer):
     status = serializers.ChoiceField(
         choices=list(zip(MONITOR_STATUSES.keys(), MONITOR_STATUSES.keys())),
         default="active",
-        help_text="Status of the monitor. Muted monitors do not generate events or notifications.",
+        help_text="Status of the monitor. Disabled monitors will not accept events and will not count towards the monitor quota.\n\n`muted` is deprecated. Prefer the `is_muted` field.",
     )
     is_muted = serializers.BooleanField(
         required=False,
@@ -257,8 +256,25 @@ class MonitorValidator(CamelSnakeSerializer):
     config = ConfigValidator()
     alert_rule = MonitorAlertRuleValidator(required=False)
 
-    def validate_status(self, value):
-        return MONITOR_STATUSES.get(value, value)
+    def validate(self, attrs):
+        status = attrs.get("status")
+
+        # TODO(epurkhiser): This translation of muted / active status ->
+        # isMuted will be removed in the future.
+        #
+        # When the status is set to muted we will set is_muted true, the status
+        # will not change. When the status is set to 'active' we will set
+        # is_muted to false and the status will be set to active
+        if status == "active":
+            attrs["is_muted"] = False
+
+        if status == "muted":
+            del attrs["status"]
+            attrs["is_muted"] = True
+        elif status:
+            attrs["status"] = MONITOR_STATUSES.get(status)
+
+        return attrs
 
     def validate_type(self, value):
         return MONITOR_TYPES.get(value, value)

+ 3 - 3
tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_index.py

@@ -9,6 +9,7 @@ from django.test.utils import override_settings
 from django.urls import reverse
 
 from sentry.api.fields.sentry_slug import DEFAULT_SLUG_ERROR_MESSAGE
+from sentry.constants import ObjectStatus
 from sentry.db.models import BoundedPositiveIntegerField
 from sentry.monitors.constants import TIMEOUT
 from sentry.monitors.models import (
@@ -16,7 +17,6 @@ from sentry.monitors.models import (
     Monitor,
     MonitorCheckIn,
     MonitorEnvironment,
-    MonitorObjectStatus,
     MonitorStatus,
     MonitorType,
     ScheduleType,
@@ -192,7 +192,7 @@ class CreateMonitorCheckInTest(MonitorIngestTestCase):
             )
 
     def test_pending_deletion(self):
-        monitor = self._create_monitor(status=MonitorObjectStatus.PENDING_DELETION)
+        monitor = self._create_monitor(status=ObjectStatus.PENDING_DELETION)
 
         for path_func in self._get_path_functions():
             path = path_func(monitor.guid)
@@ -236,7 +236,7 @@ class CreateMonitorCheckInTest(MonitorIngestTestCase):
         )
 
     def test_deletion_in_progress(self):
-        monitor = self._create_monitor(status=MonitorObjectStatus.DELETION_IN_PROGRESS)
+        monitor = self._create_monitor(status=ObjectStatus.DELETION_IN_PROGRESS)
 
         for path_func in self._get_path_functions():
             path = path_func(monitor.guid)

+ 31 - 8
tests/sentry/monitors/endpoints/test_organization_monitor_details.py

@@ -3,6 +3,7 @@ from datetime import timedelta
 import pytest
 
 from sentry.api.fields.sentry_slug import DEFAULT_SLUG_ERROR_MESSAGE
+from sentry.constants import ObjectStatus
 from sentry.models.environment import Environment
 from sentry.models.rule import Rule, RuleActivity, RuleActivityType
 from sentry.models.scheduledeletion import RegionScheduledDeletion
@@ -13,7 +14,6 @@ from sentry.monitors.models import (
     Monitor,
     MonitorCheckIn,
     MonitorEnvironment,
-    MonitorObjectStatus,
     ScheduleType,
 )
 from sentry.monitors.utils import get_timeout_at
@@ -167,6 +167,29 @@ class UpdateMonitorTest(MonitorTestCase):
         monitor = Monitor.objects.get(id=monitor.id)
         assert not monitor.is_muted
 
+    def test_deprecated_status_mute(self):
+        monitor = self._create_monitor()
+
+        # Mute via status
+        resp = self.get_success_response(
+            self.organization.slug, monitor.slug, method="PUT", **{"status": "muted"}
+        )
+        assert resp.data["slug"] == monitor.slug
+
+        monitor = Monitor.objects.get(id=monitor.id)
+        assert monitor.is_muted
+        assert monitor.status == ObjectStatus.ACTIVE
+
+        # Unmute via status
+        resp = self.get_success_response(
+            self.organization.slug, monitor.slug, method="PUT", **{"status": "active"}
+        )
+        assert resp.data["slug"] == monitor.slug
+
+        monitor = Monitor.objects.get(id=monitor.id)
+        assert not monitor.is_muted
+        assert monitor.status == ObjectStatus.ACTIVE
+
     def test_timezone(self):
         monitor = self._create_monitor()
 
@@ -562,7 +585,7 @@ class DeleteMonitorTest(MonitorTestCase):
         )
 
         monitor = Monitor.objects.get(id=monitor.id)
-        assert monitor.status == MonitorObjectStatus.PENDING_DELETION
+        assert monitor.status == ObjectStatus.PENDING_DELETION
         # Slug should update on deletion
         assert monitor.slug != old_slug
         assert RegionScheduledDeletion.objects.filter(
@@ -586,10 +609,10 @@ class DeleteMonitorTest(MonitorTestCase):
         )
 
         monitor = Monitor.objects.get(id=monitor.id)
-        assert monitor.status == MonitorObjectStatus.ACTIVE
+        assert monitor.status == ObjectStatus.ACTIVE
 
         monitor_environment = MonitorEnvironment.objects.get(id=monitor_environment.id)
-        assert monitor_environment.status == MonitorObjectStatus.PENDING_DELETION
+        assert monitor_environment.status == ObjectStatus.PENDING_DELETION
         assert RegionScheduledDeletion.objects.filter(
             object_id=monitor_environment.id, model_name="MonitorEnvironment"
         ).exists()
@@ -608,16 +631,16 @@ class DeleteMonitorTest(MonitorTestCase):
         )
 
         monitor = Monitor.objects.get(id=monitor.id)
-        assert monitor.status == MonitorObjectStatus.ACTIVE
+        assert monitor.status == ObjectStatus.ACTIVE
 
         monitor_environment_a = MonitorEnvironment.objects.get(id=monitor_environment_a.id)
-        assert monitor_environment_a.status == MonitorObjectStatus.PENDING_DELETION
+        assert monitor_environment_a.status == ObjectStatus.PENDING_DELETION
         assert RegionScheduledDeletion.objects.filter(
             object_id=monitor_environment_a.id, model_name="MonitorEnvironment"
         ).exists()
 
         monitor_environment_b = MonitorEnvironment.objects.get(id=monitor_environment_b.id)
-        assert monitor_environment_b.status == MonitorObjectStatus.PENDING_DELETION
+        assert monitor_environment_b.status == ObjectStatus.PENDING_DELETION
         assert RegionScheduledDeletion.objects.filter(
             object_id=monitor_environment_b.id, model_name="MonitorEnvironment"
         ).exists()
@@ -642,7 +665,7 @@ class DeleteMonitorTest(MonitorTestCase):
         )
 
         rule = Rule.objects.get(project_id=monitor.project_id, id=monitor.config["alert_rule_id"])
-        assert rule.status == MonitorObjectStatus.PENDING_DELETION
+        assert rule.status == ObjectStatus.PENDING_DELETION
         assert RuleActivity.objects.filter(rule=rule, type=RuleActivityType.DELETED.value).exists()
 
     def test_simple_with_alert_rule_deleted(self):

Some files were not shown because too many files changed in this diff