Skip to content

Commit ef00538

Browse files
committed
feat(cron-monitors): Add billing seat management to cron monitor detectors
Implements billing seat management for cron monitor detectors via the MonitorIncidentDetectorValidator. This ensures that cron monitors created through the detector API properly handle seat assignment, validation, and removal. Changes: - Add validate_enabled() to check seat availability before enabling - Modify create() to assign seats with graceful degradation when no seats are available (detector created but disabled) - Modify update() to handle enable/disable transitions with seat operations and race condition handling - Add delete() to remove seats immediately when detector is deleted - Add comprehensive test coverage for all seat management scenarios Uses generic seat APIs (assign_seat, check_assign_seat, disable_seat, remove_seat) with DataCategory.MONITOR following the same pattern as uptime monitors. Fixes [NEW-619: Ensure CRUD / enable+disable Cron Detectors in new UI handles assigning / unassigning seats](https://linear.app/getsentry/issue/NEW-619/ensure-crud-enabledisable-cron-detectors-in-new-ui-handles-assigning)
1 parent e7a917f commit ef00538

File tree

3 files changed

+282
-27
lines changed

3 files changed

+282
-27
lines changed

src/sentry/monitors/models.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050

5151
if TYPE_CHECKING:
5252
from sentry.models.project import Project
53+
from sentry.workflow_engine.models import Detector
5354

5455
MONITOR_CONFIG = {
5556
"type": "object",
@@ -812,6 +813,15 @@ class Meta:
812813
db_table = "sentry_monitorenvbrokendetection"
813814

814815

816+
def get_cron_monitor(detector: Detector) -> Monitor:
817+
"""
818+
Given a detector get the matching cron monitor.
819+
"""
820+
data_source = detector.data_sources.first()
821+
assert data_source
822+
return Monitor.objects.get(id=int(data_source.source_id))
823+
824+
815825
@data_source_type_registry.register(DATA_SOURCE_CRON_MONITOR)
816826
class CronMonitorDataSourceHandler(DataSourceTypeHandler[Monitor]):
817827
@override

src/sentry/monitors/validators.py

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
MonitorLimitsExceeded,
3636
ScheduleType,
3737
check_organization_monitor_limit,
38+
get_cron_monitor,
3839
)
3940
from sentry.monitors.schedule import get_next_schedule, get_prev_schedule
4041
from sentry.monitors.types import CrontabSchedule, slugify_monitor_slug
@@ -52,7 +53,7 @@
5253
BaseDataSourceValidator,
5354
BaseDetectorTypeValidator,
5455
)
55-
from sentry.workflow_engine.models import DataSource, Detector
56+
from sentry.workflow_engine.models import Detector
5657

5758
MONITOR_STATUSES = {
5859
"active": ObjectStatus.ACTIVE,
@@ -373,10 +374,12 @@ def create(self, validated_data):
373374
config=validated_data["config"],
374375
)
375376

376-
# Attempt to assign a seat for this monitor
377-
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
378-
if seat_outcome != Outcome.ACCEPTED:
379-
monitor.update(status=ObjectStatus.DISABLED)
377+
# Skip quota operations if requested by context (e.g., detector flow handles this)
378+
if not self.context.get("skip_quota", False):
379+
# Attempt to assign a seat for this monitor
380+
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
381+
if seat_outcome != Outcome.ACCEPTED:
382+
monitor.update(status=ObjectStatus.DISABLED)
380383

381384
request = self.context["request"]
382385
signal_monitor_created(project, request.user, False, monitor, request)
@@ -636,9 +639,12 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
636639
if self.instance:
637640
monitor_instance = self.instance
638641

642+
# Skip quota operations - the detector validator handles seat assignment
643+
context = {**self.context, "skip_quota": True}
644+
639645
monitor_validator = MonitorValidator(
640646
data=monitor_data,
641-
context=self.context,
647+
context=context,
642648
instance=monitor_instance,
643649
partial=self.partial,
644650
)
@@ -686,16 +692,60 @@ class MonitorIncidentDetectorValidator(BaseDetectorTypeValidator):
686692

687693
data_sources = serializers.ListField(child=MonitorDataSourceValidator(), required=False)
688694

695+
def validate_enabled(self, value: bool) -> bool:
696+
"""
697+
Validate that enabling a detector is allowed based on seat availability.
698+
"""
699+
detector = self.instance
700+
if detector and value and not detector.enabled:
701+
monitor = get_cron_monitor(detector)
702+
result = quotas.backend.check_assign_seat(DataCategory.MONITOR, monitor)
703+
if not result.assignable:
704+
raise serializers.ValidationError(result.reason)
705+
return value
706+
707+
def create(self, validated_data):
708+
detector = super().create(validated_data)
709+
710+
with in_test_hide_transaction_boundary():
711+
monitor = get_cron_monitor(detector)
712+
713+
# Try to assign a seat for the monitor
714+
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
715+
if seat_outcome != Outcome.ACCEPTED:
716+
detector.update(enabled=False)
717+
monitor.update(status=ObjectStatus.DISABLED)
718+
719+
return detector
720+
689721
def update(self, instance: Detector, validated_data: dict[str, Any]) -> Detector:
722+
was_enabled = instance.enabled
723+
enabled = validated_data.get("enabled", was_enabled)
724+
725+
# Handle enable/disable seat operations
726+
if was_enabled != enabled:
727+
monitor = get_cron_monitor(instance)
728+
729+
if enabled:
730+
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
731+
# We should have already validated that a seat was available in
732+
# validate_enabled, avoid races by failing here if we can't
733+
# accept the seat
734+
if seat_outcome != Outcome.ACCEPTED:
735+
raise serializers.ValidationError("Failed to update monitor")
736+
monitor.update(status=ObjectStatus.ACTIVE)
737+
else:
738+
quotas.backend.disable_seat(DataCategory.MONITOR, monitor)
739+
monitor.update(status=ObjectStatus.DISABLED)
740+
690741
super().update(instance, validated_data)
691742

692743
data_source_data = None
693744
if "data_sources" in validated_data:
694745
data_source_data = validated_data.pop("data_sources")[0]
695746

696747
if data_source_data is not None:
697-
data_source = DataSource.objects.get(detectors=instance)
698-
monitor = Monitor.objects.get(id=data_source.source_id)
748+
monitor = get_cron_monitor(instance)
699749

700750
monitor_validator = MonitorDataSourceValidator(
701751
instance=monitor,
@@ -709,3 +759,12 @@ def update(self, instance: Detector, validated_data: dict[str, Any]) -> Detector
709759
monitor_validator.save()
710760

711761
return instance
762+
763+
def delete(self) -> None:
764+
assert self.instance is not None
765+
monitor = get_cron_monitor(self.instance)
766+
767+
# Remove the seat immediately
768+
quotas.backend.remove_seat(DataCategory.MONITOR, monitor)
769+
770+
super().delete()

0 commit comments

Comments
 (0)