Просмотр исходного кода

ref(discover): Adding tests for top events with projects (#24711)

- Currently accidentally adding a condition for projects as well, but
  its just a redundant condition
- Adds a test to check that we're adjusting the filter correctly
William Mak 4 лет назад
Родитель
Сommit
550f4c7771
2 измененных файлов с 63 добавлено и 1 удалено
  1. 1 0
      src/sentry/snuba/discover.py
  2. 62 1
      tests/sentry/snuba/test_discover.py

+ 1 - 0
src/sentry/snuba/discover.py

@@ -469,6 +469,7 @@ def top_events_timeseries(
             # If we have a project field, we need to limit results by project so we dont hit the result limit
             if field in ["project", "project.id"]:
                 snuba_filter.project_ids = [event["project.id"] for event in top_events["data"]]
+                continue
             if field in FIELD_ALIASES:
                 field = FIELD_ALIASES[field].alias
             # Note that because orderby shouldn't be an array field its not included in the values

+ 62 - 1
tests/sentry/snuba/test_discover.py

@@ -2177,7 +2177,7 @@ class QueryTransformTest(TestCase):
         }
 
 
-class TimeseriesQueryTest(SnubaTestCase, TestCase):
+class TimeseriesBase(SnubaTestCase, TestCase):
     def setUp(self):
         super().setUp()
 
@@ -2215,6 +2215,8 @@ class TimeseriesQueryTest(SnubaTestCase, TestCase):
             project_id=self.project.id,
         )
 
+
+class TimeseriesQueryTest(TimeseriesBase):
     def test_invalid_field_in_function(self):
         with pytest.raises(InvalidSearchQuery):
             discover.timeseries_query(
@@ -2391,6 +2393,65 @@ class TimeseriesQueryTest(SnubaTestCase, TestCase):
                 assert d["count"] == 2
 
 
+class TopEventsTimeseriesQueryTest(TimeseriesBase):
+    @patch("sentry.snuba.discover.raw_query")
+    def test_project_filter_adjusts_filter(self, mock_query):
+        """ While the function is called with 2 project_ids, we should limit it down to the 1 in top_events """
+        project2 = self.create_project(organization=self.organization)
+        top_events = {
+            "data": [
+                {
+                    "project": self.project.slug,
+                    "project.id": self.project.id,
+                }
+            ]
+        }
+        start = before_now(minutes=5)
+        end = before_now(seconds=1)
+        discover.top_events_timeseries(
+            selected_columns=["project", "count()"],
+            params={
+                "start": start,
+                "end": end,
+                "project_id": [self.project.id, project2.id],
+            },
+            rollup=3600,
+            top_events=top_events,
+            timeseries_columns=["count()"],
+            user_query="",
+            orderby=["count()"],
+            limit=10000,
+            organization=self.organization,
+        )
+        mock_query.assert_called_with(
+            aggregations=[["count", None, "count"]],
+            conditions=[],
+            # Should be limited to the project in top_events
+            filter_keys={"project_id": [self.project.id]},
+            selected_columns=[
+                "project_id",
+                [
+                    "transform",
+                    [
+                        ["toString", ["project_id"]],
+                        ["array", [f"'{project.id}'" for project in [self.project, project2]]],
+                        ["array", [f"'{project.slug}'" for project in [self.project, project2]]],
+                        "''",
+                    ],
+                    "project",
+                ],
+            ],
+            start=start,
+            end=end,
+            rollup=3600,
+            orderby=["time", "project_id"],
+            groupby=["time", "project_id"],
+            dataset=Dataset.Discover,
+            limit=10000,
+            referrer=None,
+        )
+
+
 def format_project_event(project_slug, event_id):
     return f"{project_slug}:{event_id}"