Browse Source

feat(querybuilder): Use query builder in group events endpoint (#41276)

- This adds a feature flag where the query buildere will be used so it
can be gradually rolled out
- this is the last place that get_filter is being used, if this change
is successful the entire get_filter function and all it depends on can
be deleted
William Mak 2 years ago
parent
commit
08e022578a

+ 45 - 14
src/sentry/api/endpoints/group_events.py

@@ -1,23 +1,27 @@
 from __future__ import annotations
 
 from datetime import datetime, timedelta
-from functools import partial
-from typing import TYPE_CHECKING, Optional, Sequence, cast
+from typing import TYPE_CHECKING, Any, Optional, Sequence, cast
 
 from django.utils import timezone
 from rest_framework.exceptions import ParseError
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import eventstore
+from sentry import eventstore, features
 from sentry.api.base import EnvironmentMixin, region_silo_endpoint
 from sentry.api.bases import GroupEndpoint
 from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.helpers.environments import get_environments
-from sentry.api.helpers.events import get_direct_hit_response, get_filter_for_group
+from sentry.api.helpers.events import (
+    get_direct_hit_response,
+    get_filter_for_group,
+    get_query_builder_for_group,
+)
 from sentry.api.paginator import GenericOffsetPaginator
 from sentry.api.serializers import EventSerializer, SimpleEventSerializer, serialize
 from sentry.api.utils import InvalidParams, get_date_range_from_params
+from sentry.eventstore.models import Event
 from sentry.exceptions import InvalidSearchQuery
 from sentry.search.utils import InvalidQuery, parse_query
 
@@ -86,9 +90,12 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):  # type: ignore
             "end": end if end else default_end,
         }
         referrer = f"api.group-events.{group.issue_category.name.lower()}"
+        use_builder = features.has(
+            "organizations:events-use-querybuilder", group.project.organization, actor=request.user
+        )
 
         direct_hit_resp = get_direct_hit_response(
-            request, query, params, f"{referrer}.direct-hit", group
+            request, query, params, f"{referrer}.direct-hit", group, use_builder
         )
         if direct_hit_resp:
             return direct_hit_resp
@@ -97,19 +104,43 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):  # type: ignore
             params["environment"] = [env.name for env in environments]
 
         try:
-            snuba_filter, dataset = get_filter_for_group(
-                request.GET.get("query", None), params, group
-            )
+            if use_builder:
+                snuba_query = get_query_builder_for_group(
+                    request.GET.get("query", ""), params, group
+                )
+            else:
+                snuba_filter, dataset = get_filter_for_group(
+                    request.GET.get("query", None), params, group
+                )
         except InvalidSearchQuery as e:
             raise ParseError(detail=str(e))
 
         full = request.GET.get("full", False)
