Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/monitors/consumers/monitor_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ def _process_checkin(item: CheckinItem, txn: Transaction | Span) -> None:
# When accepting for upsert attempt to assign a seat for the monitor,
# otherwise the monitor is marked as disabled
if monitor and quotas_outcome == PermitCheckInStatus.ACCEPTED_FOR_UPSERT:
seat_outcome = quotas.backend.assign_monitor_seat(monitor)
seat_outcome = quotas.backend.assign_seat(DataCategory.MONITOR, monitor)
if seat_outcome != Outcome.ACCEPTED:
monitor.update(status=ObjectStatus.DISABLED)

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/monitors/endpoints/base_monitor_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sentry.api.base import BaseEndpointMixin
from sentry.api.helpers.environments import get_environments
from sentry.api.serializers import serialize
from sentry.constants import ObjectStatus
from sentry.constants import DataCategory, ObjectStatus
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion
from sentry.models.environment import Environment
Expand Down Expand Up @@ -132,7 +132,7 @@ def delete_monitor(self, request: Request, project: Project, monitor: Monitor) -
if isinstance(monitor_object, Monitor):
new_slug = get_random_string(length=24)
# we disable the monitor seat so that it can be re-used for another monitor
quotas.backend.disable_monitor_seat(monitor=monitor)
quotas.backend.disable_seat(DataCategory.MONITOR, monitor)
quotas.backend.update_monitor_slug(monitor.slug, new_slug, monitor.project_id)
monitor_object.update(slug=new_slug)

Expand Down
8 changes: 4 additions & 4 deletions src/sentry/monitors/endpoints/organization_monitor_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)
from sentry.apidocs.parameters import GlobalParams, MonitorParams, OrganizationParams
from sentry.apidocs.utils import inline_sentry_response_serializer
from sentry.constants import ObjectStatus
from sentry.constants import DataCategory, ObjectStatus
from sentry.db.models.query import in_iexact
from sentry.models.environment import Environment
from sentry.models.organization import Organization
Expand Down Expand Up @@ -311,7 +311,7 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response:
status = result.get("status")
# If enabling monitors, ensure we can assign all before moving forward
if status == ObjectStatus.ACTIVE:
assign_result = quotas.backend.check_assign_monitor_seats(monitors)
assign_result = quotas.backend.check_assign_seats(DataCategory.MONITOR, monitors)
if not assign_result.assignable:
return self.respond(assign_result.reason, status=400)

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

# Attempt to unassign the monitor seat
if status == ObjectStatus.DISABLED:
quotas.backend.disable_monitor_seat(monitor)
quotas.backend.disable_seat(DataCategory.MONITOR, monitor)

monitor.update(**result)
updated.append(monitor)
Expand Down
10 changes: 5 additions & 5 deletions src/sentry/monitors/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from sentry.api.fields.sentry_slug import SentrySerializerSlugField
from sentry.api.serializers.rest_framework import CamelSnakeSerializer
from sentry.api.serializers.rest_framework.project import ProjectField
from sentry.constants import ObjectStatus
from sentry.constants import DataCategory, ObjectStatus
from sentry.db.models import BoundedPositiveIntegerField
from sentry.db.models.fields.slug import DEFAULT_SLUG_MAX_LENGTH
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
Expand Down Expand Up @@ -328,7 +328,7 @@ def validate_status(self, value):
# context. It is the caller's responsibility to ensure that a
# monitor is provided in context for this to be validated.
if status == ObjectStatus.ACTIVE and monitor:
result = quotas.backend.check_assign_monitor_seat(monitor)
result = quotas.backend.check_assign_seat(DataCategory.MONITOR, monitor)
if not result.assignable:
raise ValidationError(result.reason)

Expand Down Expand Up @@ -374,7 +374,7 @@ def create(self, validated_data):
)

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

Expand Down Expand Up @@ -441,7 +441,7 @@ def update(self, instance, validated_data):
if "status" in params:
# Attempt to assign a monitor seat
if params["status"] == ObjectStatus.ACTIVE and instance.status != ObjectStatus.ACTIVE:
outcome = quotas.backend.assign_monitor_seat(instance)
outcome = quotas.backend.assign_seat(DataCategory.MONITOR, instance)
# The MonitorValidator checks if a seat assignment is available.
# This protects against a race condition
if outcome != Outcome.ACCEPTED:
Expand All @@ -454,7 +454,7 @@ def update(self, instance, validated_data):
params["status"] == ObjectStatus.DISABLED
and instance.status != ObjectStatus.DISABLED
):
quotas.backend.disable_monitor_seat(instance)
quotas.backend.disable_seat(DataCategory.MONITOR, instance)

