fix(timeseries): Handle orderbys not in groupby (#103547)

- If a field is in the orderby but not in the groupbys we currently
error in snuba, but this updates the backend so we error instead so we
can give the user a more friendly error message
- Also fixes a bug where timeseries could be passed to the orderby, but
  we don't want that

Author: wmak

src/sentry/api/endpoints/organization_events_timeseries.py

@@ -252,10 +252,14 @@ def get_rpc_config():
 
         if top_events > 0:
             raw_groupby = self.get_field_list(organization, request, param_name="groupBy")
+            raw_orderby = self.get_orderby(request)
             if len(raw_groupby) == 0:
                 raise ParseError("groupBy is a required parameter when doing topEvents")
             if "timestamp" in raw_groupby:
                 raise ParseError("Cannot group by timestamp")
+            if raw_orderby:
+                if "timestamp" in [col.strip("-") for col in raw_orderby]:
+                    raise ParseError("Cannot order by timestamp")
             if dataset in RPC_DATASETS:
                 return dataset.run_top_events_timeseries_query(
                     params=snuba_params,

src/sentry/snuba/rpc_dataset_common.py

@@ -263,6 +263,9 @@ def get_table_rpc_request(cls, query: TableQuery) -> TableRequest:
             stripped_orderby = orderby_column.lstrip("-")
             if stripped_orderby in orderby_aliases:
                 resolved_column = orderby_aliases[stripped_orderby]
+            # If this orderby isn't in the aliases, check if its a selected column
+            elif stripped_orderby not in query.selected_columns:
+                raise InvalidSearchQuery("orderby must also be in the selected columns or groupby")
             else:
                 resolved_column = resolver.resolve_column(stripped_orderby)[0]
             resolved_orderby.append(

tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py

@@ -527,6 +527,69 @@ def test_top_events(self) -> None:
             "order": 2,
         }
 
+    def test_top_events_orderby_not_in_groupby(self) -> None:
+        self.store_spans(
+            [
+                self.create_span(
+                    {"sentry_tags": {"transaction": "foo", "status": "success"}},
+                    start_ts=self.start + timedelta(minutes=1),
+                    duration=2001,
+                ),
+                self.create_span(
+                    {"sentry_tags": {"transaction": "bar", "status": "success"}},
+                    start_ts=self.start + timedelta(minutes=1),
+                    duration=2000,
+                ),
+                self.create_span(
+                    {"sentry_tags": {"transaction": "baz", "status": "success"}},
+                    start_ts=self.start + timedelta(minutes=1),
+                    duration=1999,
+                ),
+                self.create_span(
+                    {"sentry_tags": {"transaction": "qux", "status": "success"}},
+                    start_ts=self.start + timedelta(minutes=1),
+                    duration=1998,
+                ),
+            ],
+            is_eap=True,
+        )
+
+        self.end = self.start + timedelta(minutes=6)
+        response = self._do_request(
+            data={
+                "start": self.start,
+                "end": self.end,
+                "interval": "1m",
+                "yAxis": "count()",
+                "groupBy": ["transaction"],
+                "orderby": ["-sum(span.self_time)"],
+                "project": self.project.id,
+                "dataset": "spans",
+                "excludeOther": 0,
+                "topEvents": 2,
+            },
+        )
+        assert response.status_code == 400, response.content
+        assert "orderby must also be in the selected columns or groupby" == response.data["detail"]
+
+    def test_top_events_orderby_is_timestamp(self) -> None:
+        response = self._do_request(
+            data={
+                "start": self.start,
+                "end": self.end,
+                "interval": "1m",
+                "yAxis": "count()",
+                "groupBy": ["transaction"],
+                "orderby": ["timestamp"],
+                "project": self.project.id,
+                "dataset": "spans",
+                "excludeOther": 0,
+                "topEvents": 2,
+            },
+        )
+        assert response.status_code == 400, response.content
+        assert "Cannot order by timestamp" == response.data["detail"]
+
     def test_top_events_with_none_as_groupby(self) -> None:
         self.store_spans(
             [