Skip to content

Commit 2f68158

Browse files
evanpurkhiserandrewshie-sentry
authored andcommitted
ref(quotas): Use generic quota methods in monitors (#103246)
1 parent d92b140 commit 2f68158

File tree

10 files changed

+87
-92
lines changed

10 files changed

+87
-92
lines changed

src/sentry/monitors/consumers/monitor_consumer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ def _process_checkin(item: CheckinItem, txn: Transaction | Span) -> None:
663663
# When accepting for upsert attempt to assign a seat for the monitor,
664664
# otherwise the monitor is marked as disabled
665665
if monitor and quotas_outcome == PermitCheckInStatus.ACCEPTED_FOR_UPSERT:
666-
seat_outcome = quotas.backend.assign_monitor_seat(monitor)
666+
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
667667
if seat_outcome != Outcome.ACCEPTED:
668668
monitor.update(status=ObjectStatus.DISABLED)
669669

src/sentry/monitors/endpoints/base_monitor_details.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from sentry.api.base import BaseEndpointMixin
1212
from sentry.api.helpers.environments import get_environments
1313
from sentry.api.serializers import serialize
14-
from sentry.constants import ObjectStatus
14+
from sentry.constants import DataCategory, ObjectStatus
1515
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
1616
from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion
1717
from sentry.models.environment import Environment
@@ -132,7 +132,7 @@ def delete_monitor(self, request: Request, project: Project, monitor: Monitor) -
132132
if isinstance(monitor_object, Monitor):
133133
new_slug = get_random_string(length=24)
134134
# we disable the monitor seat so that it can be re-used for another monitor
135-
quotas.backend.disable_monitor_seat(monitor=monitor)
135+
quotas.backend.disable_seat(DataCategory.MONITOR, monitor)
136136
quotas.backend.update_monitor_slug(monitor.slug, new_slug, monitor.project_id)
137137
monitor_object.update(slug=new_slug)
138138

src/sentry/monitors/endpoints/organization_monitor_index.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
)
3131
from sentry.apidocs.parameters import GlobalParams, MonitorParams, OrganizationParams
3232
from sentry.apidocs.utils import inline_sentry_response_serializer
33-
from sentry.constants import ObjectStatus
33+
from sentry.constants import DataCategory, ObjectStatus
3434
from sentry.db.models.query import in_iexact
3535
from sentry.models.environment import Environment
3636
from sentry.models.organization import Organization
@@ -311,7 +311,7 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response:
311311
status = result.get("status")
312312
# If enabling monitors, ensure we can assign all before moving forward
313313
if status == ObjectStatus.ACTIVE:
314-
assign_result = quotas.backend.check_assign_monitor_seats(monitors)
314+
assign_result = quotas.backend.check_assign_seats(DataCategory.MONITOR, monitors)
315315
if not assign_result.assignable:
316316
return self.respond(assign_result.reason, status=400)
317317

@@ -321,14 +321,14 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response:
321321
with transaction.atomic(router.db_for_write(Monitor)):
322322
# Attempt to assign a monitor seat
323323
if status == ObjectStatus.ACTIVE:
324-
outcome = quotas.backend.assign_monitor_seat(monitor)
324+
outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
325325
if outcome != Outcome.ACCEPTED:
326326
errored.append(monitor)
327327
continue
328328

329329
# Attempt to unassign the monitor seat
330330
if status == ObjectStatus.DISABLED:
331-
quotas.backend.disable_monitor_seat(monitor)
331+
quotas.backend.disable_seat(DataCategory.MONITOR, monitor)
332332

333333
monitor.update(**result)
334334
updated.append(monitor)

src/sentry/monitors/validators.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from sentry.api.fields.sentry_slug import SentrySerializerSlugField
1919
from sentry.api.serializers.rest_framework import CamelSnakeSerializer
2020
from sentry.api.serializers.rest_framework.project import ProjectField
21-
from sentry.constants import ObjectStatus
21+
from sentry.constants import DataCategory, ObjectStatus
2222
from sentry.db.models import BoundedPositiveIntegerField
2323
from sentry.db.models.fields.slug import DEFAULT_SLUG_MAX_LENGTH
2424
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
@@ -328,7 +328,7 @@ def validate_status(self, value):
328328
# context. It is the caller's responsibility to ensure that a
329329
# monitor is provided in context for this to be validated.
330330
if status == ObjectStatus.ACTIVE and monitor:
331-
result = quotas.backend.check_assign_monitor_seat(monitor)
331+
result = quotas.backend.check_assign_seat(DataCategory.MONITOR, monitor)
332332
if not result.assignable:
333333
raise ValidationError(result.reason)
334334

@@ -374,7 +374,7 @@ def create(self, validated_data):
374374
)
375375

