Repository: getsentry/sentry
Pull request: ref(rules): Bucket max rules into slow and fast
Allow for 100 "slow" rules (these are more expensive to process) and 200 "fast" issue alert rules per project.

Closes #51422 

Problem statement:
Ability to exceed 100 alert issues per project
### Problem Statement

For multiple teams sharing a project, the total number of alerts per project can easily exceed the 100 alert limit.  


### Solution Brainstorm

Differentiating and having a higher limit for the rules that are not as resource intensive, for example alert rules that query snuba can be expensive



### Product Area

Alerts

Hints from issue discussion:
Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2023-06-22T23:56:28.695Z>Thursday, June 22nd at 4:56 pm</time> (sfo)**. ⏲️
Routing to @getsentry/product-owners-alerts for [triage](https://develop.sentry.dev/processing-tickets/#3-triage), due by **<time datetime=2023-06-24T00:00:00.000Z>Friday, June 23rd at 5:00 pm</time> (sfo)**. ⏲️
The reason we have this limit is to prevent overuse of expensive rules that query snuba. If we want to raise it, then we can split rules into two buckets, one for rules with expensive snuba queries and one without. We can then have separate limits, so we'd allow up to 100 expensive queries, and 200 inexpensive for 300 total.

We already differentiate between cheap/expensive queries here: https://github.com/getsentry/sentry/blob/014c409e6b2eb5e8d9d4b2c0e0ba7051f7300929/src/sentry/rules/processor.py#L190-L194 This is used to prioritise processing the faster conditions first so we can exit early and skip the more expensive conditions if they're unnecessary. We just need to extract this logic into a separate function and use it to bucket rules here: https://github.com/getsentry/sentry/blob/014c409e6b2eb5e8d9d4b2c0e0ba7051f7300929/src/sentry/api/endpoints/project_rules.py#L64-L71

Commit message: ref(rules): Bucket max rules into slow and fast

Patch to review:
diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py
index f472bf44667c45..8fde1df6763afe 100644
--- a/src/sentry/api/endpoints/project_rules.py
+++ b/src/sentry/api/endpoints/project_rules.py
@@ -12,6 +12,7 @@
 from sentry.mediators import project_rules
 from sentry.models import Rule, RuleActivity, RuleActivityType, RuleStatus, Team, User
 from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues
+from sentry.rules.processor import SLOW_CONDITION_MATCHES
 from sentry.signals import alert_rule_created
 from sentry.tasks.integrations.slack import find_channel_id_for_rule
 from sentry.web.decorators import transaction_start
@@ -61,83 +62,94 @@ def post(self, request: Request, project) -> Response:
             }}
 
         """
-        if (
-            Rule.objects.filter(project=project, status=RuleStatus.ACTIVE).count()
-            >= settings.MAX_ISSUE_ALERTS_PER_PROJECT
-        ):
-            return Response(
-                f"You may not exceed {settings.MAX_ISSUE_ALERTS_PER_PROJECT} rules per project",
-                status=status.HTTP_400_BAD_REQUEST,
-            )
 
         serializer = RuleSerializer(
             context={"project": project, "organization": project.organization}, data=request.data
         )
-
-        if serializer.is_valid():
-            data = serializer.validated_data
-            # combine filters and conditions into one conditions criteria for the rule object
-            conditions = data.get("conditions", [])
-            if "filters" in data:
-                conditions.extend(data["filters"])
-
-            kwargs = {
-                "name": data["name"],
-                "environment": data.get("environment"),
-                "project": project,
-                "action_match": data["actionMatch"],
-                "filter_match": data.get("filterMatch"),
-                "conditions": conditions,
-                "actions": data.get("actions", []),
-                "frequency": data.get("frequency"),
-                "user_id": request.user.id,
-            }
-            owner = data.get("owner")
-            if owner:
-                try:
-                    kwargs["owner"] = owner.resolve_to_actor().id
-                except (User.DoesNotExist, Team.DoesNotExist):
-                    return Response(
-                        "Could not resolve owner",
-                        status=status.HTTP_400_BAD_REQUEST,
-                    )
-
-            if data.get("pending_save"):
-                client = RedisRuleStatus()
-                uuid_context = {"uuid": client.uuid}
-                kwargs.update(uuid_context)
-                find_channel_id_for_rule.apply_async(kwargs=kwargs)
-                return Response(uuid_context, status=202)
-
-            created_alert_rule_ui_component = trigger_sentry_app_action_creators_for_issues(
-                kwargs.get("actions")
-            )
-            rule = project_rules.Creator.run(request=request, **kwargs)
-            RuleActivity.objects.create(
-                rule=rule, user_id=request.user.id, type=RuleActivityType.CREATED.value
-            )
-            duplicate_rule = request.query_params.get("duplicateRule")
-            wizard_v3 = request.query_params.get("wizardV3")
-
-            self.create_audit_entry(
-                request=request,
-                organization=project.organization,
-                target_object=rule.id,
-                event=audit_log.get_event_id("RULE_ADD"),
-                data=rule.get_audit_log_data(),
+        if not serializer.is_valid():
+            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+
+        data = serializer.validated_data
+        # combine filters and conditions into one conditions criteria for the rule object
+        conditions = data.get("conditions", [])
+        if "filters" in data:
+            conditions.extend(data["filters"])
+
+        rules = Rule.objects.filter(project=project, status=RuleStatus.ACTIVE)
+        slow_rules = 0
+        for rule in rules:
+            for condition in rule.data["conditions"]:
+                for slow_conditions in SLOW_CONDITION_MATCHES:
+                    if slow_conditions in condition["id"]:
+                        slow_rules += 1
+                    break
+
+        if slow_rules >= settings.MAX_SLOW_CONDITION_ALERTS:
+            return Response(
+                f"You may not exceed {settings.MAX_SLOW_CONDITION_ALERTS} 'slow' rules per project.",
+                status=status.HTTP_400_BAD_REQUEST,
             )
-            alert_rule_created.send_robust(
-                user=request.user,
-                project=project,
-                rule=rule,
-                rule_type="issue",
-                sender=self,
-                is_api_token=request.auth is not None,
-                alert_rule_ui_component=created_alert_rule_ui_component,
-                duplicate_rule=duplicate_rule,
-                wizard_v3=wizard_v3,
+        if (len(rules) - slow_rules) >= settings.MAX_FAST_CONDITION_ALERTS:
+            return Response(
+                f"You may not exceed {settings.MAX_FAST_CONDITION_ALERTS} 'fast' rules per project.",
+                status=status.HTTP_400_BAD_REQUEST,
             )
 
-            return Response(serialize(rule, request.user))
+        kwargs = {
+            "name": data["name"],
+            "environment": data.get("environment"),
+            "project": project,
+            "action_match": data["actionMatch"],
+            "filter_match": data.get("filterMatch"),
+            "conditions": conditions,
+            "actions": data.get("actions", []),
+            "frequency": data.get("frequency"),
+            "user_id": request.user.id,
+        }
+        owner = data.get("owner")
+        if owner:
+            try:
+                kwargs["owner"] = owner.resolve_to_actor().id
+            except (User.DoesNotExist, Team.DoesNotExist):
+                return Response(
+                    "Could not resolve owner",
+                    status=status.HTTP_400_BAD_REQUEST,
+                )
+
+        if data.get("pending_save"):
+            client = RedisRuleStatus()
+            uuid_context = {"uuid": client.uuid}
+            kwargs.update(uuid_context)
+            find_channel_id_for_rule.apply_async(kwargs=kwargs)
+            return Response(uuid_context, status=202)
+
+        created_alert_rule_ui_component = trigger_sentry_app_action_creators_for_issues(
+            kwargs.get("actions")
+        )
+        rule = project_rules.Creator.run(request=request, **kwargs)
+        RuleActivity.objects.create(
+            rule=rule, user_id=request.user.id, type=RuleActivityType.CREATED.value
+        )
+        duplicate_rule = request.query_params.get("duplicateRule")
+        wizard_v3 = request.query_params.get("wizardV3")
+
+        self.create_audit_entry(
+            request=request,
+            organization=project.organization,
+            target_object=rule.id,
+            event=audit_log.get_event_id("RULE_ADD"),
+            data=rule.get_audit_log_data(),
+        )
+        alert_rule_created.send_robust(
+            user=request.user,
+            project=project,
+            rule=rule,
+            rule_type="issue",
+            sender=self,
+            is_api_token=request.auth is not None,
+            alert_rule_ui_component=created_alert_rule_ui_component,
+            duplicate_rule=duplicate_rule,
+            wizard_v3=wizard_v3,
+        )
 
-        return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+        return Response(serialize(rule, request.user))
diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py
index 2243a1ba6d5c44..ddc3c6fbc8c675 100644
--- a/src/sentry/conf/server.py
+++ b/src/sentry/conf/server.py
@@ -3374,7 +3374,8 @@ def build_cdc_postgres_init_db_volume(settings: Any) -> dict[str, dict[str, str]
 # determines if we enable analytics or not
 ENABLE_ANALYTICS = False
 
-MAX_ISSUE_ALERTS_PER_PROJECT = 100
+MAX_SLOW_CONDITION_ALERTS = 100
+MAX_FAST_CONDITION_ALERTS = 200
 MAX_QUERY_SUBSCRIPTIONS_PER_ORG = 1000
 
 MAX_REDIS_SNOWFLAKE_RETRY_COUNTER = 5
diff --git a/tests/sentry/api/endpoints/test_project_rules.py b/tests/sentry/api/endpoints/test_project_rules.py
index 2298543f521db4..59e233cb787dd7 100644
--- a/tests/sentry/api/endpoints/test_project_rules.py
+++ b/tests/sentry/api/endpoints/test_project_rules.py
@@ -189,8 +189,8 @@ def test_missing_name(self):
             status_code=status.HTTP_400_BAD_REQUEST,
         )
 
-    @override_settings(MAX_ISSUE_ALERTS_PER_PROJECT=1)
-    def test_exceed_limit(self):
+    @override_settings(MAX_FAST_CONDITION_ALERTS=1)
+    def test_exceed_limit_fast_conditions(self):
         conditions = [{"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"}]
         actions = [{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}]
         Rule.objects.filter(project=self.project).delete()
@@ -207,7 +207,37 @@ def test_exceed_limit(self):
             conditions=conditions,
             status_code=status.HTTP_400_BAD_REQUEST,
         )
-        assert resp.data == "You may not exceed 1 rules per project"
+        assert resp.data == "You may not exceed 1 'fast' rules per project."
+        # Make sure pending deletions don't affect the process
+        Rule.objects.filter(project=self.project).update(status=RuleStatus.PENDING_DELETION)
+        self.run_test(conditions=conditions, actions=actions)
+
+    @override_settings(MAX_SLOW_CONDITION_ALERTS=1)
+    def test_exceed_limit_slow_conditions(self):
+        conditions = [
+            {
+                "id": "sentry.rules.conditions.event_frequency.EventFrequencyPercentCondition",
+                "interval": "1h",
+                "value": 100,
+                "comparisonType": "count",
+            }
+        ]
+        actions = [{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}]
+        Rule.objects.filter(project=self.project).delete()
+        self.run_test(conditions=conditions, actions=actions)
+        resp = self.get_error_response(
+            self.organization.slug,
+            self.project.slug,
+            name="test",
+            frequency=30,
+            owner=self.user.get_actor_identifier(),
+            actionMatch="any",
+            filterMatch="any",
+            actions=actions,
+            conditions=conditions,
+            status_code=status.HTTP_400_BAD_REQUEST,
+        )
+        assert resp.data == "You may not exceed 1 'slow' rules per project."
         # Make sure pending deletions don't affect the process
         Rule.objects.filter(project=self.project).update(status=RuleStatus.PENDING_DELETION)
         self.run_test(conditions=conditions, actions=actions)


Code review comments:
diff_hunk: @@ -3374,7 +3374,8 @@ def build_cdc_postgres_init_db_volume(settings: Any) -> dict[str, dict[str, str]
 # determines if we enable analytics or not
 ENABLE_ANALYTICS = False
 
-MAX_ISSUE_ALERTS_PER_PROJECT = 100
+MAX_SLOW_CONDITION_ALERTS = 100
+MAX_FAST_CONDITION_ALERTS = 200
path: src/sentry/conf/server.py
text: @user1:
`*_ISSUE_ALERTS` just to make sure we differentiate from metric alerts

diff_hunk: @@ -61,83 +62,94 @@ def post(self, request: Request, project) -> Response:
             }}
 
         """
