Browse Source

feat(metrics): Don't error when querying for metrics that aren't in the indexer (#70525)

- This allows the span dataset to query for metrics that aren't in the
indexer
- Sets the metric_id to 0 in case it leaks through, and ignores the
columns in these cases
William Mak 9 months ago
parent
commit
d87b110df9

+ 26 - 11
src/sentry/search/events/builder/metrics.py

@@ -104,6 +104,7 @@ class MetricsQueryBuilder(QueryBuilder):
         self.metric_ids: set[int] = set()
         self._indexer_cache: dict[str, int | None] = {}
         self._use_default_tags: bool | None = None
+        self._has_nullable: bool = False
         # always true if this is being called
         config.has_metrics = True
         assert dataset is None or dataset in [Dataset.PerformanceMetrics, Dataset.Metrics]
@@ -594,35 +595,43 @@ class MetricsQueryBuilder(QueryBuilder):
         resolve_only: bool,
     ) -> SelectType | None:
         prefix = self._get_metric_prefix(snql_function, arguments.get("column"))
+        # If the metric_id is 0 that means this is a function that won't return but we don't want to error the query
+        nullable = arguments.get("metric_id") == 0
+        if nullable:
+            self._has_nullable = True
 
         if snql_function.snql_distribution is not None and (prefix is None or prefix == "d"):
             resolved_function = snql_function.snql_distribution(arguments, alias)
             if not resolve_only:
-                if snql_function.is_percentile:
-                    self.percentiles.append(resolved_function)
-                else:
-                    self.distributions.append(resolved_function)
+                if not nullable:
+                    if snql_function.is_percentile:
+                        self.percentiles.append(resolved_function)
+                    else:
+                        self.distributions.append(resolved_function)
                 # Still add to aggregates so groupby is correct
                 self.aggregates.append(resolved_function)
             return resolved_function
         if snql_function.snql_set is not None and (prefix is None or prefix == "s"):
             resolved_function = snql_function.snql_set(arguments, alias)
             if not resolve_only:
-                self.sets.append(resolved_function)
+                if not nullable:
+                    self.sets.append(resolved_function)
                 # Still add to aggregates so groupby is correct
                 self.aggregates.append(resolved_function)
             return resolved_function
         if snql_function.snql_counter is not None and (prefix is None or prefix == "c"):
             resolved_function = snql_function.snql_counter(arguments, alias)
             if not resolve_only:
-                self.counters.append(resolved_function)
+                if not nullable:
+                    self.counters.append(resolved_function)
                 # Still add to aggregates so groupby is correct
                 self.aggregates.append(resolved_function)
             return resolved_function
         if snql_function.snql_gauge is not None and (prefix is None or prefix == "g"):
             resolved_function = snql_function.snql_gauge(arguments, alias)
             if not resolve_only:
-                self.gauges.append(resolved_function)
+                if not nullable:
+                    self.gauges.append(resolved_function)
                 # Still add to aggregates so groupby is correct
                 self.aggregates.append(resolved_function)
             return resolved_function
@@ -630,10 +639,11 @@ class MetricsQueryBuilder(QueryBuilder):
             resolved_function = snql_function.snql_metric_layer(arguments, alias)
             if not resolve_only:
                 self.aggregates.append(resolved_function)
-                if snql_function.is_percentile:
-                    self.percentiles.append(resolved_function)
-                else:
-                    self.metrics_layer_functions.append(resolved_function)
+                if not nullable:
+                    if snql_function.is_percentile:
+                        self.percentiles.append(resolved_function)
+                    else:
+                        self.metrics_layer_functions.append(resolved_function)
             return resolved_function
         return None
 
@@ -1301,6 +1311,11 @@ class MetricsQueryBuilder(QueryBuilder):
 
         result["data"] = list(value_map.values())
         result["meta"] = [{"name": key, "type": value} for key, value in meta_dict.items()]
+        # Nullable columns won't be in the meta
+        if self._has_nullable:
+            for function in self.aggregates:
+                if function.alias not in [meta["name"] for meta in result["meta"]]:
+                    result["meta"].append({"name": function.alias, "type": "Nullable"})
 
         # Data might be missing for fields after merging the requests, eg a transaction with no users
         for row in result["data"]:

+ 3 - 2
src/sentry/search/events/constants.py

@@ -326,6 +326,8 @@ DEFAULT_METRIC_TAGS = {
     "trace.status",
     "messaging.destination.name",
 }