376376
# Attempt to assign a seat for this monitor
377-
seat_outcome = quotas.backend.assign_monitor_seat(monitor)
377+
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
378378
if seat_outcome != Outcome.ACCEPTED:
379379
monitor.update(status=ObjectStatus.DISABLED)
380380

@@ -441,7 +441,7 @@ def update(self, instance, validated_data):
441441
if "status" in params:
442442
# Attempt to assign a monitor seat
443443
if params["status"] == ObjectStatus.ACTIVE and instance.status != ObjectStatus.ACTIVE:
444-
outcome = quotas.backend.assign_monitor_seat(instance)
444+
outcome = quotas.backend.assign_seat(DataCategory.MONITOR, instance)
445445
# The MonitorValidator checks if a seat assignment is available.
446446
# This protects against a race condition
447447
if outcome != Outcome.ACCEPTED:
@@ -454,7 +454,7 @@ def update(self, instance, validated_data):
454454
params["status"] == ObjectStatus.DISABLED
455455
and instance.status != ObjectStatus.DISABLED
456456
):
457-
quotas.backend.disable_monitor_seat(instance)
457+
quotas.backend.disable_seat(DataCategory.MONITOR, instance)
458458

459459
if params:
460460
instance.update(**params)

src/sentry/quotas/base.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from collections.abc import Iterable, Mapping
3+
from collections.abc import Iterable, Mapping, Sequence
44
from dataclasses import dataclass
55
from enum import IntEnum, unique
66
from typing import TYPE_CHECKING, Any, Literal
@@ -329,7 +329,6 @@ class Quota(Service):
329329
"get_quotas",
330330
"get_blended_sample_rate",
331331
"get_transaction_sampling_tier_for_volume",
332-
"assign_monitor_seat",
333332
"check_accept_monitor_checkin",
334333
"update_monitor_slug",
335334
)
@@ -605,7 +604,7 @@ def check_assign_monitor_seats(self, monitor: list[Monitor]) -> SeatAssignmentRe
605604
return SeatAssignmentResult(assignable=True)
606605

