Improve

src/sentry/incidents/serializers/alert_rule.py

@@ -6,7 +6,7 @@
 from django.db import router, transaction
 from django.utils import timezone
 from rest_framework import serializers
-from snuba_sdk import Function, Limit
+from snuba_sdk import Column, Condition, Function, Limit, Op
 
 from sentry import features
 from sentry.api.fields.actor import ActorField
@@ -27,7 +27,11 @@
 )
 from sentry.incidents.models import AlertRule, AlertRuleThresholdType, AlertRuleTrigger
 from sentry.snuba.dataset import Dataset
-from sentry.snuba.entity_subscription import get_entity_subscription
+from sentry.snuba.entity_subscription import (
+    ENTITY_TIME_COLUMNS,
+    get_entity_key_from_query_builder,
+    get_entity_subscription,
+)
 from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
 from sentry.snuba.tasks import build_query_builder
 
@@ -283,7 +287,6 @@ def _validate_query(self, data):
     def _validate_snql_query(self, data, entity_subscription, projects):
         end = timezone.now()
         start = end - timedelta(minutes=10)
-
         try:
             query_builder = build_query_builder(
                 entity_subscription,
@@ -297,7 +300,6 @@ def _validate_snql_query(self, data, entity_subscription, projects):
                     "end": end,
                 },
             )
-            query_builder.limit = Limit(1)
         except (InvalidSearchQuery, ValueError) as e:
             raise serializers.ValidationError(f"Invalid Query or Metric: {e}")
 
@@ -309,6 +311,15 @@ def _validate_snql_query(self, data, entity_subscription, projects):
         dataset = Dataset(data["dataset"].value)
         self._validate_time_window(dataset, data.get("time_window"))
 
+        time_col = ENTITY_TIME_COLUMNS[get_entity_key_from_query_builder(query_builder)]
+        query_builder.add_conditions(
+            [
+                Condition(Column(time_col), Op.GTE, start),
+                Condition(Column(time_col), Op.LT, end),
+            ]
+        )
+        query_builder.limit = Limit(1)
+
         try:
             query_builder.run_query(referrer="alertruleserializer.test_query")
         except Exception:

src/sentry/search/events/builder/metrics.py

@@ -120,7 +120,9 @@ def _on_demand_metric_spec(self) -> Optional[OndemandMetricSpec]:
             sentry_sdk.capture_exception(e)
             return None
 
-    def _get_metrics_query_from_on_demand_spec(self, spec: OndemandMetricSpec) -> MetricsQuery:
+    def _get_metrics_query_from_on_demand_spec(
+        self, spec: OndemandMetricSpec, require_time_range: bool = True
+    ) -> MetricsQuery:
         if self.params.organization is None:
             raise InvalidSearchQuery("An on demand metrics query requires an organization")
 
@@ -133,6 +135,20 @@ def _get_metrics_query_from_on_demand_spec(self, spec: OndemandMetricSpec) -> Me
             alias = spec.mri
             include_series = False
 
+        # Since the query builder is very convoluted, we first try to get the start and end from the validated
+        # parameters but in case it's none it can be that the `skip_time_conditions` was True, thus in that case we
+        # try to see if start and end were supplied directly in the constructor.
+        start = self.start or self.params.start
+        end = self.end or self.params.end
+
+        # The time range can be required or not, since the query generated by the builder can either be used to execute
+        # the query on its own (requiring a time range) or it can be used to get the snql code necessary to create a
+        # query subscription from the outside.
+        if require_time_range and (start is None or end is None):
+            raise InvalidSearchQuery(
+                "The on demand metric query requires a time range to be executed"
+            )
+
         return MetricsQuery(
             select=[MetricField(spec.op, spec.mri, alias=alias)],
             where=[
@@ -149,8 +165,8 @@ def _get_metrics_query_from_on_demand_spec(self, spec: OndemandMetricSpec) -> Me
             org_id=self.params.organization.id,
             project_ids=[p.id for p in self.params.projects],
             include_series=include_series,
-            start=self.start,
-            end=self.end,
+            start=start,
+            end=end,
         )
 
     def validate_aggregate_arguments(self) -> None:
@@ -813,7 +829,7 @@ def run_query(self, referrer: str, use_cache: bool = False) -> Any:
                 with sentry_sdk.start_span(op="metric_layer", description="transform_query"):
                     if self._on_demand_metric_spec:
                         metrics_query = self._get_metrics_query_from_on_demand_spec(
-                            self._on_demand_metric_spec
+                            spec=self._on_demand_metric_spec, require_time_range=True
                         )
                     else:
                         intermediate_query = self.get_metrics_layer_snql_query()
@@ -1013,7 +1029,7 @@ def get_snql_query(self) -> Request:
 
             if self._on_demand_metric_spec:
                 metrics_query = self._get_metrics_query_from_on_demand_spec(
-                    self._on_demand_metric_spec
+                    spec=self._on_demand_metric_spec, require_time_range=False
                 )
             else:
                 intermediate_query = self.get_metrics_layer_snql_query()
@@ -1253,7 +1269,7 @@ def run_query(self, referrer: str, use_cache: bool = False) -> Any:
                 with sentry_sdk.start_span(op="metric_layer", description="transform_query"):
                     if self._on_demand_metric_spec:
                         metrics_query = self._get_metrics_query_from_on_demand_spec(
-                            self._on_demand_metric_spec
+                            spec=self._on_demand_metric_spec, require_time_range=True
                         )
                     elif self.use_metrics_layer:
                         snuba_query = self.get_snql_query()[0].query

src/sentry/snuba/entity_subscription.py

@@ -191,7 +191,7 @@ def build_query_builder(
             params=params,
             offset=None,
             limit=None,
-            skip_time_conditions=False,
+            skip_time_conditions=True,
             parser_config_overrides={"blocked_keys": ALERT_BLOCKED_FIELDS},
         )
 
@@ -284,7 +284,7 @@ def build_query_builder(
             offset=None,
             limit=None,
             functions_acl=["identity"],
-            skip_time_conditions=False,
+            skip_time_conditions=True,
             parser_config_overrides={"blocked_keys": ALERT_BLOCKED_FIELDS},
         )
 
@@ -379,7 +379,7 @@ def build_query_builder(
             selected_columns=self.get_snql_aggregations(),
             params=params,
             offset=None,
-            skip_time_conditions=False,
+            skip_time_conditions=True,
             granularity=self.get_granularity(),
             use_metrics_layer=self.use_metrics_layer,
             on_demand_metrics_enabled=self.on_demand_metrics_enabled,