Skip to content

Commit 14e9f3b

Browse files
feat(uptime): Add billing seat management for detector validators (#102989)
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 62cf0fa commit 14e9f3b

File tree

4 files changed

+281
-25
lines changed

4 files changed

+281
-25
lines changed
Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,26 @@
11
from sentry.deletions.base import ModelDeletionTask
2-
from sentry.uptime.models import UptimeSubscription
2+
from sentry.uptime.models import UptimeSubscription, get_detector
3+
from sentry.uptime.subscriptions.subscriptions import delete_uptime_subscription, remove_uptime_seat
34

45

56
class UptimeSubscriptionDeletionTask(ModelDeletionTask[UptimeSubscription]):
67
def delete_instance(self, instance: UptimeSubscription) -> None:
7-
from sentry import quotas
8-
from sentry.constants import DataCategory
9-
from sentry.uptime.models import get_detector
10-
from sentry.uptime.subscriptions.subscriptions import delete_uptime_subscription
118

129
detector = get_detector(instance)
1310

1411
# XXX: Typically quota updates would be handled by the
1512
# delete_uptime_detector function exposed in the
1613
# uptime.subscriptions.subscriptions module. However if a Detector is
1714
# deleted without using this, we need to still ensure the billing east
18-
# is revoked.
15+
# is revoked. This should never happen.
1916
#
2017
# Since the delete_uptime_detector function is already scheduling the
2118
# detector for deletion, you may think we could remove the quota
2219
# remove_seat call there, since it will happen here. But this would
2320
# mean the customers quota is not freed up _immediately_ when the
2421
# detector is deleted using that method.
22+
remove_uptime_seat(detector)
2523

26-
# TODO(epurkhiser): It's very likely the new Workflow Engine detector
27-
# APIs will NOT free up customers seats immediately. We'll probably
28-
# need some other hook for this
29-
30-
# Ensure the billing seat for the parent detector is removed
31-
quotas.backend.remove_seat(DataCategory.UPTIME, detector)
32-
33-
# Ensure the remote subscription is also removed
24+
# Ensure the remote subscription is removed if it wasn't already (again
25+
# it should have been as part of delete_uptime_detector)
3426
delete_uptime_subscription(instance)

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)