-        data_fn = partial(
-            eventstore.get_events if full else eventstore.get_unfetched_events,
-            referrer=referrer,
-            filter=snuba_filter,
-            dataset=dataset,
-        )
+
+        def data_fn(offset: int, limit: int) -> Any:
+            if use_builder:
+                results = snuba_query.run_query(referrer=referrer)
+                results = [
+                    Event(
+                        event_id=evt["id"],
+                        project_id=evt["project.id"],
+                    )
+                    for evt in results["data"]
+                ]
+                if full:
+                    eventstore.bind_nodes(results)
+
+                return results
+            else:
+                if full:
+                    return eventstore.get_events(
+                        referrer=referrer, filter=snuba_filter, dataset=dataset
+                    )
+                else:
+                    return eventstore.get_unfetched_events(
+                        referrer=referrer, filter=snuba_filter, dataset=dataset
+                    )
+
         serializer = EventSerializer() if full else SimpleEventSerializer()
         return self.paginate(
             request=request,

+ 31 - 2
src/sentry/api/helpers/events.py

@@ -7,7 +7,9 @@ from rest_framework.response import Response
 
 from sentry import eventstore
 from sentry.api.serializers import serialize
+from sentry.eventstore.models import Event
 from sentry.issues.query import apply_performance_conditions
+from sentry.search.events.builder import QueryBuilder
 from sentry.search.events.filter import get_filter
 from sentry.snuba.dataset import Dataset
 from sentry.types.issues import GroupCategory
@@ -24,6 +26,7 @@ def get_direct_hit_response(
     snuba_params: Mapping[str, Any],
     referrer: str,
     group: Group,
+    use_builder: bool,
 ) -> Optional[Response]:
     """
     Checks whether a query is a direct hit for an event, and if so returns
@@ -31,8 +34,20 @@ def get_direct_hit_response(
     """
     event_id = normalize_event_id(query)
     if event_id:
-        snuba_filter, dataset = get_filter_for_group(f"id:{event_id}", snuba_params, group)
-        results = eventstore.get_events(referrer=referrer, filter=snuba_filter, dataset=dataset)
+        if use_builder:
+            snuba_query = get_query_builder_for_group(f"id:{event_id}", snuba_params, group)
+            results = snuba_query.run_query(referrer=referrer)
+            results = [
+                Event(
+                    event_id=event_id,
+                    project_id=evt["project.id"],
+                )
+                for evt in results["data"]
+            ]
+            eventstore.bind_nodes(results)
+        else:
+            snuba_filter, dataset = get_filter_for_group(f"id:{event_id}", snuba_params, group)
+            results = eventstore.get_events(referrer=referrer, filter=snuba_filter, dataset=dataset)
 
         if len(results) == 1:
             response = Response(serialize(results, request.user))
@@ -41,6 +56,20 @@ def get_direct_hit_response(
     return None
 
 
+def get_query_builder_for_group(
+    query: str, snuba_params: Mapping[str, Any], group: Group
+) -> QueryBuilder:
+    dataset = Dataset.Events
+    if group.issue_category == GroupCategory.PERFORMANCE:
+        dataset = Dataset.Transactions
+    return QueryBuilder(
+        dataset=dataset,
+        query=f"issue:{group.qualified_short_id} {query}",
+        params=snuba_params,
+        selected_columns=["id", "project.id"],
+    )
+
+
 def get_filter_for_group(
     query: str, snuba_params: Mapping[str, Any], group: Group
 ) -> Tuple[Filter, Dataset]:

+ 1 - 0
src/sentry/features/__init__.py

@@ -82,6 +82,7 @@ default_manager.add("organizations:discover-frontend-use-events-endpoint", Organ
 default_manager.add("organizations:discover-query-builder-as-landing-page", OrganizationFeature, True)
 default_manager.add("organizations:dynamic-sampling-deprecated", OrganizationFeature, True)
 default_manager.add("organizations:dynamic-sampling-demo", OrganizationFeature, True)
+default_manager.add("organizations:events-use-querybuilder", OrganizationFeature, True)
 default_manager.add("organizations:enterprise-perf", OrganizationFeature)
 default_manager.add("organizations:grouping-stacktrace-ui", OrganizationFeature, True)
 default_manager.add("organizations:grouping-title-ui", OrganizationFeature, True)

+ 30 - 19
tests/snuba/api/endpoints/test_group_events.py

@@ -15,6 +15,11 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
     def setUp(self):
         super().setUp()
         self.min_ago = before_now(minutes=1)
+        self.features = {}
+
+    def do_request(self, url):
+        with self.feature(self.features):
+            return self.client.get(url, format="json")
 
     def test_simple(self):
         self.login_as(user=self.user)
@@ -37,7 +42,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
         )
 
         url = f"/api/0/issues/{event_1.group.id}/events/"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert len(response.data) == 2
@@ -66,47 +71,47 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
             project_id=self.project.id,
         )
         url = f"/api/0/issues/{event_1.group.id}/events/"
-        response = self.client.get(url + "?query=foo:baz", format="json")
+        response = self.do_request(url + "?query=foo:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
         assert response.data[0]["eventID"] == str(event_1.event_id)
 
-        response = self.client.get(url + "?query=!foo:baz", format="json")
+        response = self.do_request(url + "?query=!foo:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
         assert response.data[0]["eventID"] == str(event_2.event_id)
 
-        response = self.client.get(url + "?query=bar:biz", format="json")
+        response = self.do_request(url + "?query=bar:biz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
         assert response.data[0]["eventID"] == str(event_2.event_id)
 
-        response = self.client.get(url + "?query=bar:biz%20foo:baz", format="json")
+        response = self.do_request(url + "?query=bar:biz%20foo:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 0
 
-        response = self.client.get(url + "?query=bar:buz%20foo:baz", format="json")
+        response = self.do_request(url + "?query=bar:buz%20foo:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
         assert response.data[0]["eventID"] == str(event_1.event_id)
 
-        response = self.client.get(url + "?query=bar:baz", format="json")
+        response = self.do_request(url + "?query=bar:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 0
 
-        response = self.client.get(url + "?query=a:b", format="json")
+        response = self.do_request(url + "?query=a:b")
         assert response.status_code == 200, response.content
         assert len(response.data) == 0
 
-        response = self.client.get(url + "?query=bar:b", format="json")
+        response = self.do_request(url + "?query=bar:b")
         assert response.status_code == 200, response.content
         assert len(response.data) == 0
 
-        response = self.client.get(url + "?query=bar:baz", format="json")
+        response = self.do_request(url + "?query=bar:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 0
 
-        response = self.client.get(url + "?query=!bar:baz", format="json")
+        response = self.do_request(url + "?query=!bar:baz")
         assert response.status_code == 200, response.content
         assert len(response.data) == 2
         assert {e["eventID"] for e in response.data} == {event_1.event_id, event_2.event_id}
@@ -130,7 +135,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
             project_id=self.project.id,
         )
         url = f"/api/0/issues/{event_1.group.id}/events/?query={event_1.event_id}"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
@@ -165,7 +170,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
 
         # Single Word Query
         url = f"/api/0/issues/{group.id}/events/?query={query_1}"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
@@ -173,7 +178,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
 
         # Multiple Word Query
         url = f"/api/0/issues/{group.id}/events/?query={query_2}"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert len(response.data) == 2
@@ -194,7 +199,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
             project_id=self.project.id,
         )
         url = f"/api/0/issues/{event_1.group.id}/events/?query=release:latest"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert len(response.data) == 1
@@ -218,7 +223,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
         (group_id,) = {e.group.id for e in events.values()}
 
         url = f"/api/0/issues/{group_id}/events/"
-        response = self.client.get(url + "?environment=production", format="json")
+        response = self.do_request(url + "?environment=production")
 
         assert response.status_code == 200, response.content
         assert set(map(lambda x: x["eventID"], response.data)) == {
@@ -233,7 +238,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
             str(event.event_id) for event in events.values()
         }
 
-        response = self.client.get(url + "?environment=invalid", format="json")
+        response = self.do_request(url + "?environment=invalid")
 
         assert response.status_code == 200, response.content
         assert response.data == []
@@ -350,7 +355,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
 
         for event in (event_1, event_2):
             url = f"/api/0/issues/{event.group.id}/events/"
-            response = self.client.get(url, format="json")
+            response = self.do_request(url)
             assert response.status_code == 200, response.content
             assert len(response.data) == 1, response.data
             assert list(map(lambda x: x["eventID"], response.data)) == [str(event.event_id)]
@@ -366,9 +371,15 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
         self.login_as(user=self.user)
 
         url = f"/api/0/issues/{event_1.groups[0].id}/events/"
-        response = self.client.get(url, format="json")
+        response = self.do_request(url)
 
         assert response.status_code == 200, response.content
         assert sorted(map(lambda x: x["eventID"], response.data)) == sorted(
             [str(event_1.event_id), str(event_2.event_id)]
         )
+
+
+class GroupEventsTestWithBuilder(GroupEventsTest):
+    def setUp(self):
+        super().setUp()
+        self.features["organizations:events-use-querybuilder"] = True