feat(tracemetrics): Enforce metrics query params behind feature flag (#102683)

Zylphrex · web-flow · commit 47c69d19fd38 · 2025-11-04T18:29:05.000Z
diff --git a/src/sentry/api/endpoints/organization_events.py b/src/sentry/api/endpoints/organization_events.py
@@ -31,7 +31,7 @@
 from sentry.ratelimits.config import RateLimitConfig
 from sentry.search.eap.trace_metrics.config import (
     TraceMetricsSearchResolverConfig,
-    get_trace_metric_info,
+    get_trace_metric_from_request,
 )
 from sentry.search.eap.types import AdditionalQueries, FieldsACL, SearchResolverConfig
 from sentry.snuba import (
@@ -531,7 +531,7 @@ def get_rpc_config():
                     )
                 elif scoped_dataset == TraceMetrics:
                     # tracemetrics uses aggregate conditions
-                    metric_name, metric_type = get_trace_metric_info(request)
+                    metric_name, metric_type = get_trace_metric_from_request(request, organization)
 
                     return TraceMetricsSearchResolverConfig(
                         metric_name=metric_name,
diff --git a/src/sentry/api/endpoints/organization_events_stats.py b/src/sentry/api/endpoints/organization_events_stats.py
@@ -21,7 +21,7 @@
 from sentry.models.organization import Organization
 from sentry.search.eap.trace_metrics.config import (
     TraceMetricsSearchResolverConfig,
-    get_trace_metric_info,
+    get_trace_metric_from_request,
 )
 from sentry.search.eap.types import SearchResolverConfig
 from sentry.search.events.types import SnubaParams
@@ -243,7 +243,7 @@ def get_rpc_config():
 
                 if scoped_dataset == TraceMetrics:
                     # tracemetrics uses aggregate conditions
-                    metric_name, metric_type = get_trace_metric_info(request)
+                    metric_name, metric_type = get_trace_metric_from_request(request, organization)
 
                     return TraceMetricsSearchResolverConfig(
                         metric_name=metric_name,
diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py
@@ -590,6 +590,9 @@ def register_temporary_features(manager: FeatureManager) -> None:
     manager.add("organizations:tracemetrics-stats", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
     # Enable trace metrics in trace view UI
     manager.add("organizations:tracemetrics-traceview-ui", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
+    # Enable trace metrics top level params
+    manager.add("organizations:tracemetrics-top-level-params", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
+
     # Enable feature flag to send expanded sentry apps webhooks (issue.created for all group categories)
     manager.add("organizations:expanded-sentry-apps-webhooks", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
     # Enable using paginated projects endpoint for Jira integration
diff --git a/src/sentry/search/eap/trace_metrics/config.py b/src/sentry/search/eap/trace_metrics/config.py
@@ -1,6 +1,7 @@
 from dataclasses import dataclass
 from typing import Literal, cast
 
+from rest_framework.exceptions import ErrorDetail, ValidationError
 from rest_framework.request import Request
 from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue
 from sentry_protos.snuba.v1.trace_item_filter_pb2 import (
@@ -9,6 +10,8 @@
     TraceItemFilter,
 )
 
+from sentry import features
+from sentry.models.organization import Organization
 from sentry.search.eap.resolver import SearchResolver
 from sentry.search.eap.types import SearchResolverConfig
 
@@ -55,33 +58,36 @@ def extra_conditions(self, search_resolver: SearchResolver) -> TraceItemFilter |
 ALLOWED_METRIC_TYPES: list[MetricType] = ["counter", "gauge", "distribution"]
 
 
-def get_trace_metric_info(
+def get_trace_metric_from_request(
     request: Request,
+    organization: Organization,
 ) -> tuple[str, MetricType]:
     metric_name = request.GET.get("metricName")
     metric_type = request.GET.get("metricType")
 
-    # TODO: remove this when these args are not optional anymore
-    if not metric_name:
-        metric_name = ""
-    if not metric_type:
-        metric_type = ""
-
-    # errors = {}
-
-    # if not metric_name:
-    #     errors["metricName"] = ErrorDetail("This field is required.", code="required")
-
-    # if not metric_type:
-    #     errors["metricType"] = ErrorDetail("This field is required.", code="required")
-    # elif metric_type not in ALLOWED_METRIC_TYPES:
-    #     errors["metricType"] = ErrorDetail(
-    #         string=f'"{metric_type}" is not a valid choice.', code="invalid_choice"
-    #     )
+    if not features.has(
+        "organizations:tracemetrics-top-level-params", organization=organization, actor=request.user
+    ):
+        if not metric_name:
+            metric_name = ""
+        if not metric_type:
+            metric_type = ""
+    else:
+        errors = {}
+
+        if not metric_name:
+            errors["metricName"] = ErrorDetail("This field is required.", code="required")
+
+        if not metric_type:
+            errors["metricType"] = ErrorDetail("This field is required.", code="required")
+        elif metric_type not in ALLOWED_METRIC_TYPES:
+            errors["metricType"] = ErrorDetail(
+                string=f'"{metric_type}" is not a valid choice.', code="invalid_choice"
+            )
 
-    # if errors:
-    #     raise ValidationError(errors)
+        if errors:
+            raise ValidationError(errors)
 
-    # assert metric_name
+        assert metric_name
 
     return metric_name, cast(MetricType, metric_type)
diff --git a/src/sentry/search/eap/trace_metrics/formulas.py b/src/sentry/search/eap/trace_metrics/formulas.py
@@ -10,6 +10,7 @@
 
 from sentry.exceptions import InvalidSearchQuery
 from sentry.search.eap.columns import (
+    AttributeArgumentDefinition,
     FormulaDefinition,
     ResolvedArguments,
     ResolverSettings,
@@ -80,8 +81,13 @@ def per_minute(args: ResolvedArguments, settings: ResolverSettings) -> Column.Bi
     "per_second": FormulaDefinition(
         default_search_type="rate",
         arguments=[
-            ValueArgumentDefinition(
-                argument_types={"string"},
+            AttributeArgumentDefinition(
+                attribute_types={
+                    "string",
+                    "number",
+                    "integer",
+                },
+                default_arg="value",
             ),
             ValueArgumentDefinition(
                 argument_types={"string"},
@@ -90,12 +96,18 @@ def per_minute(args: ResolvedArguments, settings: ResolverSettings) -> Column.Bi
         ],
         formula_resolver=per_second,
         is_aggregate=True,
+        infer_search_type_from_arguments=False,
     ),
     "per_minute": FormulaDefinition(
         default_search_type="rate",
         arguments=[
-            ValueArgumentDefinition(
-                argument_types={"string"},
+            AttributeArgumentDefinition(
+                attribute_types={
+                    "string",
+                    "number",
+                    "integer",
+                },
+                default_arg="value",
             ),
             ValueArgumentDefinition(
                 argument_types={"string"},
@@ -104,5 +116,6 @@ def per_minute(args: ResolvedArguments, settings: ResolverSettings) -> Column.Bi
         ],
         formula_resolver=per_minute,
         is_aggregate=True,
+        infer_search_type_from_arguments=False,
     ),
 }
diff --git a/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py b/tests/snuba/api/endpoints/test_organization_events_trace_metrics.py
@@ -9,36 +9,36 @@
 class OrganizationEventsTraceMetricsEndpointTest(OrganizationEventsEndpointTestBase):
     dataset = "tracemetrics"
 
-    @pytest.mark.skip(reason="not implemented yet")
     def test_missing_metric_name_and_type(self):
-        response = self.do_request(
-            {
-                "field": ["sum(value)"],
-                "dataset": self.dataset,
-                "project": self.project.id,
+        with self.feature("organizations:tracemetrics-top-level-params"):
+            response = self.do_request(
+                {
+                    "field": ["sum(value)"],
+                    "dataset": self.dataset,
+                    "project": self.project.id,
+                }
+            )
+            assert response.status_code == 400, response.content
+            assert response.data == {
+                "metricName": ErrorDetail("This field is required.", code="required"),
+                "metricType": ErrorDetail("This field is required.", code="required"),
             }
-        )
-        assert response.status_code == 400, response.content
-        assert response.data == {
-            "metricName": ErrorDetail("This field is required.", code="required"),
-            "metricType": ErrorDetail("This field is required.", code="required"),
-        }
 
-    @pytest.mark.skip(reason="not implemented yet")
     def test_invalid_metric_type(self):
-        response = self.do_request(
-            {
-                "metricName": "foo",
-                "metricType": "bar",
-                "field": ["sum(value)"],
-                "dataset": self.dataset,
-                "project": self.project.id,
+        with self.feature("organizations:tracemetrics-top-level-params"):
+            response = self.do_request(
+                {
+                    "metricName": "foo",
+                    "metricType": "bar",
+                    "field": ["sum(value)"],
+                    "dataset": self.dataset,
+                    "project": self.project.id,
+                }
+            )
+            assert response.status_code == 400, response.content
+            assert response.data == {
+                "metricType": ErrorDetail('"bar" is not a valid choice.', code="invalid_choice"),
             }
-        )
-        assert response.status_code == 400, response.content
-        assert response.data == {
-            "metricType": ErrorDetail('"bar" is not a valid choice.', code="invalid_choice"),
-        }
 
     def test_simple_deprecated(self) -> None:
         trace_metrics = [