+SPAN_MESSAGING_LATENCY = "g:spans/messaging.message.receive.latency@millisecond"
+SELF_TIME_LIGHT = "d:spans/exclusive_time_light@millisecond"
 SPAN_METRICS_MAP = {
     "user": "s:spans/user@none",
     "span.self_time": "d:spans/exclusive_time@millisecond",
@@ -338,9 +340,8 @@ SPAN_METRICS_MAP = {
     "mobile.frozen_frames": "g:spans/mobile.frozen_frames@none",
     "mobile.total_frames": "g:spans/mobile.total_frames@none",
     "mobile.frames_delay": "g:spans/mobile.frames_delay@second",
-    "messaging.message.receive.latency": "g:spans/messaging.message.receive.latency@millisecond",
+    "messaging.message.receive.latency": SPAN_MESSAGING_LATENCY,
 }
-SELF_TIME_LIGHT = "d:spans/exclusive_time_light@millisecond"
 # 50 to match the size of tables in the UI + 1 for pagination reasons
 METRICS_MAX_LIMIT = 101
 

+ 7 - 2
src/sentry/search/events/datasets/spans_metrics.py

@@ -20,6 +20,7 @@ from sentry.snuba.referrer import Referrer
 
 class SpansMetricsDatasetConfig(DatasetConfig):
     missing_function_error = IncompatibleMetricsQuery
+    nullable_metrics = {constants.SPAN_MESSAGING_LATENCY}
 
     def __init__(self, builder: builder.SpansMetricsQueryBuilder):
         self.builder = builder
@@ -55,8 +56,12 @@ class SpansMetricsDatasetConfig(DatasetConfig):
         metric_id = self.builder.resolve_metric_index(constants.SPAN_METRICS_MAP.get(value, value))
         # If its still None its not a custom measurement
         if metric_id is None:
-            raise IncompatibleMetricsQuery(f"Metric: {value} could not be resolved")
-        self.builder.metric_ids.add(metric_id)
+            if constants.SPAN_METRICS_MAP.get(value, value) in self.nullable_metrics:
+                metric_id = 0
+            else:
+                raise IncompatibleMetricsQuery(f"Metric: {value} could not be resolved")
+        if metric_id != 0:
+            self.builder.metric_ids.add(metric_id)
         return metric_id
 
     @property

+ 1 - 1
src/sentry/search/events/fields.py

@@ -694,7 +694,7 @@ def get_json_meta_type(field_alias, snuba_type, builder=None):
         return alias_definition.result_type
 
     snuba_json = get_json_type(snuba_type)
-    if snuba_json != "string":
+    if snuba_json not in ["string", "null"]:
         if function is not None:
             result_type = function.instance.get_result_type(function.field, function.arguments)
             if result_type is not None:

+ 1 - 0
src/sentry/utils/snuba.py

@@ -1451,6 +1451,7 @@ JSON_TYPE_MAP = {
     "Float32": "number",
     "Float64": "number",
     "DateTime": "date",
+    "Nullable": "null",
 }
 
 

+ 36 - 0
tests/snuba/api/endpoints/test_organization_events_span_metrics.py

@@ -1666,6 +1666,41 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             },
         ]
 
+    def test_messaging_does_not_exist_as_metric(self):
+        self.store_span_metric(
+            100,
+            internal_metric=constants.SPAN_METRICS_MAP["span.duration"],
+            tags={"messaging.destination.name": "foo", "trace.status": "ok"},
+            timestamp=self.min_ago,
+        )
+        response = self.do_request(
+            {
+                "field": [
+                    "messaging.destination.name",
+                    "trace.status",
+                    "avg(messaging.message.receive.latency)",
+                    "avg(span.duration)",
+                ],
+                "query": "",
+                "project": self.project.id,
+                "dataset": "spansMetrics",
+                "statsPeriod": "1h",
+            }
+        )
+
+        assert response.status_code == 200, response.content
+        data = response.data["data"]
+        assert data == [
+            {
+                "messaging.destination.name": "foo",
+                "trace.status": "ok",
+                "avg(messaging.message.receive.latency)": None,
+                "avg(span.duration)": 100,
+            },
+        ]
+        meta = response.data["meta"]
+        assert meta["fields"]["avg(messaging.message.receive.latency)"] == "null"
+
     def test_trace_status_rate(self):
         self.store_span_metric(
             1,
@@ -1713,6 +1748,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "query": "",
                 "project": self.project.id,
                 "dataset": "spansMetrics",
+                "statsPeriod": "1h",
             }
         )