eps support + fix

src/sentry/snuba/metrics/extraction.py

@@ -124,6 +124,7 @@
     "failure_rate": "on_demand_failure_rate",
     "apdex": "on_demand_apdex",
     "epm": "on_demand_epm",
+    "eps": "on_demand_eps",
 }
 
 # Mapping to infer metric type from Discover function.
@@ -141,6 +142,7 @@
     "failure_rate": "c",
     "apdex": "c",
     "epm": "c",
+    "eps": "c",
 }
 
 # Query fields that on their own do not require on-demand metric extraction but if present in an on-demand query
@@ -630,6 +632,7 @@ def apdex_tag_spec(project: Project, argument: Optional[str]) -> List[TagSpec]:
     "on_demand_failure_rate": failure_tag_spec,
     "on_demand_apdex": apdex_tag_spec,
     "on_demand_epm": None,
+    "on_demand_eps": None,
 }
 
 
@@ -705,7 +708,12 @@ def _field_for_hash(self) -> Optional[str]:
         # with condition `f` and this will create a problem, since we might already have data for the `count()` and when
         # `apdex()` is created in the UI, we will use that metric but that metric didn't extract in the past the tags
         # that are used for apdex calculation, effectively causing problems with the data.
-        if self.op in ["on_demand_epm", "on_demand_failure_count", "on_demand_failure_rate"]:
+        if self.op in [
+            "on_demand_epm",
+            "on_demand_eps",
+            "on_demand_failure_count",
+            "on_demand_failure_rate",
+        ]:
             return self.op
         elif self.op == "on_demand_apdex":
             return f"{self.op}:{self._argument}"

src/sentry/snuba/metrics/fields/base.py

@@ -62,6 +62,7 @@
     miserable_users,
     on_demand_apdex_snql_factory,
     on_demand_epm_snql_factory,
+    on_demand_eps_snql_factory,
     on_demand_failure_count_snql_factory,
     on_demand_failure_rate_snql_factory,
     rate_snql_factory,
@@ -1799,6 +1800,12 @@ def generate_where_statements(
             snql_func=on_demand_epm_snql_factory,
             default_null_value=0,
         ),
+        DerivedOp(
+            op="on_demand_eps",
+            can_orderby=True,
+            snql_func=on_demand_eps_snql_factory,
+            default_null_value=0,
+        ),
         DerivedOp(
             op="on_demand_failure_count",
             can_orderby=True,

src/sentry/snuba/metrics/fields/snql.py

@@ -911,8 +911,16 @@ def on_demand_apdex_snql_factory(
 
 
 def on_demand_epm_snql_factory(
-    aggregate_filter: Function, org_id: int, use_case_id: UseCaseID, alias: Optional[str]
+    aggregate_filter: Function,
+    interval: float,
+    alias: Optional[str],
+) -> Function:
+    return rate_snql_factory(aggregate_filter, interval, 60, alias)
+
+
+def on_demand_eps_snql_factory(
+    aggregate_filter: Function,
+    interval: float,
+    alias: Optional[str],
 ) -> Function:
-    """Return the count based on the aggregation."""
-    # Dividing by the time interval happens in a different place
-    return total_count(aggregate_filter)
+    return rate_snql_factory(aggregate_filter, interval, 1, alias)

src/sentry/snuba/metrics/query_builder.py

@@ -1138,6 +1138,10 @@ def get_snuba_queries(self):
                     params = self._alias_to_metric_field[field[2]].params
                 except KeyError:
                     params = None
+
+                # In order to support on demand metrics which require an interval (e.g. epm),
+                # we want to pass the interval down via params so we can pass it to the associated snql_factory
+                params = {"interval": self._metrics_query.interval, **(params or {})}
                 select += metric_field_obj.generate_select_statements(
                     projects=self._projects,
                     use_case_id=self._use_case_id,

src/sentry/snuba/metrics/utils.py

@@ -98,6 +98,7 @@
     # Custom operations used for on demand derived metrics.
     "on_demand_apdex",
     "on_demand_epm",
+    "on_demand_eps",
     "on_demand_failure_count",
     "on_demand_failure_rate",
 ]
@@ -289,6 +290,7 @@ class MetricMetaWithTagKeys(MetricMeta):
     # Custom operations used for on demand derived metrics.
     "on_demand_apdex",
     "on_demand_epm",
+    "on_demand_eps",
     "on_demand_failure_count",
     "on_demand_failure_rate",
 )

tests/sentry/relay/config/test_metric_extraction.py

@@ -446,7 +446,24 @@ def test_get_metric_extraction_config_with_epm(default_project):
             "condition": {"name": "event.duration", "op": "gte", "value": 1000.0},
             "field": None,
             "mri": "c:transactions/on_demand@none",
-            # XXX: What should the tags be??
+            "tags": [{"key": "query_hash", "value": ANY}],
+        }
+
+
+@django_db_all
+def test_get_metric_extraction_config_with_eps(default_project):
+    with Feature({ON_DEMAND_METRICS: True, ON_DEMAND_METRICS_WIDGETS: True}):
+        create_widget(["eps()"], "transaction.duration:>=1000", default_project)
+
+        config = get_metric_extraction_config(default_project)
+
+        assert config
+        assert len(config["metrics"]) == 1
+        assert config["metrics"][0] == {
+            "category": "transaction",
+            "condition": {"name": "event.duration", "op": "gte", "value": 1000.0},
+            "field": None,
+            "mri": "c:transactions/on_demand@none",
             "tags": [{"key": "query_hash", "value": ANY}],
         }
 