if params:
instance.update(**params)
Expand Down
5 changes: 2 additions & 3 deletions src/sentry/quotas/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from collections.abc import Iterable, Mapping
from collections.abc import Iterable, Mapping, Sequence
from dataclasses import dataclass
from enum import IntEnum, unique
from typing import TYPE_CHECKING, Any, Literal
Expand Down Expand Up @@ -329,7 +329,6 @@ class Quota(Service):
"get_quotas",
"get_blended_sample_rate",
"get_transaction_sampling_tier_for_volume",
"assign_monitor_seat",
"check_accept_monitor_checkin",
"update_monitor_slug",
)
Expand Down Expand Up @@ -605,7 +604,7 @@ def check_assign_monitor_seats(self, monitor: list[Monitor]) -> SeatAssignmentRe
return SeatAssignmentResult(assignable=True)

def check_assign_seats(
self, data_category: DataCategory, seat_objects: list[SeatObject]
self, data_category: DataCategory, seat_objects: Sequence[SeatObject]
) -> SeatAssignmentResult:
"""
Determines if a list of assignable seat objects can be assigned seat.
Expand Down
26 changes: 13 additions & 13 deletions tests/sentry/monitors/consumers/test_monitor_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from sentry_kafka_schemas.schema_types.ingest_monitors_v1 import CheckIn

from sentry import audit_log, killswitches
from sentry.constants import ObjectStatus
from sentry.constants import DataCategory, ObjectStatus
from sentry.db.models import BoundedPositiveIntegerField
from sentry.models.environment import Environment
from sentry.monitors.constants import TIMEOUT, PermitCheckInStatus
Expand Down Expand Up @@ -1332,20 +1332,20 @@ def test_monitor_quotas_drop(self, check_accept_monitor_checkin: mock.MagicMock)
checkins = MonitorCheckIn.objects.filter(monitor_id=monitor.id)
assert len(checkins) == 0

@mock.patch("sentry.quotas.backend.assign_monitor_seat")
@mock.patch("sentry.quotas.backend.assign_seat")
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
def test_monitor_accept_upsert_with_seat(
self,
check_accept_monitor_checkin,
assign_monitor_seat,
assign_seat,
):
"""
Validates that a monitor can be upserted and processes a full check-in
when the PermitCheckInStatus is ACCEPTED_FOR_UPSERT and a seat is
allocated with a Outcome.ACCEPTED.
"""
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
assign_monitor_seat.return_value = Outcome.ACCEPTED
assign_seat.return_value = Outcome.ACCEPTED

with outbox_runner():
self.send_checkin(
Expand All @@ -1361,28 +1361,28 @@ def test_monitor_accept_upsert_with_seat(
assert monitor is not None

check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
assign_monitor_seat.assert_called_with(monitor)
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)

assert_org_audit_log_exists(
organization=self.organization,
event=audit_log.get_event_id("MONITOR_ADD"),
data={"upsert": True, **monitor.get_audit_log_data()},
)

@mock.patch("sentry.quotas.backend.assign_monitor_seat")
@mock.patch("sentry.quotas.backend.assign_seat")
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
def test_monitor_accept_upsert_no_seat(
self,
check_accept_monitor_checkin,
assign_monitor_seat,
assign_seat,
):
"""
Validates that a monitor can be upserted but have the check-in dropped
when the PermitCheckInStatus is ACCEPTED_FOR_UPSERT and a seat is
unable to be allocated with a Outcome.RATE_LIMITED
"""
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
assign_monitor_seat.return_value = Outcome.RATE_LIMITED
assign_seat.return_value = Outcome.RATE_LIMITED

self.send_checkin(
"my-monitor",
Expand All @@ -1403,21 +1403,21 @@ def test_monitor_accept_upsert_no_seat(
assert monitor.status == ObjectStatus.DISABLED

check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
assign_monitor_seat.assert_called_with(monitor)
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)

@mock.patch("sentry.quotas.backend.assign_monitor_seat")
@mock.patch("sentry.quotas.backend.assign_seat")
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
def test_monitor_accept_upsert_existing_monitor(
self,
check_accept_monitor_checkin,
assign_monitor_seat,
assign_seat,
):
"""
Validate the unusual casse where a seat does not already exist but a
monitor does exist. We should ensure assign_monitor_seat is called
"""
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
assign_monitor_seat.return_value = Outcome.RATE_LIMITED
assign_seat.return_value = Outcome.RATE_LIMITED

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

check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
assign_monitor_seat.assert_called_with(monitor)
assign_seat.assert_called_with(DataCategory.MONITOR, monitor)
70 changes: 33 additions & 37 deletions tests/sentry/monitors/endpoints/test_base_monitor_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import pytest

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

@patch("sentry.quotas.backend.check_assign_monitor_seat")
@patch("sentry.quotas.backend.assign_monitor_seat")
def test_activate_monitor_success(
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
) -> None:
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
assign_monitor_seat.return_value = Outcome.ACCEPTED
@patch("sentry.quotas.backend.check_assign_seat")
@patch("sentry.quotas.backend.assign_seat")
def test_activate_monitor_success(self, assign_seat: MagicMock, check_seat: MagicMock) -> None:
check_seat.return_value = SeatAssignmentResult(assignable=True)
assign_seat.return_value = Outcome.ACCEPTED

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

monitor = Monitor.objects.get(id=monitor.id)
assert monitor.status == ObjectStatus.ACTIVE
assert assign_monitor_seat.called
assert assign_seat.called

@patch("sentry.quotas.backend.check_assign_monitor_seat")
@patch("sentry.quotas.backend.assign_monitor_seat")
@patch("sentry.quotas.backend.check_assign_seat")
@patch("sentry.quotas.backend.assign_seat")
def test_no_activate_if_already_activated(
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
self, assign_seat: MagicMock, check_seat: MagicMock
) -> None:
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
assign_monitor_seat.return_value = Outcome.ACCEPTED
check_seat.return_value = SeatAssignmentResult(assignable=True)
assign_seat.return_value = Outcome.ACCEPTED

monitor = self._create_monitor()

Expand All @@ -820,10 +818,10 @@ def test_no_activate_if_already_activated(

monitor = Monitor.objects.get(id=monitor.id)
assert monitor.status == ObjectStatus.ACTIVE
assert not assign_monitor_seat.called
assert not assign_seat.called

@patch("sentry.quotas.backend.disable_monitor_seat")
def test_no_disable_if_already_disabled(self, disable_monitor_seat: MagicMock) -> None:
@patch("sentry.quotas.backend.disable_seat")
def test_no_disable_if_already_disabled(self, disable_seat: MagicMock) -> None:
monitor = self._create_monitor()

self.get_success_response(
Expand All @@ -832,21 +830,21 @@ def test_no_disable_if_already_disabled(self, disable_monitor_seat: MagicMock) -
monitor.update(status=ObjectStatus.DISABLED)
monitor = Monitor.objects.get(id=monitor.id)
assert monitor.status == ObjectStatus.DISABLED
assert not disable_monitor_seat.called
assert not disable_seat.called

@patch("sentry.quotas.backend.update_monitor_slug")
@patch("sentry.quotas.backend.check_assign_monitor_seat")
@patch("sentry.quotas.backend.assign_monitor_seat")
@patch("sentry.quotas.backend.check_assign_seat")
@patch("sentry.quotas.backend.assign_seat")
def test_update_slug_sends_right_slug_to_assign(
self, assign_monitor_seat, check_assign_monitor_seat, update_monitor_slug
self, assign_seat, check_seat, update_monitor_slug
):
check_assign_monitor_seat.return_value = SeatAssignmentResult(assignable=True)
check_seat.return_value = SeatAssignmentResult(assignable=True)

def dummy_assign(monitor):
def dummy_assign(data_category, monitor):
assert monitor.slug == old_slug
return Outcome.ACCEPTED

assign_monitor_seat.side_effect = dummy_assign
assign_seat.side_effect = dummy_assign

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

@patch("sentry.quotas.backend.check_assign_monitor_seat")
@patch("sentry.quotas.backend.assign_monitor_seat")
def test_activate_monitor_invalid(
self, assign_monitor_seat: MagicMock, check_assign_monitor_seat: MagicMock
) -> None:
@patch("sentry.quotas.backend.check_assign_seat")
@patch("sentry.quotas.backend.assign_seat")
def test_activate_monitor_invalid(self, assign_seat: MagicMock, check_seat: MagicMock) -> None:
result = SeatAssignmentResult(
assignable=False,
reason="Over quota",
)
check_assign_monitor_seat.return_value = result
check_seat.return_value = result

monitor = self._create_monitor()
monitor.update(status=ObjectStatus.DISABLED)
Expand All @@ -887,17 +883,17 @@ def test_activate_monitor_invalid(
)

assert resp.data["status"][0] == result.reason
assert not assign_monitor_seat.called
assert not assign_seat.called

@patch("sentry.quotas.backend.disable_monitor_seat")
def test_deactivate_monitor(self, disable_monitor_seat: MagicMock) -> None:
@patch("sentry.quotas.backend.disable_seat")
def test_deactivate_monitor(self, disable_seat: MagicMock) -> None:
monitor = self._create_monitor()

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

assert disable_monitor_seat.called
assert disable_seat.called


class BaseDeleteMonitorTest(MonitorTestCase):
Expand All @@ -907,10 +903,10 @@ def setUp(self) -> None:
self.login_as(user=self.user)
super().setUp()

@patch("sentry.quotas.backend.disable_monitor_seat")
@patch("sentry.quotas.backend.disable_seat")
@patch("sentry.quotas.backend.update_monitor_slug")
def test_simple(
self, mock_update_monitor_slug: MagicMock, mock_disable_monitor_seat: MagicMock
self, mock_update_monitor_slug: MagicMock, mock_disable_seat: MagicMock
) -> None:
monitor = self._create_monitor()
old_slug = monitor.slug
Expand All @@ -927,7 +923,7 @@ def test_simple(
object_id=monitor.id, model_name="Monitor"
).exists()
mock_update_monitor_slug.assert_called_once()
mock_disable_monitor_seat.assert_called_once_with(monitor=monitor)
mock_disable_seat.assert_called_once_with(DataCategory.MONITOR, monitor)

def test_mismatched_org_slugs(self) -> None:
monitor = self._create_monitor()
Expand Down
Loading
Loading