Skip to content

Commit

Permalink
fix(uptime): Enforce domain limits everywhere
Browse files Browse the repository at this point in the history
Currently we're only enforcing these at the api level. Move them to autodetection as well, and add an index to make sure we can still query them cheaply.
  • Loading branch information
wedamija committed Feb 28, 2025
1 parent 54c5814 commit 53f5065
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 16 deletions.
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ social_auth: 0002_default_auto_field

tempest: 0002_make_message_type_nullable

uptime: 0026_region_mode_col
uptime: 0027_uptime_subscription_index_domain_cols

workflow_engine: 0032_remove_data_source_query_id
20 changes: 17 additions & 3 deletions src/sentry/uptime/detectors/ranking.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

from sentry.constants import UPTIME_AUTODETECTION
from sentry.uptime.models import get_active_auto_monitor_count_for_org
from sentry.uptime.subscriptions.subscriptions import MAX_AUTO_SUBSCRIPTIONS_PER_ORG
from sentry.uptime.subscriptions.subscriptions import (
MAX_AUTO_SUBSCRIPTIONS_PER_ORG,
MaxUrlsForDomainReachedException,
check_url_limits,
)
from sentry.utils import metrics, redis

if TYPE_CHECKING:
Expand Down Expand Up @@ -105,14 +109,24 @@ def delete_candidate_projects_for_org(org: Organization) -> None:
cluster.delete(key)


def get_candidate_urls_for_project(project: Project) -> list[tuple[str, int]]:
def get_candidate_urls_for_project(project: Project, limit=5) -> list[tuple[str, int]]:
"""
Gets all the candidate urls for a project. Returns a tuple of (url, times_url_seen). Urls are sorted by
`times_url_seen` desc.
"""
key = get_project_base_url_rank_key(project)
cluster = _get_cluster()
return cluster.zrange(key, 0, -1, desc=True, withscores=True, score_cast_func=int)
candidate_urls = cluster.zrange(key, 0, -1, desc=True, withscores=True, score_cast_func=int)
urls = []
for candidate_url, url_count in candidate_urls:
try:
check_url_limits(candidate_url)
urls.append((candidate_url, url_count))
except MaxUrlsForDomainReachedException:
pass
if len(urls) == limit:
break
return urls


def delete_candidate_urls_for_project(project: Project) -> None:
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/uptime/detectors/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def process_project_url_ranking(project: Project, project_url_count: int) -> boo

found_url = False

