Browse Source

feature(discover) - Adding an order key to top events response (#18441)

- This will allow the frontend to order the tooltip, so that it matches
  what's in the table
- This modifies the tests so the test events have an order from count
  too
William Mak 4 years ago
parent
commit
b4966b6dd0

+ 10 - 4
src/sentry/api/bases/organization_events.py

@@ -18,7 +18,7 @@ from sentry.api.serializers.snuba import SnubaTSResultSerializer
 from sentry.models.project import Project
 from sentry.models.group import Group
 from sentry.snuba import discover
-from sentry.utils.compat import map, zip
+from sentry.utils.compat import map
 from sentry.utils.dates import get_rollup_from_request
 from sentry.utils import snuba
 
@@ -187,10 +187,16 @@ class OrganizationEventsV2EndpointBase(OrganizationEventsEndpointBase):
 
     def serialize_multiple_axis(self, serializer, event_result, columns, query_columns):
         # Return with requested yAxis as the key
-        return {
-            column: serializer.serialize(event_result, get_function_alias(query_column))
-            for column, query_column in zip(columns, query_columns)
+        result = {
+            columns[index]: serializer.serialize(
+                event_result, get_function_alias(query_column), order=index
+            )
+            for index, query_column in enumerate(query_columns)
         }
+        # Set order if multi-axis + top events
+        if "order" in event_result.data:
+            result["order"] = event_result.data["order"]
+        return result
 
 
 class KeyTransactionBase(OrganizationEventsV2EndpointBase):

+ 6 - 1
src/sentry/api/serializers/snuba.py

@@ -290,7 +290,7 @@ class SnubaTSResultSerializer(BaseSnubaSerializer):
     Serializer for time-series Snuba data.
     """
 
-    def serialize(self, result, column="count"):
+    def serialize(self, result, column="count", order=None):
         data = [
             (key, list(group))
             for key, group in itertools.groupby(result.data["data"], key=lambda r: r["time"])
@@ -314,5 +314,10 @@ class SnubaTSResultSerializer(BaseSnubaSerializer):
 
         if result.data.get("totals"):
             res["totals"] = {"count": result.data["totals"][column]}
+        # If order is passed let that overwrite whats in data since its order for multi-axis
+        if order is not None:
+            res["order"] = order
+        elif "order" in result.data:
+            res["order"] = result.data["order"]
 
         return res

+ 22 - 9
src/sentry/snuba/discover.py

@@ -788,6 +788,16 @@ def timeseries_query(selected_columns, query, params, rollup, reference_event=No
     return SnubaTSResult({"data": result}, snuba_filter.start, snuba_filter.end, rollup)
 
 
+def create_result_key(result_row, fields, issues):
+    values = []
+    for field in fields:
+        if field == "issue.id":
+            values.append(issues.get(result_row["issue.id"], "unknown"))
+        else:
+            values.append(six.text_type(result_row.get(field)))
+    return ",".join(values)
+
+
 def top_events_timeseries(
     timeseries_columns,
     selected_columns,
@@ -897,17 +907,20 @@ def top_events_timeseries(
 
     results = {}
     for row in result["data"]:
-        values = []
-        for field in translated_groupby:
-            if field == "issue.id":
-                values.append(issues.get(row["issue.id"], "unknown"))
-            else:
-                values.append(six.text_type(row.get(field)))
-        result_key = ",".join(values)
-        results.setdefault(result_key, []).append(row)
+        result_key = create_result_key(row, translated_groupby, issues)
+        results.setdefault(result_key, {"data": []})["data"].append(row)
+    # Using the top events add the order to the results
+    for index, item in enumerate(top_events["data"]):
+        result_key = create_result_key(item, translated_groupby, issues)
+        results[result_key]["order"] = index
     for key, item in six.iteritems(results):
         results[key] = SnubaTSResult(
-            {"data": zerofill(item, snuba_filter.start, snuba_filter.end, rollup, "time")},
+            {
+                "data": zerofill(
+                    item["data"], snuba_filter.start, snuba_filter.end, rollup, "time"
+                ),
+                "order": item["order"],
+            },
             snuba_filter.start,
             snuba_filter.end,
             rollup,

+ 35 - 12
tests/snuba/api/endpoints/test_organization_events_stats.py

@@ -534,10 +534,12 @@ class OrganizationEventsStatsEndpointTest(APITestCase, SnubaTestCase):
             )
 
         assert response.status_code == 200, response.content
+        response.data["user_count"]["order"] == 0
         assert [attrs for time, attrs in response.data["user_count"]["data"]] == [
             [{"count": 1}],
             [{"count": 1}],
         ]
+        response.data["event_count"]["order"] == 1
         assert [attrs for time, attrs in response.data["event_count"]["data"]] == [
             [{"count": 1}],
             [{"count": 2}],
@@ -613,7 +615,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                     "fingerprint": ["group1"],
                 },
                 "project": self.project2,
-                "count": 3,
+                "count": 7,
             },
             {
                 "data": {
@@ -623,7 +625,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                     "user": {"email": self.user2.email},
                 },
                 "project": self.project2,
-                "count": 3,
+                "count": 6,
             },
             {
                 "data": {
@@ -633,7 +635,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                     "user": {"email": "foo@example.com"},
                 },
                 "project": self.project,
-                "count": 3,
+                "count": 5,
             },
             {
                 "data": {
@@ -643,7 +645,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                     "user": {"email": "bar@example.com"},
                 },
                 "project": self.project,
-                "count": 3,
+                "count": 4,
             },
             {"data": transaction_data, "project": self.project, "count": 3},
             # Not in the top 5
@@ -708,6 +710,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
             results = data[
                 ",".join([message, self.event_data[index]["data"]["user"].get("email", "None")])
             ]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"]}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -758,6 +761,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         for index, event in enumerate(self.events[:5]):
             message = event.message or event.transaction
             results = data[",".join([message, event.project.slug])]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"]}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -796,6 +800,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                 issue = event.group.qualified_short_id
 
             results = data[",".join([issue, message])]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"]}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -822,12 +827,14 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert len(data) == 1
 
         results = data[self.transaction.transaction]
+        assert results["order"] == 0
         assert [attrs for time, attrs in results["data"]] == [
             [{"count": 3}],
             [{"count": 0}],
         ]
 
     def test_top_events_with_functions_on_different_transactions(self):
+        """ Transaction2 has less events, but takes longer so order should be self.transaction then transaction2 """
         transaction_data = load_data("transaction")
         transaction_data["start_timestamp"] = iso_format(self.day_ago + timedelta(minutes=2))
         transaction_data["timestamp"] = iso_format(self.day_ago + timedelta(minutes=6))
@@ -854,12 +861,14 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert len(data) == 2
 
         results = data[self.transaction.transaction]
+        assert results["order"] == 1
         assert [attrs for time, attrs in results["data"]] == [
             [{"count": 3}],
             [{"count": 0}],
         ]
 
         results = data[transaction2.transaction]
+        assert results["order"] == 0
         assert [attrs for time, attrs in results["data"]] == [
             [{"count": 1}],
             [{"count": 0}],
@@ -893,6 +902,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert len(data) == 1
 
         transaction2_data = data["/foo_bar/"]
+        assert transaction2_data["order"] == 0
         assert [attrs for time, attrs in transaction2_data["data"]] == [
             [{"count": 1}],
             [{"count": 0}],
@@ -923,6 +933,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
             results = data[
                 ",".join([message, self.event_data[index]["data"]["user"].get("email", "None")])
             ]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"] / (3600.0 / 60.0)}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -952,6 +963,9 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
             results = data[
                 ",".join([message, self.event_data[index]["data"]["user"].get("email", "None")])
             ]
+            assert results["order"] == index
+            assert results["rpm()"]["order"] == 0
+            assert results["count()"]["order"] == 1
             assert [{"count": self.event_data[index]["count"] / (3600.0 / 60.0)}] in [
                 attrs for time, attrs in results["rpm()"]["data"]
             ]
@@ -983,6 +997,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         for index, event in enumerate(self.events[:5]):
             message = event.message or event.transaction
             results = data[",".join(["False", message])]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"]}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -1007,13 +1022,16 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         data = response.data
         assert response.status_code == 200, response.content
         assert len(data) == 5
+        # Transactions won't be in the results because of the query
+        del self.events[4]
+        del self.event_data[4]
 
-        for index, event in enumerate(self.events[:6]):
-            if event.message:
-                results = data[",".join([event.message, event.timestamp])]
-                assert [{"count": self.event_data[index]["count"]}] in [
-                    attrs for time, attrs in results["data"]
-                ]
+        for index, event in enumerate(self.events[:5]):
+            results = data[",".join([event.message, event.timestamp])]
+            assert results["order"] == index
+            assert [{"count": self.event_data[index]["count"]}] in [
+                attrs for time, attrs in results["data"]
+            ]
 
     def test_top_events_with_int(self):
         with self.feature("organizations:discover-basic"):
@@ -1036,6 +1054,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert len(data) == 1
 
         results = data[",".join([self.transaction.transaction, "120000"])]
+        assert results["order"] == 0
         assert [attrs for time, attrs in results["data"]] == [
             [{"count": 3}],
             [{"count": 0}],
@@ -1061,8 +1080,9 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert response.status_code == 200, response.content
         assert len(data) == 5
 
+        assert data["bar@example.com"]["order"] == 0
         assert [attrs for time, attrs in data["bar@example.com"]["data"]] == [
-            [{"count": 6}],
+            [{"count": 7}],
             [{"count": 0}],
         ]
         assert [attrs for time, attrs in data["127.0.0.1"]["data"]] == [
@@ -1090,8 +1110,9 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
         assert response.status_code == 200, response.content
         assert len(data) == 5
 
+        assert data["bar@example.com,bar@example.com"]["order"] == 0
         assert [attrs for time, attrs in data["bar@example.com,bar@example.com"]["data"]] == [
-            [{"count": 6}],
+            [{"count": 7}],
             [{"count": 0}],
         ]
         assert [attrs for time, attrs in data["127.0.0.1,None"]["data"]] == [
@@ -1131,6 +1152,7 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
                 issue = event.group.qualified_short_id
 
             results = data[issue]
+            assert results["order"] == index
             assert [{"count": self.event_data[index]["count"]}] in [
                 attrs for time, attrs in results["data"]
             ]
@@ -1162,3 +1184,4 @@ class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
             [{"count": 3}],
             [{"count": 0}],
         ]
+        assert results["order"] == 0