ref(eventstore): Simplify eventstore interface take 2 (#14952)

This is a second attempt at simplifying the eventstore interface to
accept a single filter argument instead of the snuba_args dict. A filter
consists of start and end timestamp, a list of conditions and any project,
group and event IDs for filtering.

Ideally I think we would not distinguish the project, group and event
filtering from regular conditions and do away with the concept of
filter_keys however since we perform a number of additional steps on
these keys we need to keep these separate.

V1 - https://github.com/getsentry/sentry/pull/14339

src/sentry/api/bases/organization_events.py

@@ -1,12 +1,11 @@
 from __future__ import absolute_import
 
 from rest_framework.exceptions import PermissionDenied
-from copy import copy
 
-from sentry import eventstore, features
+from sentry import eventstore
 from sentry.api.bases import OrganizationEndpoint, OrganizationEventsError
 from sentry.api.event_search import (
-    get_snuba_query_args,
+    get_filter,
     resolve_field_list,
     InvalidSearchQuery,
     get_reference_event_conditions,
@@ -24,10 +23,17 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
     def get_snuba_query_args(self, request, organization, params):
         query = request.GET.get("query")
         try:
-            snuba_args = get_snuba_query_args(query=query, params=params)
+            filter = get_filter(query, params)
         except InvalidSearchQuery as exc:
             raise OrganizationEventsError(exc.message)
 
+        snuba_args = {
+            "start": filter.start,
+            "end": filter.end,
+            "conditions": filter.conditions,
+            "filter_keys": filter.filter_keys,
+        }
+
         sort = request.GET.getlist("sort")
         if sort:
             snuba_args["orderby"] = sort
@@ -57,14 +63,6 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
                 snuba_args, reference_event_id
             )
 
-        # TODO(lb): remove once boolean search is fully functional
-        has_boolean_op_flag = features.has(
-            "organizations:boolean-search", organization, actor=request.user
-        )
-        if snuba_args.pop("has_boolean_terms", False) and not has_boolean_op_flag:
-            raise OrganizationEventsError(
-                "Boolean search operator OR and AND not allowed in this search."
-            )
         return snuba_args
 
     def get_snuba_query_args_legacy(self, request, organization):
@@ -90,18 +88,16 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
 
         query = request.GET.get("query")
         try:
-            snuba_args = get_snuba_query_args(query=query, params=params)
+            _filter = get_filter(query, params)
         except InvalidSearchQuery as exc:
             raise OrganizationEventsError(exc.message)
 
-        # TODO(lb): remove once boolean search is fully functional
-        has_boolean_op_flag = features.has(
-            "organizations:boolean-search", organization, actor=request.user
-        )
-        if snuba_args.pop("has_boolean_terms", False) and not has_boolean_op_flag:
-            raise OrganizationEventsError(
-                "Boolean search operator OR and AND not allowed in this search."
-            )
+        snuba_args = {
+            "start": _filter.start,
+            "end": _filter.end,
+            "conditions": _filter.conditions,
+            "filter_keys": _filter.filter_keys,
+        }
 
         # 'legacy' endpoints cannot access transactions dataset.
         # as they often have assumptions about which columns are returned.
@@ -110,6 +106,7 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
             raise OrganizationEventsError(
                 "Invalid query. You cannot reference non-events data in this endpoint."
             )
+
         return snuba_args
 
     def next_event_id(self, snuba_args, event):
@@ -117,10 +114,7 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
         Returns the next event ID if there is a subsequent event matching the
         conditions provided. Ignores the project_id.
         """
-        conditions = self._apply_start_and_end(snuba_args)
-        next_event = eventstore.get_next_event_id(
-            event, conditions=conditions, filter_keys=snuba_args["filter_keys"]
-        )
+        next_event = eventstore.get_next_event_id(event, filter=self._get_filter(snuba_args))
 
         if next_event:
             return next_event[1]
@@ -130,21 +124,19 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
         Returns the previous event ID if there is a previous event matching the
         conditions provided. Ignores the project_id.
         """
-        conditions = self._apply_start_and_end(snuba_args)
-        prev_event = eventstore.get_prev_event_id(
-            event, conditions=conditions, filter_keys=snuba_args["filter_keys"]
-        )
+        prev_event = eventstore.get_prev_event_id(event, filter=self._get_filter(snuba_args))
 
         if prev_event:
             return prev_event[1]
 
-    def _apply_start_and_end(self, snuba_args):
-        conditions = copy(snuba_args["conditions"])
-        if "start" in snuba_args:
-            conditions.append(["timestamp", ">=", snuba_args["start"]])
-        if "end" in snuba_args:
-            conditions.append(["timestamp", "<=", snuba_args["end"]])
-        return conditions
+    def _get_filter(self, snuba_args):
+        return eventstore.Filter(
+            conditions=snuba_args["conditions"],
+            start=snuba_args.get("start", None),
+            end=snuba_args.get("end", None),
+            project_ids=snuba_args["filter_keys"].get("project_id", None),
+            group_ids=snuba_args["filter_keys"].get("issue", None),
+        )
 
     def oldest_event_id(self, snuba_args, event):
         """

src/sentry/api/endpoints/group_events.py

@@ -8,10 +8,10 @@ from rest_framework.response import Response
 from functools import partial
 
 
-from sentry import eventstore, features
+from sentry import eventstore
 from sentry.api.base import DocSection, EnvironmentMixin
 from sentry.api.bases import GroupEndpoint
-from sentry.api.event_search import get_snuba_query_args
+from sentry.api.event_search import get_filter
 from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.helpers.environments import get_environments
 from sentry.api.helpers.events import get_direct_hit_response
@@ -86,17 +86,7 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             params["environment"] = [env.name for env in environments]
 
         full = request.GET.get("full", False)
-        snuba_args = get_snuba_query_args(request.GET.get("query", None), params)
-
-        # TODO(lb): remove once boolean search is fully functional
-        if snuba_args:
-            has_boolean_op_flag = features.has(
-                "organizations:boolean-search", group.project.organization, actor=request.user
-            )
-            if snuba_args.pop("has_boolean_terms", False) and not has_boolean_op_flag:
-                raise GroupEventsError(
-                    "Boolean search operator OR and AND not allowed in this search."
-                )
+        snuba_filter = get_filter(request.GET.get("query", None), params)
 
         snuba_cols = None if full else eventstore.full_columns
 
@@ -104,7 +94,7 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             eventstore.get_events,
             additional_columns=snuba_cols,
             referrer="api.group-events",
-            **snuba_args
+            filter=snuba_filter,
         )
 
         serializer = EventSerializer() if full else SimpleEventSerializer()

src/sentry/api/endpoints/organization_eventid.py

@@ -47,7 +47,9 @@ class EventIdLookupEndpoint(OrganizationEndpoint):
 
         try:
             event = eventstore.get_events(
-                filter_keys={"project_id": project_slugs_by_id.keys(), "event_id": event_id},
+                filter=eventstore.Filter(
+                    project_ids=project_slugs_by_id.keys(), event_ids=[event_id]
+                ),
                 limit=1,
             )[0]
         except IndexError:

src/sentry/api/endpoints/organization_events.py

@@ -50,7 +50,13 @@ class OrganizationEventsEndpoint(OrganizationEventsEndpointBase):
                 eventstore.get_events,
                 additional_columns=cols,
                 referrer="api.organization-events",
-                **snuba_args
+                filter=eventstore.Filter(
+                    start=snuba_args["start"],
+                    end=snuba_args["end"],
+                    conditions=snuba_args["conditions"],
+                    project_ids=snuba_args["filter_keys"].get("project_id", None),
+                    group_ids=snuba_args["filter_keys"].get("issue", None),
+                ),
             )
 
         serializer = EventSerializer() if full else SimpleEventSerializer()

src/sentry/api/endpoints/project_event_details.py

@@ -60,15 +60,13 @@ class ProjectEventDetailsEndpoint(ProjectEndpoint):
             if requested_environments:
                 conditions.append(["environment", "IN", requested_environments])
 
-            filter_keys = {"project_id": [event.project_id], "issue": [event.group_id]}
-
-            next_event = eventstore.get_next_event_id(
-                event, conditions=conditions, filter_keys=filter_keys
+            filter = eventstore.Filter(
+                conditions=conditions, project_ids=[event.project_id], group_ids=[event.group_id]
             )
 
-            prev_event = eventstore.get_prev_event_id(
-                event, conditions=conditions, filter_keys=filter_keys
-            )
+            next_event = eventstore.get_next_event_id(event, filter=filter)
+
+            prev_event = eventstore.get_prev_event_id(event, filter=filter)
 
             next_event_id = next_event[1] if next_event else None
             prev_event_id = prev_event[1] if prev_event else None

src/sentry/api/endpoints/project_events.py

@@ -51,8 +51,7 @@ class ProjectEventsEndpoint(ProjectEndpoint):
 
         data_fn = partial(
             eventstore.get_events,
-            conditions=conditions,
-            filter_keys={"project_id": [project.id]},
+            filter=eventstore.Filter(conditions=conditions, project_ids=[project.id]),
             additional_columns=cols,
             referrer="api.project-events",
         )

src/sentry/api/event_search.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import
 
 import re
-from collections import namedtuple, defaultdict
+from collections import namedtuple
 from copy import deepcopy
 from datetime import datetime
 
@@ -624,7 +624,11 @@ def convert_search_filter_to_snuba_query(search_filter):
             return condition
 
 
-def get_snuba_query_args(query=None, params=None):
+def get_filter(query=None, params=None):
+    """
+    Returns an eventstore filter given the search text provided by the user and
+    URL params
+    """
     # NOTE: this function assumes project permissions check already happened
     parsed_terms = []
     if query is not None:
@@ -637,7 +641,7 @@ def get_snuba_query_args(query=None, params=None):
     if params is not None:
         parsed_terms.extend(convert_endpoint_params(params))
 
-    kwargs = {"conditions": [], "filter_keys": defaultdict(list)}
+    kwargs = {"start": None, "end": None, "conditions": [], "project_ids": [], "group_ids": []}
 
     projects = {}
     has_project_term = any(
@@ -658,18 +662,19 @@ def get_snuba_query_args(query=None, params=None):
             elif snuba_name in ("start", "end"):
                 kwargs[snuba_name] = term.value.value
             elif snuba_name in ("project_id", "issue"):
+                if snuba_name == "issue":
+                    snuba_name = "group_ids"
+                if snuba_name == "project_id":
+                    snuba_name = "project_ids"
                 value = term.value.value
                 if isinstance(value, int):
                     value = [value]
-                kwargs["filter_keys"][snuba_name].extend(value)
+                kwargs[snuba_name].extend(value)
             else:
                 converted_filter = convert_search_filter_to_snuba_query(term)
                 kwargs["conditions"].append(converted_filter)
-        else:  # SearchBoolean
-            # TODO(lb): remove when boolean terms fully functional
-            kwargs["has_boolean_terms"] = True
-            kwargs["conditions"].append(convert_search_boolean_to_snuba_query(term))
-    return kwargs
+
+    return eventstore.Filter(**kwargs)
 
 
 FIELD_ALIASES = {

src/sentry/api/helpers/events.py

@@ -3,7 +3,7 @@ from __future__ import absolute_import
 from rest_framework.response import Response
 
 from sentry import eventstore
-from sentry.api.event_search import get_snuba_query_args
+from sentry.api.event_search import get_filter
 from sentry.utils.validators import normalize_event_id
 from sentry.api.serializers import serialize
 
@@ -15,9 +15,9 @@ def get_direct_hit_response(request, query, snuba_params, referrer):
     """
     event_id = normalize_event_id(query)
     if event_id:
-        snuba_args = get_snuba_query_args(query=u"id:{}".format(event_id), params=snuba_params)
+        snuba_filter = get_filter(query=u"id:{}".format(event_id), params=snuba_params)
 
-        results = eventstore.get_events(referrer=referrer, **snuba_args)
+        results = eventstore.get_events(referrer=referrer, filter=snuba_filter)
 
         if len(results) == 1:
             response = Response(serialize(results, request.user))

src/sentry/eventstore/__init__.py

@@ -2,7 +2,7 @@ from __future__ import absolute_import
 
 from sentry.utils.services import LazyServiceWrapper
 
-from .base import EventStorage, Columns  # NOQA
+from .base import EventStorage, Columns, Filter  # NOQA
 
 backend = LazyServiceWrapper(
     EventStorage, "sentry.eventstore.snuba.SnubaEventStorage", {}, metrics_path="eventstore"

src/sentry/eventstore/base.py

@@ -66,6 +66,54 @@ class Columns(Enum):
     CONTEXTS_VALUE = "contexts.value"
 
 
+class Filter(object):
+    """
+    A set of conditions, start/end times and project, group and event ID sets
+    used to restrict the results of a Snuba query.
+
+    start (DateTime): Start datetime - default None
+    end (DateTime): Start datetime - default None
+    conditions (Sequence[Sequence[str, str, Any]]): List of conditions to fetch - default None
+    project_ids (Sequence[int]): List of project IDs to fetch - default None
+    group_ids (Sequence[int]): List of group IDs to fetch - defualt None
+    event_ids (Sequence[int]): List of event IDs to fetch - default None
+    """
+
+    def __init__(
+        self,
+        start=None,
+        end=None,
+        conditions=None,
+        project_ids=None,
+        group_ids=None,
+        event_ids=None,
+    ):
+        self.start = start
+        self.end = end
+        self.conditions = conditions
+        self.project_ids = project_ids
+        self.group_ids = group_ids
+        self.event_ids = event_ids
+
+    @property
+    def filter_keys(self):
+        """
+        Get filter_keys value required for raw snuba query
+        """
+        filter_keys = {}
+
+        if self.project_ids:
+            filter_keys["project_id"] = self.project_ids
+
+        if self.group_ids:
+            filter_keys["issue"] = self.group_ids
+
+        if self.event_ids:
+            filter_keys["event_id"] = self.event_ids
+
+        return filter_keys
+
+
 class EventStorage(Service):
     __all__ = (
         "minimal_columns",
@@ -102,60 +150,50 @@ class EventStorage(Service):
         Columns.USERNAME,
     ]
 
-    def get_event_by_id(self, project_id, event_id, additional_columns):
-        """
-        Gets a single event given a project_id and event_id.
-
-        Keyword arguments:
-        project_id (int): Project ID
-        event_id (str): Event ID
-        additional_columns: (Sequence[Column]) - List of addition columns to fetch - default None
-        """
-        raise NotImplementedError
-
-    def get_events(
-        self, start, end, additional_columns, conditions, filter_keys, orderby, limit, offset
-    ):
+    def get_events(self, filter, additional_columns, orderby, limit, offset, referrer):
         """
         Fetches a list of events given a set of criteria.
 
-        Keyword arguments:
-        start (DateTime): Start datetime - default datetime.utcfromtimestamp(0)
-        end (DateTime): End datetime - default datetime.utcnow()
+        Arguments:
+        filter (Filter): Filter
         additional_columns (Sequence[Column]): List of additional columns to fetch - default None
-        conditions (Sequence[Sequence[str, str, Any]]): List of conditions to fetch - default None
-        filter_keys (Mapping[str, Any]): Filter keys - default None
         orderby (Sequence[str]): List of fields to order by - default ['-time', '-event_id']
         limit (int): Query limit - default 100
         offset (int): Query offset - default 0
+        referrer (string): Referrer - default "eventstore.get_events"
         """
         raise NotImplementedError
 
-    def get_next_event_id(self, event, conditions, filter_keys):
+    def get_event_by_id(self, project_id, event_id, additional_columns):
+        """
+        Gets a single event given a project_id and event_id.
+
+        Arguments:
+        project_id (int): Project ID
+        event_id (str): Event ID
+        additional_columns: (Sequence[Column]) - List of addition columns to fetch - default None
+        """
+        raise NotImplementedError
+
+    def get_next_event_id(self, event, filter):
         """
         Gets the next event given a current event and some conditions/filters.
         Returns a tuple of (project_id, event_id)
 
         Arguments:
         event (Event): Event object
-
-        Keyword arguments:
-        conditions (Sequence[Sequence[str, str, Any]]): List of conditions - default None
-        filter_keys (Mapping[str, Any]): Filter keys - default None
+        filter (Filter): Filter
         """
         raise NotImplementedError
 
-    def get_prev_event_id(self, event, conditions, filter_keys):
+    def get_prev_event_id(self, event, filter):
         """
         Gets the previous event given a current event and some conditions/filters.
         Returns a tuple of (project_id, event_id)
 
         Arguments:
         event (Event): Event object
-
-        Keyword arguments:
-        conditions (Sequence[Sequence[str, str, Any]]): List of conditions - default None
-        filter_keys (Mapping[str, Any]): Filter keys - default None
+        filter (Filter): Filter
         """
         raise NotImplementedError