Skip to content

Commit 7a7eabe

Browse files
evanpurkhiserandrewshie-sentry
authored andcommitted
feat(crons): Add billing seat management for detector validators (#102995)
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 94c5643 commit 7a7eabe

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)