Browse Source

feat(metrics): Support sorting on summary for all mris (#66627)

Going to use this as the default sort in most cases to just sort by the
column corresponding to the MRI. This will enable sorting on
measurements as well that wasn't allowed before.
Tony Xiao 1 year ago
parent
commit
5def5def59

+ 38 - 5
src/sentry/sentry_metrics/querying/samples_list.py

@@ -152,7 +152,7 @@ class AbstractSamplesListExecutor(ABC):
 
 
 class SegmentsSamplesListExecutor(AbstractSamplesListExecutor):
-    sortable_columns = {"timestamp", "span.duration"}
+    sortable_columns = {"timestamp", "span.duration", "summary"}
 
     SORT_MAPPING = {
         "span.duration": "transaction.duration",
@@ -165,13 +165,21 @@ class SegmentsSamplesListExecutor(AbstractSamplesListExecutor):
         raise NotImplementedError
 
     @classmethod
-    def convert_sort(cls, sort: str) -> tuple[Literal["", "-"], str] | None:
+    def convert_sort(cls, sort: str, mri: str) -> tuple[Literal["", "-"], str] | None:
         direction: Literal["", "-"] = ""
+
         if sort.startswith("-"):
             direction = "-"
             sort = sort[1:]
+
         if sort in cls.SORT_MAPPING:
             return direction, cls.SORT_MAPPING[sort]
+
+        if sort == "summary":
+            column = cls.mri_to_column(mri)
+            if column is not None:
+                return direction, column
+
         return None
 
     @classmethod
@@ -224,7 +232,7 @@ class SegmentsSamplesListExecutor(AbstractSamplesListExecutor):
         may not contain all the necessary data.
         """
         assert self.sort
-        sort = self.convert_sort(self.sort)
+        sort = self.convert_sort(self.sort, self.mri)
         assert sort is not None
         direction, sort_column = sort
 
@@ -426,13 +434,31 @@ class TransactionMeasurementsSamplesListExecutor(SegmentsSamplesListExecutor):
 
 
 class SpansSamplesListExecutor(AbstractSamplesListExecutor):
-    sortable_columns = {"timestamp", "span.duration", "span.self_time"}
+    sortable_columns = {"timestamp", "span.duration", "span.self_time", "summary"}
 
     @classmethod
     @abstractmethod
     def mri_to_column(cls, mri) -> str | None:
         raise NotImplementedError
 
+    @classmethod
+    def convert_sort(cls, sort: str, mri: str) -> tuple[Literal["", "-"], str] | None:
+        direction: Literal["", "-"] = ""
+
+        if sort.startswith("-"):
+            direction = "-"
+            sort = sort[1:]
+
+        if sort == "summary":
+            column = cls.mri_to_column(mri)
+            if column is not None:
+                return direction, column
+
+        if sort in cls.sortable_columns:
+            return direction, sort
+
+        return None
+
     @classmethod
     def supports_mri(cls, mri: str) -> bool:
         return cls.mri_to_column(mri) is not None
@@ -443,7 +469,14 @@ class SpansSamplesListExecutor(AbstractSamplesListExecutor):
         there's no reason to split this into 2 queries. We can go ahead and
         just do it all in a single query.
         """
+        assert self.sort
+        sort = self.convert_sort(self.sort, self.mri)
+        assert sort is not None
+        direction, sort_column = sort
+
         fields = self.fields[:]
+        if sort_column not in fields:
+            fields.append(sort_column)
 
         column = self.mri_to_column(self.mri)
         assert column is not None
@@ -455,7 +488,7 @@ class SpansSamplesListExecutor(AbstractSamplesListExecutor):
             self.params,
             snuba_params=self.snuba_params,
             selected_columns=fields,
-            orderby=self.sort,
+            orderby=f"{direction}{sort_column}",
             limit=limit,
             offset=0,
         )

+ 4 - 4
tests/sentry/api/endpoints/test_organization_metrics.py

@@ -236,7 +236,7 @@ class OrganizationMetricsSamplesEndpointTest(BaseSpansTestCase, APITestCase):
             "field": ["id", "span.self_time"],
             "project": [self.project.id],
             "statsPeriod": "14d",
-            "sort": "-span.self_time",
+            "sort": "-summary",
         }
         response = self.do_request(query)
         assert response.status_code == 200, response.data
@@ -329,7 +329,7 @@ class OrganizationMetricsSamplesEndpointTest(BaseSpansTestCase, APITestCase):
                 "field": ["id", "span.duration"],
                 "project": [self.project.id],
                 "statsPeriod": "14d",
-                "sort": "-span.duration",
+                "sort": "-summary",
             }
             response = self.do_request(query)
             assert response.status_code == 200, response.data
@@ -399,7 +399,7 @@ class OrganizationMetricsSamplesEndpointTest(BaseSpansTestCase, APITestCase):
             "field": ["id", "span.duration"],
             "project": [self.project.id],
             "statsPeriod": "14d",
-            "sort": "-span.duration",
+            "sort": "-summary",
         }
         response = self.do_request(query)
         assert response.status_code == 200, response.data
@@ -493,7 +493,7 @@ class OrganizationMetricsSamplesEndpointTest(BaseSpansTestCase, APITestCase):
             "field": ["id", "span.duration"],
             "project": [self.project.id],
             "statsPeriod": "14d",
-            "sort": "-span.duration",
+            "sort": "-summary",
         }
         response = self.do_request(query)
         assert response.status_code == 200, response.data