Browse Source

fix(ondemand): Wrong column usage here was breaking (#66437)

- If fields had `-` in them the query would fail since we weren't
resolving the column to a tag correctly
- Fixes: SENTRY-2W5S
William Mak 1 year ago
parent
commit
4f323d0628

+ 17 - 12
src/sentry/search/events/builder/metrics.py

@@ -1029,18 +1029,23 @@ class MetricsQueryBuilder(QueryBuilder):
         groupbys = self.groupby
         if not groupbys and self.use_on_demand:
             # Need this otherwise top_events returns only 1 item
-            groupbys = [Column(col) for col in self._get_group_bys()]
-        groupby_aliases = [
-            (
-                groupby.alias
-                if isinstance(groupby, (AliasedExpression, CurriedFunction))
-                else groupby.name
-            )
-            for groupby in groupbys
-            if not (
-                isinstance(groupby, CurriedFunction) and groupby.function == "team_key_transaction"
-            )
-        ]
+            groupbys = [self.resolve_column(col) for col in self._get_group_bys()]
+            # Later the query is made by passing these columns to metrics layer so we can just have the aliases be the
+            # raw groupbys
+            groupby_aliases = self._get_group_bys()
+        else:
+            groupby_aliases = [
+                (
+                    groupby.alias
+                    if isinstance(groupby, (AliasedExpression, CurriedFunction))
+                    else groupby.name
+                )
+                for groupby in groupbys
+                if not (
+                    isinstance(groupby, CurriedFunction)
+                    and groupby.function == "team_key_transaction"
+                )
+            ]
         # The typing for these are weak (all using Any) since the results from snuba can contain an assortment of types
         value_map: dict[str, Any] = defaultdict(dict)
         groupby_values: list[Any] = []

+ 39 - 0
tests/snuba/api/endpoints/test_organization_events_stats_mep.py

@@ -2027,3 +2027,42 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithOnDemandW
         )
 
         assert response.status_code == 400, response.content
+
+    def test_top_events_with_tag(self):
+        query = "transaction.duration:>=100"
+        yAxis = ["count()"]
+        field = "count()"
+        groupbys = ["some-field"]
+        spec = OnDemandMetricSpec(
+            field=field, groupbys=groupbys, query=query, spec_type=MetricSpecType.DYNAMIC_QUERY
+        )
+        self.store_on_demand_metric(
+            1,
+            spec=spec,
+            additional_tags={
+                "some-field": "bar",
+                "environment": "production",
+            },
+            timestamp=self.day_ago,
+        )
+        response = self.do_request(
+            data={
+                "project": self.project.id,
+                "start": iso_format(self.day_ago),
+                "end": iso_format(self.day_ago + timedelta(hours=2)),
+                "interval": "1h",
+                "orderby": ["-count()"],
+                "environment": "production",
+                "query": query,
+                "yAxis": yAxis,
+                "field": [
+                    "some-field",
+                    "count()",
+                ],
+                "topEvents": 5,
+                "dataset": "metrics",
+                "useOnDemandMetrics": "true",
+            },
+        )
+
+        assert response.status_code == 200, response.content