Skip to content

Commit 674fb06

Browse files
committed
feat(uptime): Add billing seat management for detector validators
Add billing seat assignment and removal to UptimeDomainCheckFailureValidator to ensure uptime detectors created, updated, or deleted via the detector APIs correctly handle billing seats. Changes: - Assign seats when creating enabled detectors (gracefully disable if no seats) - Assign/remove seats when enabling/disabling detectors via updates - Remove seats immediately when deleting detectors - Raise validation errors when enabling fails due to no available seats Also add comprehensive tests to verify all billing seat operations work correctly across create, update, and delete operations. Fixes [NEW-527: Ensure CRUD / enable+disable detectors in new UI handles assigning / unassigning seats](https://linear.app/getsentry/issue/NEW-527/ensure-crud-enabledisable-detectors-in-new-ui-handles-assigning)
1 parent 2575556 commit 674fb06

File tree

3 files changed

+275
-11
lines changed

3 files changed

+275
-11
lines changed

src/sentry/uptime/endpoints/validators.py

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
from rest_framework import serializers
88
from rest_framework.fields import URLField
99

10-
from sentry import audit_log
10+
from sentry import audit_log, quotas
1111
from sentry.api.fields import ActorField
1212
from sentry.api.serializers.rest_framework import CamelSnakeSerializer
1313
from sentry.auth.superuser import is_active_superuser
14-
from sentry.constants import ObjectStatus
14+
from sentry.constants import DataCategory, ObjectStatus
1515
from sentry.models.environment import Environment
1616
from sentry.uptime.models import (
1717
UptimeSubscription,
@@ -28,6 +28,10 @@
2828
check_url_limits,
2929
create_uptime_detector,
3030
create_uptime_subscription,
31+
delete_uptime_subscription,
32+
disable_uptime_detector,
33+
enable_uptime_detector,
34+
remove_uptime_seat,
3135
update_uptime_detector,
3236
update_uptime_subscription,
3337
)
@@ -462,7 +466,47 @@ def create_source(self, validated_data: dict[str, Any]) -> UptimeSubscription:
462466
class UptimeDomainCheckFailureValidator(BaseDetectorTypeValidator):
463467
data_sources = serializers.ListField(child=UptimeMonitorDataSourceValidator(), required=False)
464468

469+
def validate_enabled(self, value: bool) -> bool:
470+
"""
471+
Validate that enabling a detector is allowed based on seat availability.
472+
473+
This check will ONLY be performed when a detector instance is provided via
474+
context (i.e., during updates). For creation, seat assignment is handled
475+
in the create() method after the detector is created.
476+
"""
477+
detector = self.instance
478+
479+
# Only validate on updates when trying to enable a currently disabled detector
480+
if detector and value and not detector.enabled:
481+
result = quotas.backend.check_assign_seat(DataCategory.UPTIME, detector)
482+
if not result.assignable:
483+
raise serializers.ValidationError(result.reason)
484+
485+
return value
486+
487+
def create(self, validated_data):
488+
detector = super().create(validated_data)
489+
490+
try:
491+
enable_uptime_detector(detector, ensure_assignment=True)
492+
except UptimeMonitorNoSeatAvailable:
493+
# No need to do anything if we failed to handle seat
494+
# assignment. The monitor will be created, but not enabled
495+
pass
496+
497+
return detector
498+
465499
def update(self, instance: Detector, validated_data: dict[str, Any]) -> Detector:
500+
# Handle seat management when enabling/disabling
501+
was_enabled = instance.enabled
502+
enabled = validated_data.get("enabled", was_enabled)
503+
504+
if was_enabled != enabled:
505+
if enabled:
506+
enable_uptime_detector(instance)
507+
else:
508+
disable_uptime_detector(instance)
509+
466510
super().update(instance, validated_data)
467511

468512
data_source = None
@@ -497,5 +541,11 @@ def update_data_source(self, instance: Detector, data_source: dict[str, Any]):
497541

498542
return instance
499543

500-
def create(self, validated_data):
501-
return super().create(validated_data)
544+
def delete(self) -> None:
545+
assert self.instance is not None
546+
547+
remove_uptime_seat(self.instance)
548+
uptime_subscription = get_uptime_subscription(self.instance)
549+
delete_uptime_subscription(uptime_subscription)
550+
551+
super().delete()

src/sentry/uptime/subscriptions/subscriptions.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,18 @@ def disable_uptime_detector(detector: Detector, skip_quotas: bool = False):
476476
delete_remote_uptime_subscription.delay(uptime_subscription.id)
477477

478478

479+
def ensure_uptime_seat(detector: Detector) -> None:
480+
"""
481+
Ensures that a billing seat is assigned for the uptime detector.
482+
483+
Raises UptimeMonitorNoSeatAvailable if no seats are available.
484+
"""
485+
outcome = quotas.backend.assign_seat(DataCategory.UPTIME, detector)
486+
if outcome != Outcome.ACCEPTED:
487+
result = quotas.backend.check_assign_seat(DataCategory.UPTIME, detector)
488+
raise UptimeMonitorNoSeatAvailable(result)
489+
490+
479491
def enable_uptime_detector(
480492
detector: Detector, ensure_assignment: bool = False, skip_quotas: bool = False
481493
):
@@ -494,11 +506,11 @@ def enable_uptime_detector(
494506
return
495507

496508
if not skip_quotas:
497-
outcome = quotas.backend.assign_seat(DataCategory.UPTIME, detector)
498-
if outcome != Outcome.ACCEPTED:
499-
disable_uptime_detector(detector)
500-
result = quotas.backend.check_assign_seat(DataCategory.UPTIME, detector)
501-
raise UptimeMonitorNoSeatAvailable(result)
509+
try:
510+
ensure_uptime_seat(detector)
511+
except UptimeMonitorNoSeatAvailable:
512+
disable_uptime_detector(detector, skip_quotas=True)
513+
raise
502514

503515
uptime_subscription: UptimeSubscription = get_uptime_subscription(detector)
504516
detector.update(enabled=True)
@@ -513,9 +525,14 @@ def enable_uptime_detector(
513525
)
514526

515527

528+
def remove_uptime_seat(detector: Detector):
529+
quotas.backend.remove_seat(DataCategory.UPTIME, detector)
530+
531+
516532
def delete_uptime_detector(detector: Detector):
517533
uptime_subscription = get_uptime_subscription(detector)
518-
quotas.backend.remove_seat(DataCategory.UPTIME, detector)
534+
535+
remove_uptime_seat(detector)
519536
detector.update(status=ObjectStatus.PENDING_DELETION)
520537
RegionScheduledDeletion.schedule(detector, days=0)
521538
delete_uptime_subscription(uptime_subscription)

tests/sentry/uptime/endpoints/test_validators.py

Lines changed: 198 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,21 @@
1+
from unittest import mock
2+
3+
from sentry.constants import DataCategory
4+
from sentry.quotas.base import SeatAssignmentResult
15
from sentry.testutils.cases import TestCase, UptimeTestCase
26
from sentry.uptime.endpoints.validators import (
7+
UptimeDomainCheckFailureValidator,
38
UptimeMonitorDataSourceValidator,
49
compute_http_request_size,
510
)
6-
from sentry.uptime.models import UptimeSubscription
11+
from sentry.uptime.grouptype import UptimeDomainCheckFailure
12+
from sentry.uptime.models import UptimeSubscription, get_uptime_subscription
13+
from sentry.uptime.types import (
14+
DEFAULT_DOWNTIME_THRESHOLD,
15+
DEFAULT_RECOVERY_THRESHOLD,
16+
UptimeMonitorMode,
17+
)
18+
from sentry.utils.outcomes import Outcome
719

820

921
class ComputeHttpRequestSizeTest(UptimeTestCase):
@@ -97,3 +109,188 @@ def test_too_big_request(self):
97109
data = self.get_valid_data(body="0" * 1000)
98110
validator = UptimeMonitorDataSourceValidator(data=data, context=self.context)
99111
assert not validator.is_valid()
112+
113+
114+
class UptimeDomainCheckFailureValidatorTest(UptimeTestCase):
115+
def setUp(self):
116+
super().setUp()
117+
self.context = {
118+
"organization": self.organization,
119+
"project": self.project,
120+
"request": self.make_request(user=self.user),
121+
}
122+
123+
def get_valid_data(self, **kwargs):
124+
return {
125+
"name": kwargs.get("name", "Test Uptime Monitor"),
126+
"type": UptimeDomainCheckFailure.slug,
127+
"enabled": kwargs.get("enabled", True),
128+
"config": kwargs.get(
129+
"config",
130+
{
131+
"mode": UptimeMonitorMode.MANUAL.value,
132+
"environment": None,
133+
"recovery_threshold": DEFAULT_RECOVERY_THRESHOLD,
134+
"downtime_threshold": DEFAULT_DOWNTIME_THRESHOLD,
135+
},
136+
),
137+
"dataSources": kwargs.get(
138+
"data_sources",
139+
[
140+
{
141+
"url": "https://sentry.io",
142+
"intervalSeconds": 60,
143+
"timeoutMs": 1000,
144+
}
145+
],
146+
),
147+
}
148+
149+
@mock.patch(
150+
"sentry.quotas.backend.assign_seat",
151+
return_value=Outcome.ACCEPTED,
152+
)
153+
def test_create_enabled_assigns_seat(self, mock_assign_seat: mock.MagicMock) -> None:
154+
"""Test that creating an enabled detector assigns a billing seat."""
155+
validator = UptimeDomainCheckFailureValidator(
156+
data=self.get_valid_data(enabled=True), context=self.context
157+
)
158+
assert validator.is_valid(), validator.errors
159+
detector = validator.save()
160+
161+
detector.refresh_from_db()
162+
assert detector.enabled is True
163+
164+
# Verify seat was assigned
165+
mock_assign_seat.assert_called_with(DataCategory.UPTIME, detector)
166+
167+
@mock.patch(
168+
"sentry.quotas.backend.assign_seat",
169+
return_value=Outcome.RATE_LIMITED,
170+
)
171+
def test_create_enabled_no_seat_available(self, mock_assign_seat: mock.MagicMock) -> None:
172+
"""Test that creating a detector with no seats available creates it but leaves it disabled."""
173+
validator = UptimeDomainCheckFailureValidator(
174+
data=self.get_valid_data(enabled=True), context=self.context
175+
)
176+
assert validator.is_valid(), validator.errors
177+
detector = validator.save()
178+
179+
detector.refresh_from_db()
180+
# Detector created but not enabled due to no seat assignment
181+
assert detector.enabled is False
182+
183+
# Verify seat assignment was attempted
184+
mock_assign_seat.assert_called_with(DataCategory.UPTIME, detector)
185+
186+
uptime_subscription = get_uptime_subscription(detector)
187+
assert uptime_subscription.status == UptimeSubscription.Status.DISABLED.value
188+
189+
@mock.patch(
190+
"sentry.quotas.backend.assign_seat",
191+
return_value=Outcome.ACCEPTED,
192+
)
193+
def test_update_enable_assigns_seat(self, mock_assign_seat: mock.MagicMock) -> None:
194+
"""Test that enabling a previously disabled detector assigns a seat."""
195+
# Create a disabled detector
196+
detector = self.create_uptime_detector(enabled=False)
197+
198+
validator = UptimeDomainCheckFailureValidator(
199+
instance=detector, data={"enabled": True}, context=self.context, partial=True
200+
)
201+
assert validator.is_valid(), validator.errors
202+
validator.save()
203+
204+
detector.refresh_from_db()
205+
assert detector.enabled is True
206+
207+
# Verify seat was assigned
208+
mock_assign_seat.assert_called_with(DataCategory.UPTIME, detector)
209+
210+
uptime_subscription = get_uptime_subscription(detector)
211+
assert uptime_subscription.status == UptimeSubscription.Status.ACTIVE.value
212+
213+
@mock.patch(
214+
"sentry.quotas.backend.check_assign_seat",
215+
return_value=SeatAssignmentResult(assignable=False, reason="No seats available"),
216+
)
217+
def test_update_enable_no_seat_available(self, mock_check_assign_seat: mock.MagicMock) -> None:
218+
"""Test that enabling fails with validation error when no seats are available."""
219+
# Create a disabled detector
220+
detector = self.create_uptime_detector(enabled=False)
221+
222+
validator = UptimeDomainCheckFailureValidator(
223+
instance=detector, data={"enabled": True}, context=self.context, partial=True
224+
)
225+
226+
# Validation should fail due to no seats available
227+
assert not validator.is_valid()
228+
assert "enabled" in validator.errors
229+
assert validator.errors["enabled"] == ["No seats available"]
230+
231+
detector.refresh_from_db()
232+
# Detector should still be disabled
233+
assert detector.enabled is False
234+
235+
# Verify seat availability check was performed
236+
mock_check_assign_seat.assert_called_with(DataCategory.UPTIME, detector)
237+
238+
@mock.patch("sentry.quotas.backend.disable_seat")
239+
def test_update_disable_removes_seat(self, mock_disable_seat: mock.MagicMock) -> None:
240+
"""Test that disabling a previously enabled detector removes the seat."""
241+
# Create an enabled detector
242+
detector = self.create_uptime_detector(enabled=True)
243+
244+
validator = UptimeDomainCheckFailureValidator(
245+
instance=detector, data={"enabled": False}, context=self.context, partial=True
246+
)
247+
assert validator.is_valid(), validator.errors
248+
validator.save()
249+
250+
detector.refresh_from_db()
251+
assert detector.enabled is False
252+
253+
# Verify disable_seat was called
254+
mock_disable_seat.assert_called_with(DataCategory.UPTIME, detector)
255+
256+
uptime_subscription = get_uptime_subscription(detector)
257+
assert uptime_subscription.status == UptimeSubscription.Status.DISABLED.value
258+
259+
@mock.patch("sentry.quotas.backend.remove_seat")
260+
def test_delete_removes_seat(self, mock_remove_seat: mock.MagicMock) -> None:
261+
"""Test that deleting a detector removes its billing seat immediately."""
262+
detector = self.create_uptime_detector(enabled=True)
263+
264+
validator = UptimeDomainCheckFailureValidator(
265+
instance=detector, data={}, context=self.context
266+
)
267+
268+
validator.delete()
269+
270+
# Verify remove_seat was called immediately
271+
mock_remove_seat.assert_called_with(DataCategory.UPTIME, detector)
272+
273+
@mock.patch(
274+
"sentry.quotas.backend.assign_seat",
275+
return_value=Outcome.ACCEPTED,
276+
)
277+
def test_update_no_enable_change_no_seat_call(self, mock_assign_seat: mock.MagicMock) -> None:
278+
"""Test that updating without changing enabled status doesn't trigger seat operations."""
279+
# Create an enabled detector
280+
detector = self.create_uptime_detector(enabled=True)
281+
282+
# Clear any previous mock calls from creation
283+
mock_assign_seat.reset_mock()
284+
285+
validator = UptimeDomainCheckFailureValidator(
286+
instance=detector, data={"name": "Updated Name"}, context=self.context, partial=True
287+
)
288+
assert validator.is_valid(), validator.errors
289+
validator.save()
290+
291+
detector.refresh_from_db()
292+
assert detector.name == "Updated Name"
293+
assert detector.enabled is True
294+
295+
# Verify no seat operations were called
296+
mock_assign_seat.assert_not_called()

0 commit comments

Comments
 (0)