Skip to content

Commit a93dfcf

Browse files
feat(crons): Always create detectors for all monitors in MonitorValidator (#104004)
Previously, detectors were only created conditionally (when alert_rule was present), but every cron monitor should have a detector. The original endpoint code from commit 450bde5 always called ensure_cron_detector() for all monitors. Changes: - Moved ensure_cron_detector() call from endpoint into MonitorValidator.create() - Detector creation now happens for ALL monitors (not just those with alert_rule) - Detector is created BEFORE alert_rule creation, allowing IssueAlertMigrator to find and link it to workflows - Added skip_detector_creation context flag to prevent duplicate creation when using MonitorDataSourceValidator (new detector flow) - Updated tests to verify detector creation This ensures both the old (MonitorValidator) and new (detector API) flows correctly create detectors for all monitors.
1 parent 1986052 commit a93dfcf

File tree

6 files changed

+42
-31
lines changed

6 files changed

+42
-31
lines changed

src/sentry/monitors/endpoints/organization_monitor_index.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
MonitorSerializer,
4747
MonitorSerializerResponse,
4848
)
49-
from sentry.monitors.utils import ensure_cron_detector
5049
from sentry.monitors.validators import MonitorBulkEditValidator, MonitorValidator
5150
from sentry.search.utils import tokenize_query
5251
from sentry.types.actor import Actor
@@ -285,7 +284,6 @@ def post(self, request: AuthenticatedHttpRequest, organization) -> Response:
285284
return self.respond(validator.errors, status=400)
286285

287286
monitor = validator.save()
288-
ensure_cron_detector(monitor)
289287
return self.respond(serialize(monitor, request.user), status=201)
290288

