From 6110606cd0a3fa1b731c71f77bdd019b418d591d Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Thu, 13 Nov 2025 16:55:57 -0500 Subject: [PATCH] ref(crons): Remove Monitor.is_muted field, make it computed (stage 3) [NEW-564: There needs to be some way to mute the entire cron detector](https://linear.app/getsentry/issue/NEW-564/there-needs-to-be-some-way-to-mute-the-entire-cron-detector) Stage 3 of the migration completes the transition to environment-based muting: - Removes the `Monitor.is_muted` database field - Converts `Monitor.is_muted` to a computed property that returns True only when ALL environments are muted - Removes dual-write synchronization logic from endpoints and tasks - Updates muting logic to propagate from monitor to environments - Updates sorting and filtering queries to use environment muting state - Updates all tests to use environment-based muting Migration 0012 removes the is_muted field from the Monitor model. The monitor is now considered muted if and only if all of its environments are muted. If a monitor has no environments, it is considered unmuted. --- migrations_lockfile.txt | 2 +- .../base_monitor_environment_details.py | 2 - .../endpoints/organization_monitor_index.py | 37 ++++- src/sentry/monitors/logic/incidents.py | 4 +- .../0012_remove_monitor_is_muted_field.py | 33 ++++ src/sentry/monitors/models.py | 27 ++-- .../tasks/detect_broken_monitor_envs.py | 3 - src/sentry/monitors/validators.py | 12 +- .../monitors/clock_tasks/test_check_missed.py | 3 +- .../consumers/test_monitor_consumer.py | 13 +- .../endpoints/test_base_monitor_details.py | 7 +- .../test_base_monitor_environment_details.py | 9 +- .../test_organization_detector_index.py | 4 +- .../test_organization_monitor_index.py | 81 +++++++--- .../sentry/monitors/logic/test_mark_failed.py | 42 +---- tests/sentry/monitors/logic/test_mark_ok.py | 1 - tests/sentry/monitors/test_models.py | 147 +++++------------- tests/sentry/monitors/test_validators.py | 38 +++-- 18 files changed, 234 insertions(+), 231 deletions(-) create mode 100644 src/sentry/monitors/migrations/0012_remove_monitor_is_muted_field.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 19a04753b536be..2ffdb88aa97dad 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -17,7 +17,7 @@ hybridcloud: 0024_add_project_distribution_scope insights: 0002_backfill_team_starred -monitors: 0011_backfill_monitor_environment_is_muted +monitors: 0012_remove_monitor_is_muted_field nodestore: 0001_squashed_0002_nodestore_no_dictfield diff --git a/src/sentry/monitors/endpoints/base_monitor_environment_details.py b/src/sentry/monitors/endpoints/base_monitor_environment_details.py index 330be3b288c71b..bbdb5364a5a76e 100644 --- a/src/sentry/monitors/endpoints/base_monitor_environment_details.py +++ b/src/sentry/monitors/endpoints/base_monitor_environment_details.py @@ -22,8 +22,6 @@ def update_monitor_environment( is_muted = request.data.get("isMuted") if type(is_muted) is bool: monitor_environment.update(is_muted=is_muted) - # Dual-write: Sync is_muted back to monitor if all environments are muted - monitor.ensure_is_muted() self.create_audit_entry( request=request, diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index c60eb767d151b0..271e38a6669672 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_index.py @@ -143,11 +143,17 @@ def get(self, request: AuthenticatedHttpRequest, organization: Organization) -> sort_fields = [] if sort == "status": + # Check if all environments are muted by seeing if any unmuted environments exist + has_unmuted_env = monitor_environments_query.filter(is_muted=False) + queryset = queryset.annotate( environment_status_ordering=Case( - # Sort DISABLED and is_muted monitors to the bottom of the list + # Sort DISABLED and fully muted monitors to the bottom of the list When(status=ObjectStatus.DISABLED, then=Value(len(DEFAULT_STATUS_ORDER) + 1)), - When(is_muted=True, then=Value(len(DEFAULT_STATUS_ORDER))), + When( + Exists(monitor_environments_query) & ~Exists(has_unmuted_env), + then=Value(len(DEFAULT_STATUS_ORDER)), + ), default=Subquery( monitor_environments_query.annotate( status_ordering=MONITOR_ENVIRONMENT_ORDERING @@ -169,10 +175,21 @@ def get(self, request: AuthenticatedHttpRequest, organization: Organization) -> elif sort == "name": sort_fields = ["name"] elif sort == "muted": + # Check if any environments are muted + has_muted_env = monitor_environments_query.filter(is_muted=True) + # Check if all environments are muted + has_unmuted_env = monitor_environments_query.filter(is_muted=False) + queryset = queryset.annotate( muted_ordering=Case( - When(is_muted=True, then=Value(2)), - When(Exists(monitor_environments_query.filter(is_muted=True)), then=Value(1)), + # No environments muted (or no environments at all) + When(~Exists(has_muted_env), then=Value(0)), + # Some environments muted (not all) + When(Exists(has_muted_env) & Exists(has_unmuted_env), then=Value(1)), + # All environments muted (and at least one environment exists) + When( + Exists(monitor_environments_query) & ~Exists(has_unmuted_env), then=Value(2) + ), default=0, ), ) @@ -315,6 +332,9 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: if not assign_result.assignable: return self.respond(assign_result.reason, status=400) + # Extract is_muted to propagate to environments, don't update Monitor directly + is_muted = result.pop("is_muted", None) + updated = [] errored = [] for monitor in monitors: @@ -330,7 +350,14 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: if status == ObjectStatus.DISABLED: quotas.backend.disable_seat(DataCategory.MONITOR_SEAT, monitor) - monitor.update(**result) + # Propagate is_muted to all monitor environments + if is_muted is not None: + MonitorEnvironment.objects.filter(monitor_id=monitor.id).update( + is_muted=is_muted + ) + + if result: + monitor.update(**result) updated.append(monitor) self.create_audit_entry( request=request, diff --git a/src/sentry/monitors/logic/incidents.py b/src/sentry/monitors/logic/incidents.py index 7f1aaaf04383d9..58787072586e24 100644 --- a/src/sentry/monitors/logic/incidents.py +++ b/src/sentry/monitors/logic/incidents.py @@ -113,8 +113,8 @@ def try_incident_threshold( # Only create an occurrence if: # - We have an active incident and fingerprint - # - The monitor and env are not muted - if not monitor_env.monitor.is_muted and not monitor_env.is_muted and incident: + # - The environment is not muted + if not monitor_env.is_muted and incident: checkins = list(MonitorCheckIn.objects.filter(id__in=[c.id for c in previous_checkins])) for checkin in checkins: dispatch_incident_occurrence(checkin, checkins, incident, received, clock_tick) diff --git a/src/sentry/monitors/migrations/0012_remove_monitor_is_muted_field.py b/src/sentry/monitors/migrations/0012_remove_monitor_is_muted_field.py new file mode 100644 index 00000000000000..768f228f292479 --- /dev/null +++ b/src/sentry/monitors/migrations/0012_remove_monitor_is_muted_field.py @@ -0,0 +1,33 @@ +# Generated by Django 5.2.1 on 2025-11-13 19:47 + +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.fields import SafeRemoveField +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("monitors", "0011_backfill_monitor_environment_is_muted"), + ] + + operations = [ + SafeRemoveField( + model_name="monitor", + name="is_muted", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/src/sentry/monitors/models.py b/src/sentry/monitors/models.py index 5465cdc9944cb0..5212ee901c7a6f 100644 --- a/src/sentry/monitors/models.py +++ b/src/sentry/monitors/models.py @@ -238,12 +238,6 @@ class Monitor(Model): check-in payloads. The slug can be changed. """ - is_muted = models.BooleanField(default=False, db_default=False) - """ - Monitor is operating normally but will not produce incidents or produce - occurrences into the issues platform. - """ - name = models.CharField(max_length=128) """ Human readable name of the monitor. Used for display purposes. @@ -439,23 +433,22 @@ def normalize_before_relocation_import( self.guid = uuid4() return old_pk - def ensure_is_muted(self) -> None: + @property + def is_muted(self) -> bool: """ - Dual-write: Sync is_muted from monitor environments back to the monitor. - - Sets Monitor.is_muted based on whether all MonitorEnvironments are muted. - If there are no environments, is_muted remains unchanged. + A monitor is considered muted if ALL of its environments are muted. + If a monitor has no environments, it is considered unmuted. """ - # Count total environments and muted environments env_counts = MonitorEnvironment.objects.filter(monitor_id=self.id).aggregate( total=models.Count("id"), muted=models.Count("id", filter=Q(is_muted=True)) ) - # Only update if there are environments - if env_counts["total"] > 0: - all_muted = env_counts["total"] == env_counts["muted"] - if self.is_muted != all_muted: - self.update(is_muted=all_muted) + # If no environments exist, monitor is not muted + if env_counts["total"] == 0: + return False + + # Monitor is muted only if ALL environments are muted + return env_counts["total"] == env_counts["muted"] def check_organization_monitor_limit(organization_id: int) -> None: diff --git a/src/sentry/monitors/tasks/detect_broken_monitor_envs.py b/src/sentry/monitors/tasks/detect_broken_monitor_envs.py index dacbfe558ff731..e7b5fa98723af7 100644 --- a/src/sentry/monitors/tasks/detect_broken_monitor_envs.py +++ b/src/sentry/monitors/tasks/detect_broken_monitor_envs.py @@ -151,7 +151,6 @@ def build_open_incidents_queryset(): resolving_checkin=None, starting_timestamp__lte=(current_time - timedelta(days=NUM_DAYS_BROKEN_PERIOD)), monitor__status=ObjectStatus.ACTIVE, - monitor__is_muted=False, monitor_environment__is_muted=False, monitorenvbrokendetection__env_muted_timestamp=None, ) @@ -243,8 +242,6 @@ def detect_broken_monitor_envs_for_org(org_id: int): with transaction.atomic(router.db_for_write(MonitorEnvBrokenDetection)): open_incident.monitor_environment.update(is_muted=True) detection.update(env_muted_timestamp=django_timezone.now()) - # Dual-write: Sync is_muted back to monitor if all environments are muted - open_incident.monitor.ensure_is_muted() for email in get_user_emails_from_monitor(open_incident.monitor, project): if not email: diff --git a/src/sentry/monitors/validators.py b/src/sentry/monitors/validators.py index 79482df1bdd526..19a7840036ad39 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -370,7 +370,6 @@ def create(self, validated_data): name=validated_data["name"], slug=validated_data.get("slug"), status=validated_data["status"], - is_muted=validated_data.get("is_muted", False), config=validated_data["config"], ) @@ -459,6 +458,11 @@ def update(self, instance, validated_data): ): quotas.backend.disable_seat(DataCategory.MONITOR_SEAT, instance) + # Forward propagate is_muted to all monitor environments when changed + is_muted = params.pop("is_muted", None) + if is_muted is not None: + MonitorEnvironment.objects.filter(monitor_id=instance.id).update(is_muted=is_muted) + if params: instance.update(**params) create_audit_entry( @@ -469,12 +473,6 @@ def update(self, instance, validated_data): data=instance.get_audit_log_data(), ) - # Dual-write is_muted to all monitor environments when changed - if "is_muted" in params: - MonitorEnvironment.objects.filter(monitor_id=instance.id).update( - is_muted=params["is_muted"] - ) - # Update monitor slug in billing if "slug" in params: quotas.backend.update_monitor_slug(existing_slug, params["slug"], instance.project_id) diff --git a/tests/sentry/monitors/clock_tasks/test_check_missed.py b/tests/sentry/monitors/clock_tasks/test_check_missed.py index 9565a5e3c3a8dc..699f7ad3984bc6 100644 --- a/tests/sentry/monitors/clock_tasks/test_check_missed.py +++ b/tests/sentry/monitors/clock_tasks/test_check_missed.py @@ -459,7 +459,6 @@ def assert_state_does_not_change_for_status( "max_runtime": None, }, status=state, - is_muted=is_muted, ) # Expected checkin was a full minute ago, if this monitor wasn't in the # `state` the monitor would usually end up marked as timed out @@ -469,7 +468,7 @@ def assert_state_does_not_change_for_status( next_checkin=ts - timedelta(minutes=1), next_checkin_latest=ts, status=MonitorStatus.ACTIVE, - is_muted=environment_is_muted, + is_muted=is_muted or environment_is_muted, ) dispatch_check_missing(ts) diff --git a/tests/sentry/monitors/consumers/test_monitor_consumer.py b/tests/sentry/monitors/consumers/test_monitor_consumer.py index 6cc990733d3275..83b080df3ed7f3 100644 --- a/tests/sentry/monitors/consumers/test_monitor_consumer.py +++ b/tests/sentry/monitors/consumers/test_monitor_consumer.py @@ -293,7 +293,14 @@ def test_failing(self) -> None: ) def test_muted(self) -> None: - monitor = self._create_monitor(is_muted=True) + monitor = self._create_monitor() + # Create a muted environment for this monitor + production_env = self.create_environment(name="production") + MonitorEnvironment.objects.create( + monitor=monitor, + environment_id=production_env.id, + is_muted=True, + ) self.send_checkin(monitor.slug, status="error") checkin = MonitorCheckIn.objects.get(guid=self.guid) @@ -301,8 +308,8 @@ def test_muted(self) -> None: monitor_environment = MonitorEnvironment.objects.get(id=checkin.monitor_environment.id) - # The created monitor environment is in line with the check-in, but the - # parent monitor is muted + # The monitor environment should still be muted and track the error status + assert monitor_environment.is_muted is True assert monitor_environment.status == MonitorStatus.ERROR assert monitor_environment.last_checkin == checkin.date_added assert monitor_environment.next_checkin == monitor.get_next_expected_checkin( diff --git a/tests/sentry/monitors/endpoints/test_base_monitor_details.py b/tests/sentry/monitors/endpoints/test_base_monitor_details.py index 91c573eb422a20..96d466a010494f 100644 --- a/tests/sentry/monitors/endpoints/test_base_monitor_details.py +++ b/tests/sentry/monitors/endpoints/test_base_monitor_details.py @@ -345,6 +345,9 @@ def test_slug_same(self) -> None: def test_can_mute(self) -> None: monitor = self._create_monitor() + # Create an environment so the monitor has an environment to mute + self._create_monitor_environment(monitor) + resp = self.get_success_response( self.organization.slug, monitor.slug, method="PUT", **{"isMuted": True} ) @@ -355,8 +358,8 @@ def test_can_mute(self) -> None: def test_can_unmute(self) -> None: monitor = self._create_monitor() - - monitor.update(is_muted=True) + # Create a muted environment so the monitor is muted + self._create_monitor_environment(monitor, is_muted=True) resp = self.get_success_response( self.organization.slug, monitor.slug, method="PUT", **{"isMuted": False} diff --git a/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py b/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py index 6628e02cbb0857..5dac3dc336a5c5 100644 --- a/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py +++ b/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py @@ -102,15 +102,12 @@ def test_muting_all_environments_mutes_monitor(self) -> None: def test_unmuting_one_environment_unmutes_monitor(self) -> None: """Test that unmuting one environment when all were muted also unmutes the monitor.""" - # Start with a muted monitor + # Start with a monitor that has all environments muted monitor = self._create_monitor(status=MonitorStatus.ACTIVE) - monitor.update(is_muted=True) # Create two muted environments - env1 = self._create_monitor_environment(monitor, name="production") - env1.update(is_muted=True) - env2 = self._create_monitor_environment(monitor, name="staging") - env2.update(is_muted=True) + env1 = self._create_monitor_environment(monitor, name="production", is_muted=True) + env2 = self._create_monitor_environment(monitor, name="staging", is_muted=True) # Verify initial state monitor.refresh_from_db() diff --git a/tests/sentry/monitors/endpoints/test_organization_detector_index.py b/tests/sentry/monitors/endpoints/test_organization_detector_index.py index 3a782fe0750eec..4f30e3727f3ee1 100644 --- a/tests/sentry/monitors/endpoints/test_organization_detector_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_detector_index.py @@ -181,7 +181,7 @@ def test_create_monitor_with_optional_fields(self): "name": "Full Config Monitor", "slug": "full-config-monitor", "status": "disabled", - "isMuted": True, + "isMuted": False, "config": { "schedule": "*/30 * * * *", "scheduleType": "crontab", @@ -205,7 +205,7 @@ def test_create_monitor_with_optional_fields(self): ) assert monitor.name == "Full Config Monitor" assert monitor.status == ObjectStatus.DISABLED - assert monitor.is_muted is True + assert monitor.is_muted is False assert monitor.config["schedule"] == "*/30 * * * *" assert monitor.config["checkin_margin"] == 15 assert monitor.config["max_runtime"] == 120 diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index a7b9c73af9f73f..f79691377bb415 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -87,7 +87,8 @@ def add_status_monitor( monitor_error = add_status_monitor("ERROR") monitor_muted = add_status_monitor("ACTIVE") - monitor_muted.update(is_muted=True) + # Mute all environments for this monitor + monitor_muted.monitorenvironment_set.update(is_muted=True) response = self.get_success_response( self.organization.slug, params={"environment": "jungle"} @@ -135,9 +136,16 @@ def test_sort_name(self) -> None: self.check_valid_response(response, monitors) def test_sort_muted(self) -> None: + # Create monitors and mute them via environments + monitor_z = self._create_monitor(name="Z monitor") + self._create_monitor_environment(monitor_z, name="prod", is_muted=True) + + monitor_y = self._create_monitor(name="Y monitor") + self._create_monitor_environment(monitor_y, name="prod", is_muted=True) + monitors = [ - self._create_monitor(name="Z monitor", is_muted=True), - self._create_monitor(name="Y monitor", is_muted=True), + monitor_z, + monitor_y, self._create_monitor(name="Some Monitor"), self._create_monitor(name="A monitor"), self._create_monitor(name="ZA monitor"), @@ -152,26 +160,35 @@ def test_sort_muted(self) -> None: self.check_valid_response(response, monitors) def test_sort_muted_envs(self) -> None: - muted_monitor_1 = self._create_monitor(name="Z monitor", is_muted=True) - self._create_monitor_environment(muted_monitor_1, name="prod") - muted_monitor_2 = self._create_monitor(name="Y monitor", is_muted=True) - self._create_monitor_environment(muted_monitor_2, name="prod") + # Monitors with all environments muted + muted_monitor_1 = self._create_monitor(name="Z monitor") + self._create_monitor_environment(muted_monitor_1, name="prod", is_muted=True) + muted_monitor_2 = self._create_monitor(name="Y monitor") + self._create_monitor_environment(muted_monitor_2, name="prod", is_muted=True) + + # Monitors with no environments muted non_muted_monitor_1 = self._create_monitor(name="Some Monitor") self._create_monitor_environment(non_muted_monitor_1, name="prod") non_muted_monitor_2 = self._create_monitor(name="A monitor") self._create_monitor_environment(non_muted_monitor_2, name="prod") + + # Monitor with all environments muted muted_env_monitor = self._create_monitor(name="Some Muted Env Monitor") self._create_monitor_environment( muted_env_monitor, name="prod", is_muted=True, ) + + # Monitor with no environments muted not_muted_env_monitor = self._create_monitor(name="ZA monitor") self._create_monitor_environment( not_muted_env_monitor, name="prod", is_muted=False, ) + + # Monitor with some environments muted (prod unmuted, dev muted) muted_other_env_monitor = self._create_monitor(name="Some muted other Env Monitor") self._create_monitor_environment(muted_other_env_monitor, name="prod") self._create_monitor_environment( @@ -180,15 +197,19 @@ def test_sort_muted_envs(self) -> None: is_muted=True, ) + # Test sorting: no muted → some muted → all muted (alphabetically within each group) response = self.get_success_response(self.organization.slug, sort="muted") expected = [ - non_muted_monitor_2, - non_muted_monitor_1, - not_muted_env_monitor, - muted_env_monitor, - muted_other_env_monitor, - muted_monitor_2, - muted_monitor_1, + # No environments muted (value 0) - alphabetically + non_muted_monitor_2, # A monitor + non_muted_monitor_1, # Some Monitor + not_muted_env_monitor, # ZA monitor + # Some environments muted (value 1) - alphabetically + muted_other_env_monitor, # Some muted other Env Monitor + # All environments muted (value 2) - alphabetically + muted_env_monitor, # Some Muted Env Monitor + muted_monitor_2, # Y monitor + muted_monitor_1, # Z monitor ] self.check_valid_response(response, expected) @@ -196,17 +217,22 @@ def test_sort_muted_envs(self) -> None: response = self.get_success_response(self.organization.slug, sort="muted", asc="0") self.check_valid_response(response, expected) + # Test with environment filter: when filtered to "prod" only, + # muted_other_env_monitor has prod unmuted, so it's in the "no muted" group response = self.get_success_response( self.organization.slug, sort="muted", environment=["prod"] ) expected = [ - non_muted_monitor_2, - non_muted_monitor_1, - muted_other_env_monitor, - not_muted_env_monitor, - muted_env_monitor, - muted_monitor_2, - muted_monitor_1, + # No environments muted in "prod" (value 0) - alphabetically + non_muted_monitor_2, # A monitor + non_muted_monitor_1, # Some Monitor + muted_other_env_monitor, # Some muted other Env Monitor (prod is unmuted!) + not_muted_env_monitor, # ZA monitor + # All environments muted in "prod" (value 2) - alphabetically + # (value 1 "some muted" is empty since we only have one env in the filter) + muted_env_monitor, # Some Muted Env Monitor + muted_monitor_2, # Y monitor + muted_monitor_1, # Z monitor ] self.check_valid_response(response, expected) @@ -613,7 +639,9 @@ def test_valid_ids(self) -> None: def test_bulk_mute_unmute(self) -> None: monitor_one = self._create_monitor(slug="monitor_one") + env_one = self._create_monitor_environment(monitor_one) monitor_two = self._create_monitor(slug="monitor_two") + env_two = self._create_monitor_environment(monitor_two) data = { "ids": [monitor_one.guid, monitor_two.guid], @@ -625,8 +653,12 @@ def test_bulk_mute_unmute(self) -> None: monitor_one.refresh_from_db() monitor_two.refresh_from_db() + env_one.refresh_from_db() + env_two.refresh_from_db() assert monitor_one.is_muted assert monitor_two.is_muted + assert env_one.is_muted + assert env_two.is_muted assert_org_audit_log_exists( organization=self.organization, event=audit_log.get_event_id("MONITOR_EDIT"), @@ -647,8 +679,12 @@ def test_bulk_mute_unmute(self) -> None: monitor_one.refresh_from_db() monitor_two.refresh_from_db() + env_one.refresh_from_db() + env_two.refresh_from_db() assert not monitor_one.is_muted assert not monitor_two.is_muted + assert not env_one.is_muted + assert not env_two.is_muted def test_bulk_disable_enable(self) -> None: monitor_one = self._create_monitor(slug="monitor_one") @@ -704,6 +740,7 @@ def test_enable_no_quota(self, check_assign_seats: MagicMock) -> None: def test_disallow_when_no_open_membership(self) -> None: monitor = self._create_monitor() + env = self._create_monitor_environment(monitor) # disable Open Membership self.organization.flags.allow_joinleave = False @@ -725,4 +762,6 @@ def test_disallow_when_no_open_membership(self) -> None: assert response.data == {"updated": [], "errored": []} monitor.refresh_from_db() + env.refresh_from_db() assert not monitor.is_muted + assert not env.is_muted diff --git a/tests/sentry/monitors/logic/test_mark_failed.py b/tests/sentry/monitors/logic/test_mark_failed.py index d494c7a06f3fb9..de16c502d2136f 100644 --- a/tests/sentry/monitors/logic/test_mark_failed.py +++ b/tests/sentry/monitors/logic/test_mark_failed.py @@ -82,12 +82,12 @@ def test_mark_failed_muted(self, mock_dispatch_incident_occurrence: mock.MagicMo "max_runtime": None, "checkin_margin": None, }, - is_muted=True, ) monitor_environment = MonitorEnvironment.objects.create( monitor=monitor, environment_id=self.environment.id, status=MonitorStatus.OK, + is_muted=True, ) assert monitor_environment.active_incident is None checkin = MonitorCheckIn.objects.create( @@ -106,44 +106,6 @@ def test_mark_failed_muted(self, mock_dispatch_incident_occurrence: mock.MagicMo assert mock_dispatch_incident_occurrence.call_count == 0 assert monitor_environment.active_incident is not None - @mock.patch("sentry.monitors.logic.incidents.dispatch_incident_occurrence") - def test_mark_failed_env_muted(self, mock_dispatch_incident_occurrence: mock.MagicMock) -> None: - monitor = Monitor.objects.create( - name="test monitor", - organization_id=self.organization.id, - project_id=self.project.id, - config={ - "schedule": [1, "month"], - "schedule_type": ScheduleType.INTERVAL, - "max_runtime": None, - "checkin_margin": None, - }, - is_muted=False, - ) - monitor_environment = MonitorEnvironment.objects.create( - monitor=monitor, - environment_id=self.environment.id, - status=MonitorStatus.OK, - is_muted=True, - ) - assert monitor_environment.active_incident is None - - checkin = MonitorCheckIn.objects.create( - monitor=monitor, - monitor_environment=monitor_environment, - project_id=self.project.id, - status=CheckInStatus.UNKNOWN, - ) - assert mark_failed(checkin, failed_at=checkin.date_added) - - monitor.refresh_from_db() - monitor_environment.refresh_from_db() - assert not monitor.is_muted - assert monitor_environment.is_muted - assert monitor_environment.status == MonitorStatus.ERROR - assert mock_dispatch_incident_occurrence.call_count == 0 - assert monitor_environment.active_incident is not None - @mock.patch("sentry.monitors.logic.incidents.dispatch_incident_occurrence") def test_mark_failed_issue_threshold( self, mock_dispatch_incident_occurrence: mock.MagicMock @@ -361,12 +323,12 @@ def test_mark_failed_issue_threshold_disabled( "max_runtime": None, "checkin_margin": None, }, - is_muted=True, ) monitor_environment = MonitorEnvironment.objects.create( monitor=monitor, environment_id=self.environment.id, status=MonitorStatus.OK, + is_muted=True, ) assert monitor_environment.active_incident is None for _ in range(0, failure_issue_threshold): diff --git a/tests/sentry/monitors/logic/test_mark_ok.py b/tests/sentry/monitors/logic/test_mark_ok.py index b60a4525ced18f..b869978404d17b 100644 --- a/tests/sentry/monitors/logic/test_mark_ok.py +++ b/tests/sentry/monitors/logic/test_mark_ok.py @@ -81,7 +81,6 @@ def test_muted_ok(self) -> None: "checkin_margin": None, "recovery_threshold": None, }, - is_muted=True, ) # Start with monitor in an ERROR state diff --git a/tests/sentry/monitors/test_models.py b/tests/sentry/monitors/test_models.py index 8ea7b1c738358d..f22ba43b5a6895 100644 --- a/tests/sentry/monitors/test_models.py +++ b/tests/sentry/monitors/test_models.py @@ -313,40 +313,34 @@ def test_build_occurrence_fingerprint_different_environments(self): assert fingerprint1 == f"crons:{monitor_env1.id}" assert fingerprint2 == f"crons:{monitor_env2.id}" - def test_ensure_environment_inherits_muted_state(self): - """Test that new environments inherit is_muted from their monitor.""" - # Test with muted monitor - muted_monitor = self.create_monitor(is_muted=True) + def test_ensure_environment_matches_monitor_muted_state(self): + """Test that new environments match the monitor's computed is_muted state. + + When creating a new environment, it will be muted if the monitor's is_muted + property returns True (i.e., all existing environments are muted). + """ + # Create monitor with all existing environments muted + muted_monitor = self.create_monitor() + # Create first environment as muted + self.create_monitor_environment( + monitor=muted_monitor, + environment_id=self.environment.id, + is_muted=True, + ) + + # New environment matches the monitor's computed muted state (all envs muted = True) muted_env = MonitorEnvironment.objects.ensure_environment( self.project, muted_monitor, "production" ) assert muted_env.is_muted is True - # Test with unmuted monitor - unmuted_monitor = self.create_monitor(is_muted=False) + # Test with monitor that has no muted environments + unmuted_monitor = self.create_monitor() unmuted_env = MonitorEnvironment.objects.ensure_environment( self.project, unmuted_monitor, "staging" ) assert unmuted_env.is_muted is False - def test_ensure_environment_does_not_override_existing(self): - """Test that ensure_environment doesn't override existing environment's is_muted.""" - monitor = self.create_monitor(is_muted=True) - - # Create an environment with is_muted=False - existing_env = self.create_monitor_environment( - monitor=monitor, - environment_id=self.environment.id, - is_muted=False, - ) - - # Call ensure_environment again - should return existing - env = MonitorEnvironment.objects.ensure_environment( - self.project, monitor, self.environment.name - ) - assert env.id == existing_env.id - assert env.is_muted is False # Should not be overridden - class CronMonitorDataSourceHandlerTest(TestCase): def setUp(self) -> None: @@ -438,12 +432,12 @@ def test_get_current_instance_count(self) -> None: CronMonitorDataSourceHandler.get_current_instance_count(self.organization) -class MonitorEnsureIsMutedTestCase(TestCase): - """Test the ensure_is_muted() dual-write logic for Monitor.""" +class MonitorIsMutedPropertyTestCase(TestCase): + """Test the is_muted computed property for Monitor.""" - def test_ensure_is_muted_all_environments_muted(self): - """Test that monitor becomes muted when all environments are muted.""" - monitor = self.create_monitor(is_muted=False) + def test_is_muted_all_environments_muted(self): + """Test that monitor.is_muted returns True when all environments are muted.""" + monitor = self.create_monitor() env1 = self.create_environment(name="production") env2 = self.create_environment(name="staging") @@ -459,16 +453,12 @@ def test_ensure_is_muted_all_environments_muted(self): is_muted=True, ) - # Call ensure_is_muted - monitor.ensure_is_muted() - - # Verify monitor is now muted - monitor.refresh_from_db() + # Verify monitor.is_muted is True assert monitor.is_muted is True - def test_ensure_is_muted_some_environments_unmuted(self): - """Test that monitor becomes unmuted when any environment is unmuted.""" - monitor = self.create_monitor(is_muted=True) + def test_is_muted_some_environments_unmuted(self): + """Test that monitor.is_muted returns False when any environment is unmuted.""" + monitor = self.create_monitor() env1 = self.create_environment(name="production") env2 = self.create_environment(name="staging") @@ -484,16 +474,12 @@ def test_ensure_is_muted_some_environments_unmuted(self): is_muted=False, ) - # Call ensure_is_muted - monitor.ensure_is_muted() - - # Verify monitor is now unmuted - monitor.refresh_from_db() + # Verify monitor.is_muted is False assert monitor.is_muted is False - def test_ensure_is_muted_all_environments_unmuted(self): - """Test that monitor becomes unmuted when all environments are unmuted.""" - monitor = self.create_monitor(is_muted=True) + def test_is_muted_all_environments_unmuted(self): + """Test that monitor.is_muted returns False when all environments are unmuted.""" + monitor = self.create_monitor() env1 = self.create_environment(name="production") env2 = self.create_environment(name="staging") @@ -509,34 +495,18 @@ def test_ensure_is_muted_all_environments_unmuted(self): is_muted=False, ) - # Call ensure_is_muted - monitor.ensure_is_muted() - - # Verify monitor is unmuted - monitor.refresh_from_db() + # Verify monitor.is_muted is False assert monitor.is_muted is False - def test_ensure_is_muted_no_environments(self): - """Test that monitor is_muted remains unchanged when there are no environments.""" - monitor = self.create_monitor(is_muted=True) - - # Call ensure_is_muted with no environments - monitor.ensure_is_muted() - - # Verify monitor is_muted is unchanged - monitor.refresh_from_db() - assert monitor.is_muted is True - - # Also test with an unmuted monitor - monitor2 = self.create_monitor(is_muted=False) - monitor2.ensure_is_muted() - monitor2.refresh_from_db() - assert monitor2.is_muted is False + def test_is_muted_no_environments(self): + """Test that monitor.is_muted returns False when there are no environments.""" + monitor = self.create_monitor() + assert monitor.is_muted is False - def test_ensure_is_muted_single_environment(self): - """Test ensure_is_muted works correctly with a single environment.""" + def test_is_muted_single_environment(self): + """Test is_muted works correctly with a single environment.""" # Test with muted environment - monitor = self.create_monitor(is_muted=False) + monitor = self.create_monitor() env = self.create_environment(name="production") self.create_monitor_environment( monitor=monitor, @@ -544,12 +514,10 @@ def test_ensure_is_muted_single_environment(self): is_muted=True, ) - monitor.ensure_is_muted() - monitor.refresh_from_db() assert monitor.is_muted is True # Test with unmuted environment - monitor2 = self.create_monitor(is_muted=True) + monitor2 = self.create_monitor() env2 = self.create_environment(name="staging") self.create_monitor_environment( monitor=monitor2, @@ -557,39 +525,4 @@ def test_ensure_is_muted_single_environment(self): is_muted=False, ) - monitor2.ensure_is_muted() - monitor2.refresh_from_db() assert monitor2.is_muted is False - - def test_ensure_is_muted_unmuting_one_environment_unmutes_monitor(self): - """Test that unmuting one environment when all were muted also unmutes the monitor.""" - # Start with monitor and all environments muted - monitor = self.create_monitor(is_muted=True) - env1 = self.create_environment(name="production") - env2 = self.create_environment(name="staging") - - monitor_env1 = self.create_monitor_environment( - monitor=monitor, - environment_id=env1.id, - is_muted=True, - ) - monitor_env2 = self.create_monitor_environment( - monitor=monitor, - environment_id=env2.id, - is_muted=True, - ) - - # Verify initial state - all muted - assert monitor.is_muted is True - assert monitor_env1.is_muted is True - assert monitor_env2.is_muted is True - - # Unmute one environment - monitor_env1.update(is_muted=False) - - # Call ensure_is_muted - monitor.ensure_is_muted() - - # Verify monitor is now unmuted - monitor.refresh_from_db() - assert monitor.is_muted is False diff --git a/tests/sentry/monitors/test_validators.py b/tests/sentry/monitors/test_validators.py index 26838f98255d0b..e6d776f1d08e2b 100644 --- a/tests/sentry/monitors/test_validators.py +++ b/tests/sentry/monitors/test_validators.py @@ -304,13 +304,20 @@ def test_create_with_status_disabled(self): assert monitor.status == ObjectStatus.DISABLED - def test_create_with_is_muted(self): - """Test creating a muted monitor.""" + def test_create_with_is_muted_noop(self): + """Test that creating a monitor with is_muted does nothing. + + Since is_muted is computed from MonitorEnvironment.is_muted, setting is_muted=True + during monitor creation has no effect because there are no environments yet. + A monitor with no environments is always considered unmuted. + + To mute a monitor, you must use the update API after the monitor has environments. + """ data = { "project": self.project.slug, "name": "My Monitor", "type": "cron_job", - "isMuted": True, # Note: camelCase as per API convention + "isMuted": True, # This has no effect on creation "config": {"schedule_type": "crontab", "schedule": "@daily"}, } validator = MonitorValidator(data=data, context=self.context) @@ -318,7 +325,8 @@ def test_create_with_is_muted(self): monitor = validator.save() - assert monitor.is_muted is True + # Monitor has no environments, so is_muted returns False regardless of input + assert monitor.is_muted is False class MonitorValidatorUpdateTest(MonitorTestCase): @@ -508,6 +516,13 @@ def test_update_owner_to_none(self): def test_update_is_muted(self): """Test updating is_muted field.""" + # Create an environment first so the monitor can be muted + env = self.create_monitor_environment( + monitor=self.monitor, + environment_id=self.environment.id, + is_muted=False, + ) + validator = MonitorValidator( instance=self.monitor, data={"is_muted": True}, @@ -523,6 +538,10 @@ def test_update_is_muted(self): updated_monitor = validator.save() assert updated_monitor.is_muted is True + # Verify the environment was also muted + env.refresh_from_db() + assert env.is_muted is True + def test_update_is_muted_propagates_to_environments(self): """Test that muting a monitor propagates to all its environments.""" # Create two monitor environments @@ -561,9 +580,6 @@ def test_update_is_muted_propagates_to_environments(self): def test_update_is_muted_false_propagates_to_environments(self): """Test that unmuting a monitor propagates to all its environments.""" - # Start with a muted monitor - self.monitor.update(is_muted=True) - # Create two muted monitor environments env1 = self.create_monitor_environment( monitor=self.monitor, @@ -577,7 +593,10 @@ def test_update_is_muted_false_propagates_to_environments(self): is_muted=True, ) - # Unmute the monitor + # Verify monitor is muted (all environments are muted) + assert self.monitor.is_muted is True + + # Unmute the monitor via validator validator = MonitorValidator( instance=self.monitor, data={"is_muted": False}, @@ -675,7 +694,6 @@ def test_update_multiple_fields(self): data={ "name": "New Name", "slug": "new-slug", - "is_muted": True, "owner": f"team:{self.team.id}", }, partial=True, @@ -690,7 +708,7 @@ def test_update_multiple_fields(self): updated_monitor = validator.save() assert updated_monitor.name == "New Name" assert updated_monitor.slug == "new-slug" - assert updated_monitor.is_muted is True + assert updated_monitor.is_muted is False assert updated_monitor.owner_team_id == self.team.id def test_update_slug_already_exists(self):