feat(api): Allow GroupEventEndpoint to be filtered by multiple environments and date range (APP-1013)

src/sentry/api/bases/organization.py

@@ -4,6 +4,7 @@ from rest_framework.exceptions import PermissionDenied
 
 from sentry.api.base import Endpoint
 from sentry.api.exceptions import ResourceDoesNotExist
+from sentry.api.helpers.environments import get_environments
 from sentry.api.permissions import SentryPermission
 from sentry.api.utils import (
     get_date_range_from_params,
@@ -11,7 +12,7 @@ from sentry.api.utils import (
 )
 from sentry.auth.superuser import is_active_superuser
 from sentry.models import (
-    ApiKey, Authenticator, Environment, Organization, OrganizationMemberTeam, Project,
+    ApiKey, Authenticator, Organization, OrganizationMemberTeam, Project,
     ProjectStatus, ReleaseProject,
 )
 from sentry.utils import auth
@@ -176,20 +177,7 @@ class OrganizationEndpoint(Endpoint):
         return projects
 
     def get_environments(self, request, organization):
-        requested_environments = set(request.GET.getlist('environment'))
-
-        if not requested_environments:
-            return []
-
-        environments = Environment.objects.filter(
-            organization_id=organization.id,
-            name__in=requested_environments,
-        )
-
-        if set(requested_environments) != set([e.name for e in environments]):
-            raise ResourceDoesNotExist
-
-        return list(environments)
+        return get_environments(request, organization)
 
     def get_filter_params(self, request, organization, date_filter_optional=False):
         """

src/sentry/api/endpoints/group_events.py

@@ -12,12 +12,17 @@ from functools32 import partial
 from sentry import options, quotas, tagstore
 from sentry.api.base import DocSection, EnvironmentMixin
 from sentry.api.bases import GroupEndpoint
+from sentry.api.exceptions import ResourceDoesNotExist
+from sentry.api.helpers.environments import get_environments
 from sentry.api.serializers.models.event import SnubaEvent
 from sentry.api.serializers import serialize
 from sentry.api.paginator import DateTimePaginator, GenericOffsetPaginator
-from sentry.models import Environment, Event, Group
-from sentry.search.utils import parse_query
-from sentry.search.utils import InvalidQuery
+from sentry.api.utils import get_date_range_from_params
+from sentry.models import Event, Group
+from sentry.search.utils import (
+    InvalidQuery,
+    parse_query,
+)
 from sentry.utils.apidocs import scenario, attach_scenarios
 from sentry.utils.validators import is_event_id
 from sentry.utils.snuba import raw_query
@@ -49,18 +54,26 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
         """
 
         try:
-            environment = self._get_environment(request, group)
-            query, tags = self._get_search_query_and_tags(request, group, environment)
+            environments = get_environments(request, group.project.organization)
+            query, tags = self._get_search_query_and_tags(
+                request,
+                group,
+                environments,
+            )
         except InvalidQuery as exc:
             return Response({'detail': six.text_type(exc)}, status=400)
-        except NoResults:
+        except (NoResults, ResourceDoesNotExist):
             return Response([])
 
-        use_snuba = options.get('snuba.events-queries.enabled')
+        use_snuba = (
+            request.GET.get('enable_snuba') == '1'
+            or options.get('snuba.events-queries.enabled')
+        )
         backend = self._get_events_snuba if use_snuba else self._get_events_legacy
-        return backend(request, group, environment, query, tags)
+        start, end = get_date_range_from_params(request.GET, optional=True)
+        return backend(request, group, environments, query, tags, start, end)
 
-    def _get_events_snuba(self, request, group, environment, query, tags):
+    def _get_events_snuba(self, request, group, environments, query, tags, start, end):
         conditions = []
         if query:
             msg_substr = ['positionCaseInsensitive', ['message', "'%s'" % (query,)]]
@@ -72,14 +85,18 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
                 conditions.append(message_condition)
 
         if tags:
-            conditions.extend([[u'tags[{}]'.format(k), '=', v] for (k, v) in tags.items()])
+            for tag_name, tag_val in tags.items():
+                operator = 'IN' if isinstance(tag_val, list) else '='
+                conditions.append([u'tags[{}]'.format(tag_name), operator, tag_val])
+
+        default_end = timezone.now()
+        default_start = default_end - timedelta(days=90)
 
-        now = timezone.now()
         data_fn = partial(
             # extract 'data' from raw_query result
             lambda *args, **kwargs: raw_query(*args, **kwargs)['data'],
-            start=now - timedelta(days=90),
-            end=now,
+            start=max(start, default_start) if start else default_start,
+            end=min(end, default_end) if end else default_end,
             conditions=conditions,
             filter_keys={
                 'project_id': [group.project_id],
@@ -97,7 +114,16 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             paginator=GenericOffsetPaginator(data_fn=data_fn)
         )
 
-    def _get_events_legacy(self, request, group, environment, query, tags):
+    def _get_events_legacy(
+        self,
+        request,
+        group,
+        environments,
+        query,
+        tags,
+        start,
+        end,
+    ):
         events = Event.objects.filter(group_id=group.id)
 
         if query:
@@ -112,8 +138,10 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             event_filter = tagstore.get_group_event_filter(
                 group.project_id,
                 group.id,
-                environment.id if environment is not None else None,
+                [env.id for env in environments],
                 tags,
+                start,
+                end,
             )
 
             if not event_filter:
@@ -121,6 +149,12 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
 
             events = events.filter(**event_filter)
 
+        # Filter start/end here in case we didn't filter by tags at all
+        if start:
+            events = events.filter(datetime__gte=start)
+        if end:
+            events = events.filter(datetime__lte=end)
+
         # filter out events which are beyond the retention period
         retention = quotas.get_event_retention(organization=group.project.organization)
         if retention:
@@ -136,16 +170,7 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             paginator_cls=DateTimePaginator,
         )
 
-    def _get_environment(self, request, group):
-        try:
-            return self._get_environment_from_request(
-                request,
-                group.project.organization_id,
-            )
-        except Environment.DoesNotExist:
-            raise NoResults
-
-    def _get_search_query_and_tags(self, request, group, environment=None):
+    def _get_search_query_and_tags(self, request, group, environments=None):
         raw_query = request.GET.get('query')
 
         if raw_query:
@@ -156,15 +181,22 @@ class GroupEventsEndpoint(GroupEndpoint, EnvironmentMixin):
             query = None
             tags = {}
 
-        if environment is not None:
-            if 'environment' in tags and tags['environment'] != environment.name:
-                # An event can only be associated with a single
-                # environment, so if the environment associated with
-                # the request is different than the environment
-                # provided as a tag lookup, the query cannot contain
-                # any valid results.
-                raise NoResults
+        if environments:
+            env_names = set(env.name for env in environments)
+            if 'environment' in tags:
+                # If a single environment was passed as part of the query, then
+                # we'll just search for that individual environment in this
+                # query, even if more are selected.
+                if tags['environment'] not in env_names:
+                    # An event can only be associated with a single
+                    # environment, so if the environments associated with
+                    # the request don't contain the environment provided as a
+                    # tag lookup, the query cannot contain any valid results.
+                    raise NoResults
             else:
-                tags['environment'] = environment.name
+                # XXX: Handle legacy backends here. Just store environment as a
+                # single tag if we only have one so that we don't break existing
+                # usage.
+                tags['environment'] = list(env_names) if len(env_names) > 1 else env_names.pop()
 
         return query, tags

src/sentry/api/helpers/environments.py

@@ -0,0 +1,21 @@
+from __future__ import absolute_import
+
+from sentry.api.exceptions import ResourceDoesNotExist
+from sentry.models import Environment
+
+
+def get_environments(request, organization):
+    requested_environments = set(request.GET.getlist('environment'))
+
+    if not requested_environments:
+        return []
+
+    environments = list(Environment.objects.filter(
+        organization_id=organization.id,
+        name__in=requested_environments,
+    ))
+
+    if set(requested_environments) != set([e.name for e in environments]):
+        raise ResourceDoesNotExist
+
+    return environments

src/sentry/tagstore/base.py

@@ -304,7 +304,7 @@ class TagStorage(Service):
         """
         raise NotImplementedError
 
-    def get_group_event_filter(self, project_id, group_id, environment_id, tags):
+    def get_group_event_filter(self, project_id, group_id, environment_ids, tags, start, end):
         """
         >>> get_group_event_filter(1, 2, 3, {'key1': 'value1', 'key2': 'value2'})
         """

src/sentry/tagstore/legacy/backend.py

@@ -442,7 +442,7 @@ class LegacyTagStorage(TagStorage):
                     },
                     extra=extra)
 
-    def get_group_event_filter(self, project_id, group_id, environment_id, tags):
+    def get_group_event_filter(self, project_id, group_id, environment_ids, tags, start, end):
         tagkeys = dict(
             models.TagKey.objects.filter(
                 project_id=project_id,

src/sentry/tagstore/snuba/backend.py

@@ -654,16 +654,22 @@ class SnubaTagStorage(TagStorage):
         # search backend.
         raise NotImplementedError
 
-    def get_group_event_filter(self, project_id, group_id, environment_id, tags):
-        start, end = self.get_time_range()
+    def get_group_event_filter(self, project_id, group_id, environment_ids, tags, start, end):
+        default_start, default_end = self.get_time_range()
+        start = max(start, default_start) if start else default_start
+        end = min(end, default_end) if end else default_end
+
         filters = {
             'project_id': [project_id],
             'issue': [group_id],
         }
-        if environment_id:
-            filters['environment'] = [environment_id]
+        if environment_ids:
+            filters['environment'] = environment_ids
 
-        conditions = [[u'tags[{}]'.format(k), '=', v] for (k, v) in tags.items()]
+        conditions = []
+        for tag_name, tag_val in tags.items():
+            operator = 'IN' if isinstance(tag_val, list) else '='
+            conditions.append([u'tags[{}]'.format(tag_name), operator, tag_val])
 
         result = snuba.raw_query(start, end, selected_columns=['event_id'],
                                  conditions=conditions, orderby='-timestamp', filter_keys=filters,

src/sentry/tagstore/v2/backend.py

@@ -663,11 +663,18 @@ class V2TagStorage(TagStorage):
                         },
                         extra=extra)
 
-    def get_group_event_filter(self, project_id, group_id, environment_id, tags):
+    def get_group_event_filter(self, project_id, group_id, environment_ids, tags, start, end):
         # NOTE: `environment_id=None` needs to be filtered differently in this method.
         # EventTag never has NULL `environment_id` fields (individual Events always have an environment),
         # and so `environment_id=None` needs to query EventTag for *all* environments (except, ironically
         # the aggregate environment).
+        if environment_ids:
+            # only the snuba backend supports multi env
+            if len(environment_ids) > 1:
+                raise NotImplementedError
+            environment_id = environment_ids[0]
+        else:
+            environment_id = None
 
         if environment_id is None:
             # filter for all 'real' environments
@@ -707,12 +714,19 @@ class V2TagStorage(TagStorage):
         # Django doesnt support union, so we limit results and try to find
         # reasonable matches
 
+        date_filters = Q()
+        if start:
+            date_filters &= Q(date_added__gte=start)
+        if end:
+            date_filters &= Q(date_added__lte=end)
+
         # get initial matches to start the filter
         kv_pairs = tag_lookups.pop()
         matches = list(
             models.EventTag.objects.filter(
                 reduce(or_, (Q(key_id=k, value_id=v)
                              for k, v in kv_pairs)),
+                date_filters,
                 project_id=project_id,
                 group_id=group_id,
             ).values_list('event_id', flat=True)[:1000]
@@ -725,6 +739,7 @@ class V2TagStorage(TagStorage):
                 models.EventTag.objects.filter(
                     reduce(or_, (Q(key_id=k, value_id=v)
                                  for k, v in kv_pairs)),
+                    date_filters,
                     project_id=project_id,
                     group_id=group_id,
                     event_id__in=matches,

tests/sentry/api/endpoints/test_group_events.py

@@ -4,6 +4,9 @@ import six
 
 from datetime import timedelta
 from django.utils import timezone
+from freezegun import freeze_time
+from mock import patch
+from rest_framework.response import Response
 
 from sentry import options, tagstore
 from sentry.models import Environment
@@ -258,3 +261,58 @@ class GroupEventsTest(APITestCase):
                 six.text_type(event_2.id),
             ]
         )
+
+    @freeze_time()
+    def test_date_filters(self):
+        self.login_as(user=self.user)
+
+        project = self.create_project()
+        group = self.create_group(project=project)
+        event_1 = self.create_event(
+            'a' * 32,
+            group=group,
+            datetime=timezone.now() - timedelta(days=2),
+        )
+        event_2 = self.create_event('b' * 32, group=group)
+
+        response = self.client.get(
+            u'/api/0/issues/{}/events/'.format(group.id),
+            data={
+                'statsPeriod': '3d',
+            },
+        )
+
+        assert response.status_code == 200, response.content
+        assert len(response.data) == 2
+        assert sorted(map(lambda x: x['id'], response.data)) == sorted(
+            [
+                six.text_type(event_1.id),
+                six.text_type(event_2.id),
+            ]
+        )
+
+        response = self.client.get(
+            u'/api/0/issues/{}/events/'.format(group.id),
+            data={
+                'statsPeriod': '1d',
+            },
+        )
+
+        assert response.status_code == 200, response.content
+        assert len(response.data) == 1
+        assert response.data[0]['id'] == six.text_type(event_2.id)
+
+    def test_force_snuba(self):
+        self.login_as(user=self.user)
+        project = self.create_project()
+        group = self.create_group(project=project)
+        with patch('sentry.api.endpoints.group_events.GroupEventsEndpoint._get_events_snuba') as get_events_snuba:
+            get_events_snuba.return_value = Response([])
+            self.client.get(
+                u'/api/0/issues/{}/events/'.format(group.id),
+                data={
+                    'statsPeriod': '3d',
+                    'enable_snuba': '1',
+                },
+            )
+            assert get_events_snuba.call_count == 1

tests/sentry/tagstore/v2/test_backend.py

@@ -4,7 +4,11 @@ import os
 import pytest
 
 from collections import OrderedDict
-from datetime import datetime
+from datetime import (
+    datetime,
+    timedelta,
+)
+from django.utils import timezone
 
 from sentry.search.base import ANY
 from sentry.testutils import TestCase
@@ -434,13 +438,14 @@ class TagStorage(TestCase):
         }
 
         # 2 events with the same tags
-        for event in (self.proj1group1event1, self.proj1group1event2):
+        for i, event in enumerate((self.proj1group1event1, self.proj1group1event2)):
             self.ts.create_event_tags(
                 project_id=self.proj1.id,
                 group_id=self.proj1group1.id,
                 environment_id=self.proj1env1.id,
                 event_id=event.id,
                 tags=tags.items(),
+                date_added=timezone.now() - timedelta(hours=i)
             )
 
         different_tags = {
@@ -461,10 +466,40 @@ class TagStorage(TestCase):
         assert self.ts.get_group_event_filter(
             self.proj1.id,
             self.proj1group1.id,
-            self.proj1env1.id,
-            tags
+            [self.proj1env1.id],
+            tags,
+            None,
+            None,
         ) == {'id__in': set([self.proj1group1event1.id, self.proj1group1event2.id])}
 
+        assert self.ts.get_group_event_filter(
+            self.proj1.id,
+            self.proj1group1.id,
+            [self.proj1env1.id],
+            tags,
+            timezone.now() - timedelta(minutes=30),
+            None,
+        ) == {'id__in': set([self.proj1group1event1.id])}
+
+        assert self.ts.get_group_event_filter(
+            self.proj1.id,
+            self.proj1group1.id,
+            [self.proj1env1.id],
+            tags,
+            None,
+            timezone.now() - timedelta(minutes=30),
+        ) == {'id__in': set([self.proj1group1event2.id])}
+
+        with pytest.raises(NotImplementedError):
+            self.ts.get_group_event_filter(
+                self.proj1.id,
+                self.proj1group1.id,
+                [self.proj1env1.id, self.proj1env2.id],
+                tags,
+                None,
+                None,
+            )
+
     def test_get_groups_user_counts(self):
         k1, _ = self.ts.get_or_create_group_tag_key(
             self.proj1.id,

tests/sentry/tasks/post_process/tests.py

@@ -244,8 +244,10 @@ class IndexEventTagsTest(TestCase):
         assert tagstore.get_group_event_filter(
             self.project.id,
             group.id,
-            self.environment.id,
+            [self.environment.id],
             {'foo': 'bar', 'biz': 'baz'},
+            None,
+            None,
         ) == {'id__in': set([event.id])}
 
         # ensure it safely handles repeat runs
@@ -262,6 +264,8 @@ class IndexEventTagsTest(TestCase):
         assert tagstore.get_group_event_filter(
             self.project.id,
             group.id,
-            self.environment.id,
+            [self.environment.id],
             {'foo': 'bar', 'biz': 'baz'},
+            None,
+            None,
         ) == {'id__in': set([event.id])}