Skip to content

Commit b333159

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 b333159

File tree

3 files changed

+274
-9
lines changed

3 files changed

+274
-9
lines changed

src/sentry/uptime/endpoints/validators.py

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -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,35 @@ 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 create(self, validated_data):
470+
detector = super().create(validated_data)
471+
472+
if detector.enabled:
473+
try:
474+
enable_uptime_detector(detector, ensure_assignment=True)
475+
except UptimeMonitorNoSeatAvailable:
476+
# No need to do anything if we failed to handle seat
477+
# assignment. The monitor will be created, but not enabled
478+
pass
479+
else:
480+
disable_uptime_detector(detector)
481+
482+
return detector
483+
465484
def update(self, instance: Detector, validated_data: dict[str, Any]) -> Detector:
485+
# Handle seat management when enabling/disabling
486+
was_enabled = instance.enabled
487+
enabled = validated_data.get("enabled", was_enabled)
488+
489+
if was_enabled != enabled:
490+
if enabled:
491+
try:
492+
enable_uptime_detector(instance)
493+
except UptimeMonitorNoSeatAvailable as err:
494+
raise serializers.ValidationError({"enabled": [err.result.reason]})
495+
else:
496+
disable_uptime_detector(instance)
497+
466498
super().update(instance, validated_data)
467499

468500
data_source = None
@@ -497,5 +529,11 @@ def update_data_source(self, instance: Detector, data_source: dict[str, Any]):
497529

498530
return instance
499531

500-
def create(self, validated_data):
501-
return super().create(validated_data)
532+
def delete(self) -> None:
533+
assert self.instance is not None
534+
535+
remove_uptime_seat(self.instance)
536+
uptime_subscription = get_uptime_subscription(self.instance)
537+
delete_uptime_subscription(uptime_subscription)
538+
539+
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: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,24 @@
1+
from unittest import mock
2+
3+
import pytest
4+
from rest_framework import serializers
5+
6+
from sentry.constants import DataCategory
7+
from sentry.quotas.base import SeatAssignmentResult
18
from sentry.testutils.cases import TestCase, UptimeTestCase
29
from sentry.uptime.endpoints.validators import (
10+
UptimeDomainCheckFailureValidator,
311
UptimeMonitorDataSourceValidator,
412
compute_http_request_size,
513
)
6-
from sentry.uptime.models import UptimeSubscription
14+
from sentry.uptime.grouptype import UptimeDomainCheckFailure
15+
from sentry.uptime.models import UptimeSubscription, get_uptime_subscription
16+
from sentry.uptime.types import (
17+
DEFAULT_DOWNTIME_THRESHOLD,
18+
DEFAULT_RECOVERY_THRESHOLD,
19+
UptimeMonitorMode,
20+
)
21+
from sentry.utils.outcomes import Outcome
722

823

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

0 commit comments

Comments
 (0)