fix(alerts): Skip validation for EAP columns; EXP-301 (#92276)

There's some really strict validation which does not need to happen
for EAP alerts. Skipping column validation, we still validate aggregate.

Author: shruthilayaj

src/sentry/incidents/endpoints/serializers/alert_rule.py

@@ -27,6 +27,7 @@
 from sentry.sentry_apps.models.sentry_app_installation import prepare_ui_component
 from sentry.sentry_apps.services.app import app_service
 from sentry.sentry_apps.services.app.model import RpcSentryAppComponentContext
+from sentry.snuba.dataset import Dataset
 from sentry.snuba.models import SnubaQueryEventType
 from sentry.uptime.models import ProjectUptimeSubscription
 from sentry.users.models.user import User
@@ -262,7 +263,10 @@ def serialize(
         )
         # Temporary: Translate aggregate back here from `tags[sentry:user]` to `user` for the frontend.
         aggregate = translate_aggregate_field(
-            obj.snuba_query.aggregate, reverse=True, allow_mri=allow_mri
+            obj.snuba_query.aggregate,
+            reverse=True,
+            allow_mri=allow_mri,
+            allow_eap=obj.snuba_query.dataset == Dataset.EventsAnalyticsPlatform.value,
         )
 
         data: AlertRuleSerializerResponse = {

src/sentry/incidents/logic.py

@@ -60,7 +60,7 @@
     METRICS_LAYER_UNSUPPORTED_TRANSACTION_METRICS_FUNCTIONS,
     SPANS_METRICS_FUNCTIONS,
 )
-from sentry.search.events.fields import is_function, is_typed_numeric_tag, resolve_field
+from sentry.search.events.fields import is_function, resolve_field
 from sentry.search.events.types import SnubaParams
 from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer
 from sentry.seer.anomaly_detection.store_data import send_new_rule_data, update_rule_data
@@ -1753,6 +1753,7 @@ def get_opsgenie_teams(organization_id: int, integration_id: int) -> list[tuple[
 ]
 EAP_FUNCTIONS = [
     "count",
+    "count_unique",
     "avg",
     "p50",
     "p75",
@@ -1766,7 +1767,9 @@ def get_opsgenie_teams(organization_id: int, integration_id: int) -> list[tuple[
 ]
 
 
-def get_column_from_aggregate(aggregate: str, allow_mri: bool) -> str | None:
+def get_column_from_aggregate(
+    aggregate: str, allow_mri: bool, allow_eap: bool = False
+) -> str | None:
     # These functions exist as SnQLFunction definitions and are not supported in the older
     # logic for resolving functions. We parse these using `fields.is_function`, otherwise
     # they will fail using the old resolve_field logic.
@@ -1778,7 +1781,7 @@ def get_column_from_aggregate(aggregate: str, allow_mri: bool) -> str | None:
         return None if match.group("columns") == "" else match.group("columns")
 
     # Skip additional validation for EAP queries. They don't exist in the old logic.
-    if match and match.group("function") in EAP_FUNCTIONS and match.group("columns") in EAP_COLUMNS:
+    if match and match.group("function") in EAP_FUNCTIONS and allow_eap:
         return match.group("columns")
 
     if allow_mri:
@@ -1817,7 +1820,7 @@ def check_aggregate_column_support(
     aggregate: str, allow_mri: bool = False, allow_eap: bool = False
 ) -> bool:
     # TODO(ddm): remove `allow_mri` once the experimental feature flag is removed.
-    column = get_column_from_aggregate(aggregate, allow_mri)
+    column = get_column_from_aggregate(aggregate, allow_mri, allow_eap)
     match = is_function(aggregate)
     function = match.group("function") if match else None
     return (
@@ -1830,15 +1833,14 @@ def check_aggregate_column_support(
             isinstance(function, str)
             and column in INSIGHTS_FUNCTION_VALID_ARGS_MAP.get(function, [])
         )
-        or (column in EAP_COLUMNS and allow_eap)
-        or (is_typed_numeric_tag(column) and allow_eap)
+        or allow_eap
     )
 
 
 def translate_aggregate_field(
-    aggregate: str, reverse: bool = False, allow_mri: bool = False
+    aggregate: str, reverse: bool = False, allow_mri: bool = False, allow_eap: bool = False
 ) -> str:
-    column = get_column_from_aggregate(aggregate, allow_mri)
+    column = get_column_from_aggregate(aggregate, allow_mri, allow_eap)
     if not reverse:
         if column in TRANSLATABLE_COLUMNS:
             return aggregate.replace(column, TRANSLATABLE_COLUMNS[column])

src/sentry/snuba/snuba_query_validator.py

@@ -3,6 +3,7 @@
 from datetime import timedelta
 
 from django.utils import timezone
+from django.utils.translation import gettext_lazy as _
 from rest_framework import serializers
 from snuba_sdk import Column, Condition, Entity, Limit, Op
 
@@ -116,35 +117,6 @@ def validate_query(self, query: str):
                 )
         return query
 
-    def validate_aggregate(self, value: str):
-        allow_mri = features.has(
-            "organizations:custom-metrics",
-            self.context["organization"],
-            actor=self.context.get("user", None),
-        ) or features.has(
-            "organizations:insights-alerts",
-            self.context["organization"],
-            actor=self.context.get("user", None),
-        )
-        allow_eap = features.has(
-            "organizations:visibility-explore-view",
-            self.context["organization"],
-            actor=self.context.get("user", None),
-        )
-        try:
-            if not check_aggregate_column_support(
-                value,
-                allow_mri=allow_mri,
-                allow_eap=allow_eap,
-            ):
-                raise serializers.ValidationError(
-                    "Invalid Metric: We do not currently support this field."
-                )
-        except InvalidSearchQuery as e:
-            raise serializers.ValidationError(f"Invalid Metric: {e}")
-
-        return translate_aggregate_field(value, allow_mri=allow_mri)
-
     def validate_event_types(self, value: Sequence[str]) -> list[SnubaQueryEventType.EventType]:
         try:
             return [SnubaQueryEventType.EventType[event_type.upper()] for event_type in value]
@@ -156,6 +128,7 @@ def validate_event_types(self, value: Sequence[str]) -> list[SnubaQueryEventType
 
     def validate(self, data):
         data = super().validate(data)
+        self._validate_aggregate(data)
         self._validate_query(data)
 
         query_type = data["query_type"]
@@ -172,6 +145,35 @@ def validate(self, data):
 
         return data
 
+    def _validate_aggregate(self, data):
+        dataset = data.setdefault("dataset", Dataset.Events)
+        aggregate = data.get("aggregate")
+        allow_mri = features.has(
+            "organizations:custom-metrics",
+            self.context["organization"],
+            actor=self.context.get("user", None),
+        ) or features.has(
+            "organizations:insights-alerts",
+            self.context["organization"],
+            actor=self.context.get("user", None),
+        )
+        allow_eap = dataset == Dataset.EventsAnalyticsPlatform
+        try:
+            if not check_aggregate_column_support(
+                aggregate,
+                allow_mri=allow_mri,
+                allow_eap=allow_eap,
+            ):
+                raise serializers.ValidationError(
+                    {"aggregate": _("Invalid Metric: We do not currently support this field.")}
+                )
+        except InvalidSearchQuery as e:
+            raise serializers.ValidationError({"aggregate": _(f"Invalid Metric: {e}")})
+
+        data["aggregate"] = translate_aggregate_field(
+            aggregate, allow_mri=allow_mri, allow_eap=allow_eap
+        )
+
     def _validate_query(self, data):
         dataset = data.setdefault("dataset", Dataset.Events)
 
@@ -184,7 +186,11 @@ def _validate_query(self, data):
             self.context["organization"],
             actor=self.context.get("user", None),
         ):
-            column = get_column_from_aggregate(data["aggregate"], allow_mri=True)
+            column = get_column_from_aggregate(
+                data["aggregate"],
+                allow_mri=True,
+                allow_eap=dataset == Dataset.EventsAnalyticsPlatform,
+            )
             if is_mri(column) and dataset != Dataset.PerformanceMetrics:
                 raise serializers.ValidationError(
                     "You can use an MRI only on alerts on performance metrics"

tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py

@@ -408,6 +408,24 @@ def test_anomaly_detection_alert_eap(self, mock_seer_request):
         assert alert_rule.sensitivity == resp.data.get("sensitivity")
         assert mock_seer_request.call_count == 1
 
+    def test_create_alert_rule_eap(self):
+        data = deepcopy(self.alert_rule_dict)
+        data["dataset"] = "events_analytics_platform"
+        data["alertType"] = "eap_metrics"
+        data["aggregate"] = "count_unique(org)"
+        with (
+            outbox_runner(),
+            self.feature(["organizations:incidents", "organizations:performance-view"]),
+        ):
+            resp = self.get_success_response(
+                self.organization.slug,
+                status_code=201,
+                **data,
+            )
+        assert "id" in resp.data
+        alert_rule = AlertRule.objects.get(id=resp.data["id"])
+        assert resp.data == serialize(alert_rule, self.user)
+
     @with_feature("organizations:anomaly-detection-alerts")
     @with_feature("organizations:anomaly-detection-rollout")
     @with_feature("organizations:incidents")