607606
def check_assign_seats(
608-
self, data_category: DataCategory, seat_objects: list[SeatObject]
607+
self, data_category: DataCategory, seat_objects: Sequence[SeatObject]
609608
) -> SeatAssignmentResult:
610609
"""
611610
Determines if a list of assignable seat objects can be assigned seat.

tests/sentry/monitors/consumers/test_monitor_consumer.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from sentry_kafka_schemas.schema_types.ingest_monitors_v1 import CheckIn
1616

1717
from sentry import audit_log, killswitches
18-
from sentry.constants import ObjectStatus
18+
from sentry.constants import DataCategory, ObjectStatus
1919
from sentry.db.models import BoundedPositiveIntegerField
2020
from sentry.models.environment import Environment
2121
from sentry.monitors.constants import TIMEOUT, PermitCheckInStatus
@@ -1332,20 +1332,20 @@ def test_monitor_quotas_drop(self, check_accept_monitor_checkin: mock.MagicMock)
13321332
checkins = MonitorCheckIn.objects.filter(monitor_id=monitor.id)
13331333
assert len(checkins) == 0
13341334

1335-
@mock.patch("sentry.quotas.backend.assign_monitor_seat")
1335+
@mock.patch("sentry.quotas.backend.assign_seat")
13361336
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
13371337
def test_monitor_accept_upsert_with_seat(
13381338
self,
13391339
check_accept_monitor_checkin,
1340-
assign_monitor_seat,
1340+
assign_seat,
13411341
):
13421342
"""
13431343
Validates that a monitor can be upserted and processes a full check-in
13441344
when the PermitCheckInStatus is ACCEPTED_FOR_UPSERT and a seat is
13451345
allocated with a Outcome.ACCEPTED.
13461346
"""
13471347
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
1348-
assign_monitor_seat.return_value = Outcome.ACCEPTED
1348+
assign_seat.return_value = Outcome.ACCEPTED
13491349

13501350
with outbox_runner():
13511351
self.send_checkin(
@@ -1361,28 +1361,28 @@ def test_monitor_accept_upsert_with_seat(
13611361
assert monitor is not None
13621362

13631363
check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
1364-
assign_monitor_seat.assert_called_with(monitor)
1364+
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)
13651365

13661366
assert_org_audit_log_exists(
13671367
organization=self.organization,
13681368
event=audit_log.get_event_id("MONITOR_ADD"),
13691369
data={"upsert": True, **monitor.get_audit_log_data()},
13701370
)
13711371

1372-
@mock.patch("sentry.quotas.backend.assign_monitor_seat")
1372+
@mock.patch("sentry.quotas.backend.assign_seat")
13731373
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
13741374
def test_monitor_accept_upsert_no_seat(
13751375
self,
13761376
check_accept_monitor_checkin,
1377-
assign_monitor_seat,
1377+
assign_seat,
13781378
):
13791379
"""
13801380
Validates that a monitor can be upserted but have the check-in dropped
13811381
when the PermitCheckInStatus is ACCEPTED_FOR_UPSERT and a seat is
13821382
unable to be allocated with a Outcome.RATE_LIMITED
13831383
"""
13841384
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
1385-
assign_monitor_seat.return_value = Outcome.RATE_LIMITED
1385+
assign_seat.return_value = Outcome.RATE_LIMITED
13861386

13871387
self.send_checkin(
13881388
"my-monitor",
@@ -1403,21 +1403,21 @@ def test_monitor_accept_upsert_no_seat(
14031403
assert monitor.status == ObjectStatus.DISABLED
14041404

14051405
check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
1406-
assign_monitor_seat.assert_called_with(monitor)
1406+
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)
14071407

1408-
@mock.patch("sentry.quotas.backend.assign_monitor_seat")
1408+
@mock.patch("sentry.quotas.backend.assign_seat")
14091409
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
14101410
def test_monitor_accept_upsert_existing_monitor(
14111411
self,
14121412
check_accept_monitor_checkin,
1413-
assign_monitor_seat,
1413+
assign_seat,
14141414
):
14151415
"""
14161416
Validate the unusual casse where a seat does not already exist but a
14171417
monitor does exist. We should ensure assign_monitor_seat is called
14181418
"""
14191419
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
1420-
assign_monitor_seat.return_value = Outcome.RATE_LIMITED
1420+
assign_seat.return_value = Outcome.RATE_LIMITED
14211421

14221422
monitor = self._create_monitor(slug="my-monitor")
14231423
self.send_checkin("my-monitor", environment="my-environment")
@@ -1431,4 +1431,4 @@ def test_monitor_accept_upsert_existing_monitor(
14311431
assert monitor.status == ObjectStatus.DISABLED
14321432

14331433
check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
1434-
assign_monitor_seat.assert_called_with(monitor)
1434+
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)

tests/sentry/monitors/endpoints/test_base_monitor_details.py

Lines changed: 33 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import pytest
66

7-
from sentry.constants import ObjectStatus
7+
from sentry.constants import DataCategory, ObjectStatus
88
from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion
99
from sentry.models.environment import Environment
1010
from sentry.models.rule import Rule, RuleActivity, RuleActivityType
@@ -785,13 +785,11 @@ def test_cannot_change_project(self) -> None:
785785
resp.data["detail"]["message"] == "existing monitors may not be moved between projects"
786786
), resp.content
787787

788-
@patch("sentry.quotas.backend.check_assign_monitor_seat")
789-
@patch("sentry.quotas.backend.assign_monitor_seat")
790-
def test_activate_monitor_success(
791-
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
792-
) -> None:
793-
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
794-
assign_monitor_seat.return_value = Outcome.ACCEPTED
788+
@patch("sentry.quotas.backend.check_assign_seat")
789+
@patch("sentry.quotas.backend.assign_seat")
790+
def test_activate_monitor_success(self, assign_seat: MagicMock, check_seat: MagicMock) -> None:
791+
check_seat.return_value = SeatAssignmentResult(assignable=True)
792+
assign_seat.return_value = Outcome.ACCEPTED
795793

796794
monitor = self._create_monitor()
797795
monitor.update(status=ObjectStatus.DISABLED)
@@ -802,15 +800,15 @@ def test_activate_monitor_success(
802800

803801
monitor = Monitor.objects.get(id=monitor.id)
804802
assert monitor.status == ObjectStatus.ACTIVE
805-
assert assign_monitor_seat.called
803+
assert assign_seat.called
806804

807-
@patch("sentry.quotas.backend.check_assign_monitor_seat")
808-
@patch("sentry.quotas.backend.assign_monitor_seat")
805+
@patch("sentry.quotas.backend.check_assign_seat")
806+
@patch("sentry.quotas.backend.assign_seat")
809807
def test_no_activate_if_already_activated(
810-
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
808+
self, assign_seat: MagicMock, check_seat: MagicMock
811809
) -> None:
812-
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
813-
assign_monitor_seat.return_value = Outcome.ACCEPTED
810+
check_seat.return_value = SeatAssignmentResult(assignable=True)
811+
assign_seat.return_value = Outcome.ACCEPTED
814812

815813
monitor = self._create_monitor()
816814

@@ -820,10 +818,10 @@ def test_no_activate_if_already_activated(
820818

821819
monitor = Monitor.objects.get(id=monitor.id)
822820
assert monitor.status == ObjectStatus.ACTIVE
823-
assert not assign_monitor_seat.called
821+
assert not assign_seat.called
824822

825-
@patch("sentry.quotas.backend.disable_monitor_seat")
826-
def test_no_disable_if_already_disabled(self, disable_monitor_seat: MagicMock) -> None:
823+
@patch("sentry.quotas.backend.disable_seat")
824+
def test_no_disable_if_already_disabled(self, disable_seat: MagicMock) -> None:
827825
monitor = self._create_monitor()
828826

829827
self.get_success_response(
@@ -832,21 +830,21 @@ def test_no_disable_if_already_disabled(self, disable_monitor_seat: MagicMock) -
832830
monitor.update(status=ObjectStatus.DISABLED)
833831
monitor = Monitor.objects.get(id=monitor.id)
834832
assert monitor.status == ObjectStatus.DISABLED
835-
assert not disable_monitor_seat.called
833+
assert not disable_seat.called
836834

837835
@patch("sentry.quotas.backend.update_monitor_slug")
838-
@patch("sentry.quotas.backend.check_assign_monitor_seat")
839-
@patch("sentry.quotas.backend.assign_monitor_seat")
836+
@patch("sentry.quotas.backend.check_assign_seat")
837+
@patch("sentry.quotas.backend.assign_seat")
840838
def test_update_slug_sends_right_slug_to_assign(
841-
self, assign_monitor_seat, check_assign_monitor_seat, update_monitor_slug
839+
self, assign_seat, check_seat, update_monitor_slug
842840
):
843-
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
841+
check_seat.return_value = SeatAssignmentResult(assignable=True)
844842

845-
def dummy_assign(monitor):
843+
def dummy_assign(data_category, monitor):
846844
assert monitor.slug == old_slug
847845
return Outcome.ACCEPTED
848846

849-
assign_monitor_seat.side_effect = dummy_assign
847+
assign_seat.side_effect = dummy_assign
850848

851849
monitor = self._create_monitor()
852850
monitor.update(status=ObjectStatus.DISABLED)
@@ -864,16 +862,14 @@ def dummy_assign(monitor):
864862
update_call_args = update_monitor_slug.call_args
865863
assert update_call_args[0] == (old_slug, new_slug, monitor.project_id)
866864

867-
@patch("sentry.quotas.backend.check_assign_monitor_seat")
868-
@patch("sentry.quotas.backend.assign_monitor_seat")
869-
def test_activate_monitor_invalid(
870-
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
871-
) -> None:
865+
@patch("sentry.quotas.backend.check_assign_seat")
866+
@patch("sentry.quotas.backend.assign_seat")
867+
def test_activate_monitor_invalid(self, assign_seat: MagicMock, check_seat: MagicMock) -> None:
872868
result = SeatAssignmentResult(
873869
assignable=False,
874870
reason="Over quota",
875871
)
876-
check_assign_monitor_seat.return_value = result
872+
check_seat.return_value = result
877873

878874
monitor = self._create_monitor()
879875
monitor.update(status=ObjectStatus.DISABLED)
@@ -887,17 +883,17 @@ def test_activate_monitor_invalid(
887883
)
888884

889885
assert resp.data["status"][0] == result.reason
890-
assert not assign_monitor_seat.called
886+
assert not assign_seat.called
891887

892-
@patch("sentry.quotas.backend.disable_monitor_seat")
893-
def test_deactivate_monitor(self, disable_monitor_seat: MagicMock) -> None:
888+
@patch("sentry.quotas.backend.disable_seat")
889+
def test_deactivate_monitor(self, disable_seat: MagicMock) -> None:
894890
monitor = self._create_monitor()
895891

896892
self.get_success_response(
897893
self.organization.slug, monitor.slug, method="PUT", **{"status": "disabled"}
898894
)
899895

900-
assert disable_monitor_seat.called
896+
assert disable_seat.called
901897

902898

903899
class BaseDeleteMonitorTest(MonitorTestCase):
@@ -907,10 +903,10 @@ def setUp(self) -> None:
907903
self.login_as(user=self.user)
908904
super().setUp()
909905

910-
@patch("sentry.quotas.backend.disable_monitor_seat")
906+
@patch("sentry.quotas.backend.disable_seat")
911907
@patch("sentry.quotas.backend.update_monitor_slug")
912908
def test_simple(
913-
self, mock_update_monitor_slug: MagicMock, mock_disable_monitor_seat: MagicMock
909+
self, mock_update_monitor_slug: MagicMock, mock_disable_seat: MagicMock
914910
) -> None:
915911
monitor = self._create_monitor()
916912
old_slug = monitor.slug
@@ -927,7 +923,7 @@ def test_simple(
927923
object_id=monitor.id, model_name="Monitor"
928924
).exists()
929925
mock_update_monitor_slug.assert_called_once()
930-
mock_disable_monitor_seat.assert_called_once_with(monitor=monitor)
926+
mock_disable_seat.assert_called_once_with(DataCategory.MONITOR, monitor)
931927

932928
def test_mismatched_org_slugs(self) -> None:
933929
monitor = self._create_monitor()

0 commit comments

Comments
 (0)