Browse Source

chore(qureybuilder0): Remove use querybuilder flag from events (#41768)

- This has been running in prod for a week so far without any major
issues (minus missing pagination for a bit there)
William Mak 2 years ago
parent
commit
46e52e6550

+ 27 - 52
src/sentry/api/endpoints/group_events.py

@@ -8,16 +8,12 @@ from rest_framework.exceptions import ParseError
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import eventstore, features
+from sentry import eventstore
 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,
-    get_query_builder_for_group,
-)
+from sentry.api.helpers.events import get_direct_hit_response, 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
@@ -90,12 +86,9 @@ 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, use_builder
+            request, query, params, f"{referrer}.direct-hit", group
         )
         if direct_hit_resp:
             return direct_hit_resp
@@ -106,48 +99,30 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):  # type: ignore
         full = request.GET.get("full", False)
 
         def data_fn(offset: int, limit: int) -> Any:
-            if use_builder:
-                try:
-                    snuba_query = get_query_builder_for_group(
-                        request.GET.get("query", ""), params, group, limit=limit, offset=offset
-                    )
-                except InvalidSearchQuery as e:
-                    raise ParseError(detail=str(e))
-                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:
-                try:
-                    snuba_filter, dataset = get_filter_for_group(
-                        request.GET.get("query", None), params, group
-                    )
-                except InvalidSearchQuery as e:
-                    raise ParseError(detail=str(e))
-                if full:
-                    return eventstore.get_events(
-                        referrer=referrer,
-                        filter=snuba_filter,
-                        dataset=dataset,
-                        offset=offset,
-                        limit=limit,
-                    )
-                else:
-                    return eventstore.get_unfetched_events(
-                        referrer=referrer,
-                        filter=snuba_filter,
-                        dataset=dataset,
-                        offset=offset,
-                        limit=limit,
-                    )
+            try:
+                snuba_query = get_query_builder_for_group(
+                    request.GET.get("query", ""), params, group, limit=limit, offset=offset
+                )
+            except InvalidSearchQuery as e:
+                raise ParseError(detail=str(e))
+            results = snuba_query.run_query(referrer=referrer)
+            results = [
+                Event(
+                    event_id=evt["id"],
+                    project_id=evt["project.id"],
+                    snuba_data={
+                        "event_id": evt["id"],
+                        "group_id": evt["issue.id"],
+                        "project_id": evt["project.id"],
+                        "timestamp": evt["timestamp"],
+                    },
+                )
+                for evt in results["data"]
+            ]
+            if full:
+                eventstore.bind_nodes(results)
+
+            return results
 
         serializer = EventSerializer() if full else SimpleEventSerializer()
         return self.paginate(

+ 14 - 35
src/sentry/api/helpers/events.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any, Mapping, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Mapping, Optional
 
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -8,15 +8,12 @@ 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
 from sentry.utils.validators import normalize_event_id
 
 if TYPE_CHECKING:
-    from sentry.eventstore import Filter
     from sentry.models.group import Group
 
 
@@ -26,7 +23,6 @@ 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
@@ -34,22 +30,18 @@ def get_direct_hit_response(
     """
     event_id = normalize_event_id(query)
     if event_id:
-        if use_builder:
-            snuba_query = get_query_builder_for_group(
-                f"id:{event_id}", snuba_params, group, offset=0, limit=5
+        snuba_query = get_query_builder_for_group(
+            f"id:{event_id}", snuba_params, group, offset=0, limit=5
+        )
+        results = snuba_query.run_query(referrer=referrer)
+        results = [
+            Event(
+                event_id=event_id,
+                project_id=evt["project.id"],
             )
-            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)
+            for evt in results["data"]
+        ]
+        eventstore.bind_nodes(results)
 
         if len(results) == 1:
             response = Response(serialize(results, request.user))
@@ -68,21 +60,8 @@ def get_query_builder_for_group(
         dataset=dataset,
         query=f"issue:{group.qualified_short_id} {query}",
         params=snuba_params,
-        selected_columns=["id", "project.id"],
+        selected_columns=["id", "project.id", "issue.id", "timestamp"],
+        orderby=["-timestamp"],
         limit=limit,
         offset=offset,
     )
-
-
-def get_filter_for_group(
-    query: str, snuba_params: Mapping[str, Any], group: Group
-) -> Tuple[Filter, Dataset]:
-    snuba_filter = get_filter(query=query, params=snuba_params)
-    dataset = Dataset.Events
-    if group.issue_category == GroupCategory.ERROR:
-        snuba_filter.group_ids = [group.id]
-    elif group.issue_category == GroupCategory.PERFORMANCE:
-        dataset = Dataset.Transactions
-        apply_performance_conditions(snuba_filter.conditions, group)
-
-    return snuba_filter, dataset

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

@@ -82,7 +82,6 @@ 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)

+ 29 - 6
tests/snuba/api/endpoints/test_group_events.py

@@ -16,6 +16,7 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
     def setUp(self):
         super().setUp()
         self.min_ago = before_now(minutes=1)
+        self.two_min_ago = before_now(minutes=2)
         self.features = {}
 
     def do_request(self, url):
@@ -391,6 +392,34 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
         assert links["next"]["results"] == "true"
         assert len(response.data) == 1
 
+    def test_orderby(self):
+        self.login_as(user=self.user)
+
+        event = self.store_event(
+            data={
+                "fingerprint": ["group_1"],
+                "event_id": "a" * 32,
+                "message": "foo",
+                "timestamp": iso_format(self.min_ago),
+            },
+            project_id=self.project.id,
+        )
+        event = self.store_event(
+            data={
+                "fingerprint": ["group_1"],
+                "event_id": "b" * 32,
+                "message": "foo",
+                "timestamp": iso_format(self.two_min_ago),
+            },
+            project_id=self.project.id,
+        )
+
+        url = f"/api/0/issues/{event.group.id}/events/"
+        response = self.do_request(url)
+        assert len(response.data) == 2
+        assert response.data[0]["eventID"] == "a" * 32
+        assert response.data[1]["eventID"] == "b" * 32
+
     def test_perf_issue(self):
         event_data = load_data(
             "transaction",
@@ -408,9 +437,3 @@ class GroupEventsTest(APITestCase, SnubaTestCase):
         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