Skip to content

Conversation

@evanpurkhiser
Copy link
Member

Implements Step 2 of the migration plan from NEW-564: There needs to be some way to mute the entire cron detector

This data migration backfills MonitorEnvironment.is_muted from Monitor.is_muted for all muted monitors. This ensures existing data is consistent before switching to read from MonitorEnvironment.is_muted.

After this migration runs:

  • Monitors with is_muted=True will have ALL environments muted
  • Monitors with is_muted=False will have environments unchanged

This establishes the correct invariant:

  • Monitor is muted ↔ ALL environments are muted
  • Monitor is unmuted ↔ ANY environment is unmuted

The migration:

  • Uses range queries for efficient batching (1000 monitors per batch)
  • Only updates environments for muted monitors (unmuted is the default)
  • Is marked as post-deployment for manual execution in production

Test verifies:

  • Muted monitor environments are updated to is_muted=True
  • Unmuted monitor environments remain unchanged
  • Monitors without environments are unaffected

@evanpurkhiser evanpurkhiser requested review from a team as code owners November 13, 2025 19:33
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ions/0011_backfill_monitor_environment_is_muted.py 87.50% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103324      +/-   ##
===========================================
+ Coverage   79.87%    80.57%   +0.69%     
===========================================
  Files        9237      9242       +5     
  Lines      394575    394768     +193     
  Branches    25146     25146              
===========================================
+ Hits       315184    318087    +2903     
+ Misses      78944     76234    -2710     
  Partials      447       447              

@evanpurkhiser evanpurkhiser removed the request for review from a team November 13, 2025 19:40
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py

for 0011_backfill_monitor_environment_is_muted in monitors

--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL

@evanpurkhiser evanpurkhiser marked this pull request as draft November 13, 2025 19:57
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from dfd68e6 to 44fa9fa Compare November 13, 2025 22:03
Comment on lines 29 to 36
muted_monitors = list(
Monitor.objects.filter(
id__gt=last_id,
is_muted=True,
).order_by(
"id"
)[:batch_size]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may as well just use RangeQuerySetWrapper and filter is_muted in memory. There's no index on id, is_muted so writing this manually won't help at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look again

@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from 44fa9fa to 88a0a41 Compare November 13, 2025 22:09
Comment on lines 29 to 31
muted_monitors = Monitor.objects.filter(is_muted=True)

for monitor in RangeQuerySetWrapperWithProgressBar(muted_monitors):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're better off filtering is_muted in memory here instead of the query, otherwise you might end up with timeouts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from 88a0a41 to 9524607 Compare November 13, 2025 22:40
@evanpurkhiser evanpurkhiser marked this pull request as ready for review November 13, 2025 22:41
@evanpurkhiser evanpurkhiser enabled auto-merge (squash) November 13, 2025 22:41
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from 9524607 to 196a9e9 Compare November 14, 2025 19:47
@evanpurkhiser evanpurkhiser requested a review from a team as a code owner November 14, 2025 19:47
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from 196a9e9 to 2cc96dd Compare November 14, 2025 22:22
Implements Step 2 of the migration plan from [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)

This data migration backfills MonitorEnvironment.is_muted from Monitor.is_muted for all muted monitors. This ensures existing data is consistent before switching to read from MonitorEnvironment.is_muted.

After this migration runs:
- Monitors with is_muted=True will have ALL environments muted
- Monitors with is_muted=False will have environments unchanged

This establishes the correct invariant:
- Monitor is muted ↔ ALL environments are muted
- Monitor is unmuted ↔ ANY environment is unmuted

The migration:
- Uses range queries for efficient batching (1000 monitors per batch)
- Only updates environments for muted monitors (unmuted is the default)
- Is marked as post-deployment for manual execution in production

Test verifies:
- Muted monitor environments are updated to is_muted=True
- Unmuted monitor environments remain unchanged
- Monitors without environments are unaffected
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-crons-add-migration-to-backfill-monitorenvironment-is-muted branch from 2cc96dd to af3ae18 Compare November 14, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants