fix(perf): Add condition to check for duration to top tags (#27479)

The top tags query was missing the condition to check for transaction.duration or measurements.lcp, which means sometimes the histogram returns slightl out of order or there will be an empty row or two. This should fix top tags to only select the correct top tags.

src/sentry/api/endpoints/organization_events_facets_performance.py

@@ -167,6 +167,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpoint(
                     tag_key=tag_key,
                     limit=tag_key_limit,
                     filter_query=filter_query,
+                    aggregate_column=aggregate_column,
                     params=params,
                     orderby=self.get_orderby(request),
                     referrer=referrer,
@@ -267,6 +268,7 @@ def query_top_tags(
     limit: int,
     referrer: str,
     orderby: Optional[List[str]],
+    aggregate_column: Optional[str] = None,
     filter_query: Optional[str] = None,
 ) -> Optional[List[Any]]:
     """
@@ -283,6 +285,8 @@ def query_top_tags(
         # Resolve the public aliases into the discover dataset names.
         snuba_filter, translated_columns = discover.resolve_discover_aliases(snuba_filter)
 
+    translated_aggregate_column = discover.resolve_discover_column(aggregate_column)
+
     with sentry_sdk.start_span(op="discover.discover", description="facets.top_tags"):
 
         if not orderby:
@@ -302,7 +306,10 @@ def query_top_tags(
             query=filter_query,
             params=params,
             orderby=orderby,
-            conditions=[["tags_key", "IN", [tag_key]]],
+            conditions=[
+                [translated_aggregate_column, "IS NOT NULL", None],
+                ["tags_key", "IN", [tag_key]],
+            ],
             functions_acl=["array_join"],
             referrer=f"{referrer}.top_tags",
             limit=limit,

tests/snuba/api/endpoints/test_organization_events_facets_performance_histogram.py

@@ -28,18 +28,19 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
 
         for i in range(5):
             self.store_transaction(
-                tags=[["color", "blue"], ["many", "yes"]],
-                duration=4000,
+                tags=[["color", "blue"], ["many", "yes"]], duration=4000, lcp=4000
             )
+
+        # LCP-less transaction
+        self.store_transaction(tags=[["color", "orange"], ["many", "maybe"]], lcp=None)
+
         for i in range(14):
             self.store_transaction(
-                tags=[["color", "red"], ["many", "yes"]],
-                duration=1000,
+                tags=[["color", "red"], ["many", "yes"]], duration=1000, lcp=1000
             )
         for i in range(1):
             self.store_transaction(
-                tags=[["color", "green"], ["many", "no"]],
-                duration=5000,
+                tags=[["color", "green"], ["many", "no"]], duration=5000, lcp=5000
             )
 
         self.url = reverse(
@@ -48,7 +49,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
         )
 
     def store_transaction(
-        self, name="exampleTransaction", duration=100, tags=None, project_id=None
+        self, name="exampleTransaction", duration=100, tags=None, project_id=None, lcp=None
     ):
         if tags is None:
             tags = []
@@ -64,6 +65,11 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
                 "timestamp": iso_format(self.two_mins_ago),
             }
         )
+        if lcp:
+            event["measurements"]["lcp"]["value"] = lcp
+        else:
+            del event["measurements"]["lcp"]
+
         self._transaction_count += 1
         self.store_event(data=event, project_id=project_id)
 
@@ -228,3 +234,46 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
         assert histogram_data[2]["histogram_transaction_duration_750000_750000_1"] == 4500000.0
         assert histogram_data[2]["tags_value"] == "green"
         assert histogram_data[2]["tags_key"] == "color"
+
+    def test_histograms_omit_empty_measurements(self):
+        request = {
+            "aggregateColumn": "transaction.duration",
+            "sort": "-frequency",
+            "per_page": 5,
+            "statsPeriod": "14d",
+            "tagKeyLimit": 3,
+            "numBucketsPerKey": 2,
+            "tagKey": "color",
+            "query": "(color:red or color:blue or color:green or color:orange)",
+        }
+
+        data_response = self.do_request(
+            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
+        )
+
+        assert data_response.data["tags"]["data"][2]["tags_value"] == "orange"
+
+        request["aggregateColumn"] = "measurements.lcp"
+
+        data_response = self.do_request(
+            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
+        )
+
+        assert data_response.data["tags"]["data"][2]["tags_value"] == "green"
+
+        histogram_data = data_response.data["histogram"]["data"]
+        assert len(histogram_data) == 3
+        assert histogram_data[0]["count"] == 14
+        assert histogram_data[0]["histogram_measurements_lcp_750_750_1"] == 750.0
+        assert histogram_data[0]["tags_value"] == "red"
+        assert histogram_data[0]["tags_key"] == "color"
+
+        assert histogram_data[1]["count"] == 5
+        assert histogram_data[1]["histogram_measurements_lcp_750_750_1"] == 3750.0
+        assert histogram_data[1]["tags_value"] == "blue"
+        assert histogram_data[1]["tags_key"] == "color"
+
+        assert histogram_data[2]["count"] == 1
+        assert histogram_data[2]["histogram_measurements_lcp_750_750_1"] == 4500.0
+        assert histogram_data[2]["tags_value"] == "green"
+        assert histogram_data[2]["tags_key"] == "color"