-        if (
-            Rule.objects.filter(project=project, status=RuleStatus.ACTIVE).count()
-            >= settings.MAX_ISSUE_ALERTS_PER_PROJECT
-        ):
-            return Response(
-                f"You may not exceed {settings.MAX_ISSUE_ALERTS_PER_PROJECT} rules per project",
-                status=status.HTTP_400_BAD_REQUEST,
-            )
 
         serializer = RuleSerializer(
             context={"project": project, "organization": project.organization}, data=request.data
         )
-
-        if serializer.is_valid():
-            data = serializer.validated_data
-            # combine filters and conditions into one conditions criteria for the rule object
-            conditions = data.get("conditions", [])
-            if "filters" in data:
-                conditions.extend(data["filters"])
-
-            kwargs = {
-                "name": data["name"],
-                "environment": data.get("environment"),
-                "project": project,
-                "action_match": data["actionMatch"],
-                "filter_match": data.get("filterMatch"),
-                "conditions": conditions,
-                "actions": data.get("actions", []),
-                "frequency": data.get("frequency"),
-                "user_id": request.user.id,
-            }
-            owner = data.get("owner")
-            if owner:
-                try:
-                    kwargs["owner"] = owner.resolve_to_actor().id
-                except (User.DoesNotExist, Team.DoesNotExist):
-                    return Response(
-                        "Could not resolve owner",
-                        status=status.HTTP_400_BAD_REQUEST,
-                    )
-
-            if data.get("pending_save"):
-                client = RedisRuleStatus()
-                uuid_context = {"uuid": client.uuid}
-                kwargs.update(uuid_context)
-                find_channel_id_for_rule.apply_async(kwargs=kwargs)
-                return Response(uuid_context, status=202)
-
-            created_alert_rule_ui_component = trigger_sentry_app_action_creators_for_issues(
-                kwargs.get("actions")
-            )
-            rule = project_rules.Creator.run(request=request, **kwargs)
-            RuleActivity.objects.create(
-                rule=rule, user_id=request.user.id, type=RuleActivityType.CREATED.value
-            )
-            duplicate_rule = request.query_params.get("duplicateRule")
-            wizard_v3 = request.query_params.get("wizardV3")
-
-            self.create_audit_entry(
-                request=request,
-                organization=project.organization,
-                target_object=rule.id,
-                event=audit_log.get_event_id("RULE_ADD"),
-                data=rule.get_audit_log_data(),
+        if not serializer.is_valid():
+            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+
+        data = serializer.validated_data
+        # combine filters and conditions into one conditions criteria for the rule object
+        conditions = data.get("conditions", [])
+        if "filters" in data:
+            conditions.extend(data["filters"])
+
+        rules = Rule.objects.filter(project=project, status=RuleStatus.ACTIVE)
+        slow_rules = 0
+        for rule in rules:
+            for condition in rule.data["conditions"]:
+                for slow_conditions in SLOW_CONDITION_MATCHES:
+                    if slow_conditions in condition["id"]:
path: src/sentry/api/endpoints/project_rules.py
text: @user1:
Might be worth breaking this out into a `is_condition_slow` function that we can share with the rule processor