diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index f2833d0d54b316..2f283e5c9f02f4 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -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 diff --git a/src/sentry/uptime/detectors/ranking.py b/src/sentry/uptime/detectors/ranking.py index 7fa8cf1b443c96..7a1fc3bb100d51 100644 --- a/src/sentry/uptime/detectors/ranking.py +++ b/src/sentry/uptime/detectors/ranking.py @@ -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: @@ -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: diff --git a/src/sentry/uptime/detectors/tasks.py b/src/sentry/uptime/detectors/tasks.py index 69bc19e79cb047..1fb7b4b64d4835 100644 --- a/src/sentry/uptime/detectors/tasks.py +++ b/src/sentry/uptime/detectors/tasks.py @@ -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 diff --git a/src/sentry/uptime/endpoints/validators.py b/src/sentry/uptime/endpoints/validators.py index 808a4f11406284..4f7e5571c5d4ed 100644 --- a/src/sentry/uptime/endpoints/validators.py +++ b/src/sentry/uptime/endpoints/validators.py @@ -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, @@ -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. @@ -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 diff --git a/src/sentry/uptime/migrations/0027_uptime_subscription_index_domain_cols.py b/src/sentry/uptime/migrations/0027_uptime_subscription_index_domain_cols.py new file mode 100644 index 00000000000000..c756092c953c41 --- /dev/null +++ b/src/sentry/uptime/migrations/0027_uptime_subscription_index_domain_cols.py @@ -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" + ), + ), + ] diff --git a/src/sentry/uptime/models.py b/src/sentry/uptime/models.py index 57c33e6bb9d5c7..7cc0868f27da52 100644 --- a/src/sentry/uptime/models.py +++ b/src/sentry/uptime/models.py @@ -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 diff --git a/src/sentry/uptime/subscriptions/subscriptions.py b/src/sentry/uptime/subscriptions/subscriptions.py index 98e3e7476c85df..98936e45305c76 100644 --- a/src/sentry/uptime/subscriptions/subscriptions.py +++ b/src/sentry/uptime/subscriptions/subscriptions.py @@ -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): @@ -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 + ) diff --git a/tests/sentry/uptime/detectors/test_ranking.py b/tests/sentry/uptime/detectors/test_ranking.py index db19837c29e18d..43e37cd3b85f63 100644 --- a/tests/sentry/uptime/detectors/test_ranking.py +++ b/tests/sentry/uptime/detectors/test_ranking.py @@ -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): diff --git a/tests/sentry/uptime/endpoints/test_project_uptime_alert_details.py b/tests/sentry/uptime/endpoints/test_project_uptime_alert_details.py index 8ee0165df0fee4..9530cdeec6ab6c 100644 --- a/tests/sentry/uptime/endpoints/test_project_uptime_alert_details.py +++ b/tests/sentry/uptime/endpoints/test_project_uptime_alert_details.py @@ -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(