291289
@extend_schema(

src/sentry/monitors/utils.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,9 @@ def update_issue_alert_rule(
392392
return issue_alert_rule.id
393393

394394

395-
def ensure_cron_detector(monitor: Monitor):
395+
def ensure_cron_detector(monitor: Monitor) -> Detector | None:
396+
from sentry.monitors.grouptype import MonitorIncidentType
397+
396398
try:
397399
with atomic_transaction(using=router.db_for_write(DataSource)):
398400
data_source, created = DataSource.objects.get_or_create(
@@ -401,8 +403,6 @@ def ensure_cron_detector(monitor: Monitor):
401403
source_id=str(monitor.id),
402404
)
403405
if created:
404-
from sentry.monitors.grouptype import MonitorIncidentType
405-
406406
detector = Detector.objects.create(
407407
type=MonitorIncidentType.slug,
408408
project_id=monitor.project_id,
@@ -412,9 +412,19 @@ def ensure_cron_detector(monitor: Monitor):
412412
config={},
413413
)
414414
DataSourceDetector.objects.create(data_source=data_source, detector=detector)
415+
return detector
416+
else:
417+
return Detector.objects.get(
418+
type=MonitorIncidentType.slug,
419+
project_id=monitor.project_id,
420+
data_sources=data_source,
421+
)
422+
415423
except Exception:
416424
logger.exception("Error creating cron detector")
417425

426+
return None
427+
418428

419429
def ensure_cron_detector_deletion(monitor: Monitor):
420430
with atomic_transaction(using=router.db_for_write(DataSource)):

src/sentry/monitors/validators.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
from sentry.monitors.types import CrontabSchedule, slugify_monitor_slug
4242
from sentry.monitors.utils import (
4343
create_issue_alert_rule,
44+
ensure_cron_detector,
4445
get_checkin_margin,
4546
get_max_runtime,
4647
signal_monitor_created,
@@ -373,16 +374,22 @@ def create(self, validated_data):
373374
config=validated_data["config"],
374375
)
375376

376-
# Skip quota operations if requested by context (e.g., detector flow handles this)
377-
if not self.context.get("skip_quota", False):
377+
# When called from the new detector flow, skip detector and quota operations
378+
# since they're handled at a higher level by the detector validator
379+
from_detector_flow = self.context.get("from_detector_flow", False)
380+
381+
if not from_detector_flow:
382+
detector = ensure_cron_detector(monitor)
383+
assert detector
384+
378385
# Attempt to assign a seat for this monitor
379386
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR_SEAT, monitor)
380387
if seat_outcome != Outcome.ACCEPTED:
388+
detector.update(enabled=False)
381389
monitor.update(status=ObjectStatus.DISABLED)
382390

383391
request = self.context["request"]
384392
signal_monitor_created(project, request.user, False, monitor, request)
385-
386393
validated_issue_alert_rule = validated_data.get("alert_rule")
387394
if validated_issue_alert_rule:
388395
issue_alert_rule_id = create_issue_alert_rule(
@@ -643,12 +650,9 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
643650
if self.instance:
644651
monitor_instance = self.instance
645652

646-
# Skip quota operations - the detector validator handles seat assignment
647-
context = {**self.context, "skip_quota": True}
648-
649653
monitor_validator = MonitorValidator(
650654
data=monitor_data,
651-
context=context,
655+
context={**self.context, "from_detector_flow": True},
652656
instance=monitor_instance,
653657
partial=self.partial,
654658
)

tests/sentry/monitors/endpoints/test_organization_monitor_index.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,10 @@ def test_simple_with_alert_rule(self) -> None:
589589
assert rule is not None
590590
assert rule.environment_id == self.environment.id
591591

592+
# Verify the detector was created
593+
detector = get_detector_for_monitor(monitor)
594+
assert detector is not None
595+
592596
def test_checkin_margin_zero(self) -> None:
593597
# Invalid checkin margin
594598
#

tests/sentry/monitors/test_utils.py

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from django.db import IntegrityError
44

5-
from sentry.monitors.grouptype import MonitorIncidentType
65
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR
76
from sentry.monitors.utils import (
87
ensure_cron_detector,
@@ -20,8 +19,7 @@ def setUp(self) -> None:
2019

2120
def test_creates_data_source_and_detector_for_new_monitor(self) -> None:
2221
assert not get_detector_for_monitor(self.monitor)
23-
ensure_cron_detector(self.monitor)
24-
detector = get_detector_for_monitor(self.monitor)
22+
detector = ensure_cron_detector(self.monitor)
2523
assert detector is not None
2624
assert detector.type == "monitor_check_in_failure"
2725
assert detector.project_id == self.monitor.project_id
@@ -30,32 +28,23 @@ def test_creates_data_source_and_detector_for_new_monitor(self) -> None:
3028
assert detector.owner_team_id == self.monitor.owner_team_id
3129

3230
def test_idempotent_for_existing_data_source(self) -> None:
33-
ensure_cron_detector(self.monitor)
34-
detector = get_detector_for_monitor(self.monitor)
35-
assert detector
36-
ensure_cron_detector(self.monitor)
37-
detector_after = get_detector_for_monitor(self.monitor)
31+
detector = ensure_cron_detector(self.monitor)
32+
assert detector is not None
33+
detector_after = ensure_cron_detector(self.monitor)
3834
assert detector_after is not None
3935
assert detector.id == detector_after.id
4036

4137
def test_with_owner_user(self) -> None:
4238
self.monitor.owner_user_id = self.user.id
4339
self.monitor.save()
44-
ensure_cron_detector(self.monitor)
45-
detector = Detector.objects.get(
46-
type=MonitorIncidentType.slug,
47-
project_id=self.monitor.project_id,
48-
)
40+
detector = ensure_cron_detector(self.monitor)
41+
assert detector is not None
4942
assert detector.owner_user_id == self.user.id
5043
assert detector.owner_team_id is None
5144

5245
def test_with_no_owner(self) -> None:
53-
ensure_cron_detector(self.monitor)
54-
55-
detector = Detector.objects.get(
56-
type=MonitorIncidentType.slug,
57-
project_id=self.monitor.project_id,
58-
)
46+
detector = ensure_cron_detector(self.monitor)
47+
assert detector is not None
5948
assert detector.owner_user_id is None
6049
assert detector.owner_team_id is None
6150

tests/sentry/monitors/test_validators.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
is_monitor_muted,
2020
)
2121
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR
22+
from sentry.monitors.utils import get_detector_for_monitor
2223
from sentry.monitors.validators import (
2324
MonitorDataSourceValidator,
2425
MonitorIncidentDetectorValidator,
@@ -262,6 +263,11 @@ def test_create_monitor_without_seat(self, assign_seat):
262263
monitor.refresh_from_db()
263264
assert monitor.status == ObjectStatus.DISABLED
264265

266+
# Verify the detector is also disabled when quota is exceeded
267+
detector = get_detector_for_monitor(monitor)
268+
assert detector is not None
269+
assert detector.enabled is False
270+
265271
def test_invalid_schedule(self):
266272
data = {
267273
"project": self.project.slug,

0 commit comments

Comments
 (0)