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

fix(snuba): Return empty results when user queries invalid time range for Group (#9680)

Brett Hoerner 6 лет назад
Родитель
Сommit
df2be1ae97
2 измененных файлов с 32 добавлено и 4 удалено
  1. 13 4
      src/sentry/utils/snuba.py
  2. 19 0
      tests/snuba/test_util.py

+ 13 - 4
src/sentry/utils/snuba.py

@@ -33,7 +33,11 @@ class SnubaError(Exception):
     pass
     pass
 
 
 
 
-class EntireQueryOutsideRetentionError(Exception):
+class QueryOutsideRetentionError(Exception):
+    pass
+
+
+class QueryOutsideGroupActivityError(Exception):
     pass
     pass
 
 
 
 
@@ -121,7 +125,7 @@ def raw_query(start, end, groupby=None, conditions=None, filter_keys=None,
     if retention:
     if retention:
         start = max(start, datetime.utcnow() - timedelta(days=retention))
         start = max(start, datetime.utcnow() - timedelta(days=retention))
         if start > end:
         if start > end:
-            raise EntireQueryOutsideRetentionError
+            raise QueryOutsideRetentionError
 
 
     # If the grouping, aggregation, or any of the conditions reference `issue`
     # If the grouping, aggregation, or any of the conditions reference `issue`
     # we need to fetch the issue definitions (issue -> fingerprint hashes)
     # we need to fetch the issue definitions (issue -> fingerprint hashes)
@@ -135,6 +139,12 @@ def raw_query(start, end, groupby=None, conditions=None, filter_keys=None,
 
 
     start, end = shrink_time_window(issues, start, end)
     start, end = shrink_time_window(issues, start, end)
 
 
+    # if `shrink_time_window` pushed `start` after `end` it means the user queried
+    # a Group for T1 to T2 when the group was only active for T3 to T4, so the query
+    # wouldn't return any results anyway
+    if start > end:
+        raise QueryOutsideGroupActivityError
+
     request = {k: v for k, v in six.iteritems({
     request = {k: v for k, v in six.iteritems({
         'from_date': start.isoformat(),
         'from_date': start.isoformat(),
         'to_date': end.isoformat(),
         'to_date': end.isoformat(),
@@ -195,8 +205,7 @@ def query(start, end, groupby, conditions=None, filter_keys=None,
             arrayjoin=arrayjoin, limit=limit, orderby=orderby, having=having,
             arrayjoin=arrayjoin, limit=limit, orderby=orderby, having=having,
             referrer=referrer, is_grouprelease=is_grouprelease, totals=totals
             referrer=referrer, is_grouprelease=is_grouprelease, totals=totals
         )
         )
-    except EntireQueryOutsideRetentionError:
-        # this exception could also bubble up to the caller instead
+    except (QueryOutsideRetentionError, QueryOutsideGroupActivityError):
         return OrderedDict()
         return OrderedDict()
 
 
     # Validate and scrub response, and translate snuba keys back to IDs
     # Validate and scrub response, and translate snuba keys back to IDs

+ 19 - 0
tests/snuba/test_util.py

@@ -1,7 +1,9 @@
 from __future__ import absolute_import
 from __future__ import absolute_import
 
 
+import pytest
 from datetime import datetime, timedelta
 from datetime import datetime, timedelta
 
 
+from sentry.models import GroupHash
 from sentry.testutils import TestCase
 from sentry.testutils import TestCase
 from sentry.utils import snuba
 from sentry.utils import snuba
 
 
@@ -60,7 +62,10 @@ class SnubaUtilTest(TestCase):
         group1.first_seen = now - timedelta(hours=1)
         group1.first_seen = now - timedelta(hours=1)
         group1.last_seen = now
         group1.last_seen = now
         group1.save()
         group1.save()
+        GroupHash.objects.create(project_id=group1.project_id, group=group1, hash='a' * 32)
+
         group2 = self.create_group()
         group2 = self.create_group()
+        GroupHash.objects.create(project_id=group2.project_id, group=group2, hash='b' * 32)
 
 
         # issues is a list like [(gid, pid, [(hash, tombstone_date), ...]), ...]
         # issues is a list like [(gid, pid, [(hash, tombstone_date), ...]), ...]
         issues = [(group1.id, group1.project_id, [('a' * 32, None)])]
         issues = [(group1.id, group1.project_id, [('a' * 32, None)])]
@@ -72,3 +77,17 @@ class SnubaUtilTest(TestCase):
             (group2.id, group2.project_id, [('b' * 32, None)]),
             (group2.id, group2.project_id, [('b' * 32, None)]),
         ]
         ]
         assert snuba.shrink_time_window(issues, year_ago, year_ahead) == (year_ago, year_ahead)
         assert snuba.shrink_time_window(issues, year_ago, year_ahead) == (year_ago, year_ahead)
+
+        with pytest.raises(snuba.QueryOutsideGroupActivityError):
+            # query a group for a time range before it had any activity
+            snuba.raw_query(
+                start=group1.first_seen - timedelta(days=1, hours=1),
+                end=group1.first_seen - timedelta(days=1),
+                filter_keys={
+                    'project_id': [group1.project_id],
+                    'issue': [group1.id],
+                },
+                aggregations=[
+                    ['count()', '', 'count'],
+                ],
+            )