tests/sentry/search/events/builder/test_metrics.py

@@ -2255,8 +2255,8 @@ def test_run_query_with_on_demand_apdex(self):
             ],
         )
 
-    # XXX: This is not correct since we're not yet dividing by the interval
     def test_run_query_with_on_demand_epm(self):
+        """Test events per minute for 1 event within an hour."""
         field = "epm()"
         query = "transaction.duration:>=100"
         spec = OnDemandMetricSpec(field=field, query=query)
@@ -2269,7 +2269,27 @@ def test_run_query_with_on_demand_epm(self):
             tags={"query_hash": spec.query_hash},
             timestamp=timestamp,
         )
-        # XXX: Store a second later and then 61 seconds later
+        query = TimeseriesMetricQueryBuilder(
+            self.params,
+            dataset=Dataset.PerformanceMetrics,
+            interval=3600,
+            query=query,
+            selected_columns=[field],
+            config=QueryBuilderConfig(on_demand_metrics_enabled=True),
+        )
+        result = query.run_query("test_query")
+        assert result["data"][:1] == [{"time": timestamp.isoformat(), "epm": 1 / 60}]
+        assert result["meta"] == [
+            {"name": "time", "type": "DateTime('Universal')"},
+            {"name": "epm", "type": "Float64"},
+        ]
+
+    def test_run_query_with_on_demand_eps(self):
+        """Test event per second for 1 event within an hour."""
+        field = "eps()"
+        query = "transaction.duration:>=100"
+        spec = OnDemandMetricSpec(field=field, query=query)
+        timestamp = self.start
         self.store_transaction_metric(
             value=1,
             metric=TransactionMetricKey.COUNT_ON_DEMAND.value,
@@ -2287,11 +2307,10 @@ def test_run_query_with_on_demand_epm(self):
             config=QueryBuilderConfig(on_demand_metrics_enabled=True),
         )
         result = query.run_query("test_query")
-        # XXX: How is this 0 rather than 2?
-        assert result["data"][:1] == [{"time": timestamp.isoformat(), "epm": 0}]
+        assert result["data"][:1] == [{"time": timestamp.isoformat(), "eps": 1 / 60 / 60}]
         assert result["meta"] == [
             {"name": "time", "type": "DateTime('Universal')"},
-            # {"name": "epm", "type": "Float64"}, ???
+            {"name": "eps", "type": "Float64"},
         ]
 
 

tests/sentry/snuba/test_extraction.py

@@ -343,6 +343,17 @@ def test_spec_epm(default_project):
     assert spec.tags_conditions(default_project) == []
 
 
+@django_db_all
+def test_spec_eps(default_project):
+    spec = OnDemandMetricSpec("eps()", "transaction.duration:>1s")
+
+    assert spec._metric_type == "c"
+    assert spec.field_to_extract is None
+    assert spec.op == "on_demand_eps"
+    assert spec.condition == {"name": "event.duration", "op": "gt", "value": 1000.0}
+    assert spec.tags_conditions(default_project) == []
+
+
 def test_cleanup_equivalent_specs():
     simple_spec = OnDemandMetricSpec("count()", "transaction.duration:>0")
     event_type_spec = OnDemandMetricSpec(