for url, url_count in get_candidate_urls_for_project(project)[:5]:
for url, url_count in get_candidate_urls_for_project(project, limit=5):
if process_candidate_url(project, project_url_count, url, url_count):
found_url = True
break
Expand Down
17 changes: 7 additions & 10 deletions src/sentry/uptime/endpoints/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from sentry.auth.superuser import is_active_superuser
from sentry.constants import ObjectStatus
from sentry.models.environment import Environment
from sentry.uptime.detectors.url_extraction import extract_domain_parts
from sentry.uptime.models import (
ProjectUptimeSubscription,
ProjectUptimeSubscriptionMode,
Expand All @@ -20,13 +19,14 @@
from sentry.uptime.subscriptions.subscriptions import (
MAX_MANUAL_SUBSCRIPTIONS_PER_ORG,
MaxManualUptimeSubscriptionsReached,
MaxUrlsForDomainReachedException,
UptimeMonitorNoSeatAvailable,
check_url_limits,
create_project_uptime_subscription,
update_project_uptime_subscription,
)
from sentry.utils.audit import create_audit_entry

MAX_MONITORS_PER_DOMAIN = 100
"""
The bounding upper limit on how many ProjectUptimeSubscription's can exist for
a single domain + suffix.
Expand Down Expand Up @@ -155,15 +155,12 @@ def validate(self, attrs):
return attrs

def validate_url(self, url):
url_parts = extract_domain_parts(url)
existing_count = ProjectUptimeSubscription.objects.filter(
uptime_subscription__url_domain=url_parts.domain,
uptime_subscription__url_domain_suffix=url_parts.suffix,
).count()

if existing_count >= MAX_MONITORS_PER_DOMAIN:
try:
check_url_limits(url)
except MaxUrlsForDomainReachedException as e:
raise serializers.ValidationError(
f"The domain *.{url_parts.domain}.{url_parts.suffix} has already been used in {MAX_MONITORS_PER_DOMAIN} uptime monitoring alerts, which is the limit. You cannot create any additional alerts for this domain."
f"The domain *.{e.domain}.{e.suffix} has already been used in {e.max_urls} uptime monitoring alerts, "
"which is the limit. You cannot create any additional alerts for this domain."
)
return url

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Generated by Django 5.1.5 on 2025-02-28 00:32

from django.db import migrations, models

from sentry.new_migrations.migrations import CheckedMigration


class Migration(CheckedMigration):
# This flag is used to mark that a migration shouldn't be automatically run in production.
# This should only be used for operations where it's safe to run the migration after your
# code has deployed. So this should not be used for most operations that alter the schema
# of a table.
# Here are some things that make sense to mark as post deployment:
# - Large data migrations. Typically we want these to be run manually so that they can be
# monitored and not block the deploy for a long period of time while they run.
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
# run this outside deployments so that we don't block them. Note that while adding an index
# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False

dependencies = [
("uptime", "0026_region_mode_col"),
]

operations = [
migrations.AddIndex(
model_name="uptimesubscription",
index=models.Index(
fields=["url_domain_suffix", "url_domain"], name="uptime_upti_url_dom_ead522_idx"
),
),
]
1 change: 1 addition & 0 deletions src/sentry/uptime/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class Meta:
name="uptime_uptimesubscription_unique_subscription_check_4",
),
]
indexes = (models.Index(fields=("url_domain_suffix", "url_domain")),)


@region_silo_model
Expand Down
21 changes: 21 additions & 0 deletions src/sentry/uptime/subscriptions/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
UPTIME_SUBSCRIPTION_TYPE = "uptime_monitor"
MAX_AUTO_SUBSCRIPTIONS_PER_ORG = 1
MAX_MANUAL_SUBSCRIPTIONS_PER_ORG = 100
MAX_MONITORS_PER_DOMAIN = 100


class MaxManualUptimeSubscriptionsReached(ValueError):
Expand Down Expand Up @@ -447,3 +448,23 @@ def check_and_update_regions(
)
deleted_region.delete()
return True


class MaxUrlsForDomainReachedException(Exception):
def __init__(self, domain, suffix, max_urls):
self.domain = domain
self.suffix = suffix
self.max_urls = max_urls


def check_url_limits(url):
url_parts = extract_domain_parts(url)
existing_count = ProjectUptimeSubscription.objects.filter(
uptime_subscription__url_domain=url_parts.domain,
uptime_subscription__url_domain_suffix=url_parts.suffix,
).count()

if existing_count >= MAX_MONITORS_PER_DOMAIN:
raise MaxUrlsForDomainReachedException(
url_parts.domain, url_parts.suffix, MAX_MONITORS_PER_DOMAIN
)
14 changes: 14 additions & 0 deletions tests/sentry/uptime/detectors/test_ranking.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ def test(self):
add_base_url_to_rank(self.project, url_2)
assert get_candidate_urls_for_project(self.project) == [(url_2, 2), (url_1, 1)]

def test_limits(self):
with mock.patch("sentry.uptime.subscriptions.subscriptions.MAX_MONITORS_PER_DOMAIN", 1):
other_proj = self.create_project()
url_1 = "https://sentry.io"
self.create_project_uptime_subscription(
project=other_proj, uptime_subscription=self.create_uptime_subscription(url=url_1)
)
url_2 = "https://sentry.sentry.io"
url_3 = "https://sentry.santry.io"
add_base_url_to_rank(self.project, url_1)
add_base_url_to_rank(self.project, url_2)
add_base_url_to_rank(self.project, url_3)
assert get_candidate_urls_for_project(self.project) == [(url_3, 1)]


class DeleteCandidateUrlsForProjectTest(UptimeTestCase):
def test(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def test_not_found(self):
resp = self.get_error_response(self.organization.slug, self.project.slug, 3)
assert resp.status_code == 404

@mock.patch("sentry.uptime.endpoints.validators.MAX_MONITORS_PER_DOMAIN", 1)
@mock.patch("sentry.uptime.subscriptions.subscriptions.MAX_MONITORS_PER_DOMAIN", 1)
def test_domain_limit(self):
# First monitor is for test-one.example.com
self.create_project_uptime_subscription(
Expand Down

0 comments on commit 53f5065

Please sign in to comment.