feat(discover): Zerofill queries that are grouped by time

src/sentry/api/endpoints/organization_discover_query.py

@@ -226,9 +226,10 @@ def handle_results(self, snuba_results, requested_query, projects):
                 ]
 
             for result in snuba_results['data']:
-                result['project.name'] = projects[result['project.id']]
-                if 'project.id' not in requested_query['selected_columns']:
-                    del result['project.id']
+                if 'project.id' in result:
+                    result['project.name'] = projects[result['project.id']]
+                    if 'project.id' not in requested_query['selected_columns']:
+                        del result['project.id']
 
         if 'project.name' in requested_query['groupby']:
             project_name_index = requested_query['groupby'].index('project.name')
@@ -241,9 +242,10 @@ def handle_results(self, snuba_results, requested_query, projects):
                 ]
 
             for result in snuba_results['data']:
-                result['project.name'] = projects[result['project.id']]
-                if 'project.id' not in requested_query['groupby']:
-                    del result['project.id']
+                if 'project.id' in result:
+                    result['project.name'] = projects[result['project.id']]
+                    if 'project.id' not in requested_query['groupby']:
+                        del result['project.id']
 
         # Convert snuba types to json types
         for col in snuba_results['meta']:

src/sentry/static/sentry/app/views/organizationDiscover/result/utils.jsx

@@ -82,7 +82,6 @@ export function getChartDataWithPercentages(data, query) {
  * @param {Array} data Data returned from Snuba
  * @param {Object} query Query state corresponding to data
  * @param {Object} [options] Options object
- * @param {Boolean} [options.assumeNullAsZero] (default: false) Assume null values as 0
  * @param {Boolean} [options.allSeries] (default: false) Return all series instead of top 10
  * @param {Object} [options.fieldLabelMap] (default: false) Maps value from Snuba to a defined label
  * @returns {Array}
@@ -114,8 +113,7 @@ export function getChartDataByDay(rawData, query, options = {}) {
     const dateIdx = dates.indexOf(formatDate(row.time));
 
     if (top10Series.has(key)) {
-      seriesHash[key][dateIdx].value =
-        options.assumeNullAsZero && row[aggregate] === null ? 0 : row[aggregate];
+      seriesHash[key][dateIdx].value = row[aggregate] === null ? 0 : row[aggregate];
     }
   });
 
@@ -161,7 +159,7 @@ function getEmptySeriesHash(seriesSet, dates, options = {}) {
 function getEmptySeries(dates, options) {
   return dates.map(date => {
     return {
-      value: options.assumeNullAsZero ? 0 : null,
+      value: 0,
       name: date,
     };
   });
@@ -171,7 +169,14 @@ function getEmptySeries(dates, options) {
 function getTopSeries(data, aggregate, limit = NUMBER_OF_SERIES_BY_DAY) {
   const allData = orderBy(data, ['time', aggregate], ['desc', 'desc']);
 
-  const orderedData = [...new Set(allData.map(row => row[CHART_KEY]))];
+  const orderedData = [
+    ...new Set(
+      allData
+        // `row` can be an empty time bucket, in which case it will have no `CHART_KEY` property
+        .filter(row => typeof row[CHART_KEY] !== 'undefined')
+        .map(row => row[CHART_KEY])
+    ),
+  ];
 
   return new Set(limit <= 0 ? orderedData : orderedData.slice(0, limit));
 }
@@ -182,6 +187,12 @@ function getDataWithKeys(data, query, options = {}) {
   const aggregate = aggregations[0][2];
 
   return data.map(row => {
+    // `row` can be an empty time bucket, in which case it has no value
+    // for `aggregate`
+    if (!row.hasOwnProperty(aggregate)) {
+      return row;
+    }
+
     const key = fields.length
       ? fields.map(field => getLabel(row[field], options)).join(',')
       : aggregate;

src/sentry/utils/snuba.py

@@ -216,6 +216,43 @@ def options_override(overrides):
 )
 
 
+epoch_naive = datetime(1970, 1, 1, tzinfo=None)
+
+
+def to_naive_timestamp(value):
+    """
+    Convert a time zone aware datetime to a POSIX timestamp (with fractional
+    component.)
+    """
+    return (value - epoch_naive).total_seconds()
+
+
+def zerofill(data, start, end, rollup, orderby):
+    rv = []
+    start = ((int(to_naive_timestamp(naiveify_datetime(start))) / rollup) * rollup) - rollup
+    end = ((int(to_naive_timestamp(naiveify_datetime(end))) / rollup) * rollup) + rollup
+    data_by_time = {}
+
+    if orderby.startswith('-'):
+        (end, start) = (start, end)
+        rollup = rollup * -1
+
+    for obj in data:
+        if obj['time'] in data_by_time:
+            data_by_time[obj['time']].append(obj)
+        else:
+            data_by_time[obj['time']] = [obj]
+
+    for key in six.moves.xrange(start, end, rollup):
+        if key in data_by_time and len(data_by_time[key]) > 0:
+            rv = rv + data_by_time[key]
+            data_by_time[key] = []
+        else:
+            rv.append({'time': key})
+
+    return rv
+
+
 def get_snuba_column_name(name):
     """
     Get corresponding Snuba column name from Sentry snuba map, if not found
@@ -306,6 +343,14 @@ def get_row(row):
 
     if len(translated_columns):
         result['data'] = [get_row(row) for row in result['data']]
+        if kwargs['rollup'] > 0:
+            result['data'] = zerofill(
+                result['data'],
+                kwargs['start'],
+                kwargs['end'],
+                kwargs['rollup'],
+                kwargs['orderby'],
+            )
 
     return result
 

tests/js/spec/views/organizationDiscover/result/utils.spec.jsx

@@ -148,23 +148,23 @@ describe('Utils', function() {
         {
           data: [
             {name: 1531094400000, value: 14},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 30},
           ],
           seriesName: 'python,SnubaError',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 8},
           ],
           seriesName: 'php,Exception',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 5},
           ],
           seriesName: 'javascript,Type Error',
@@ -173,7 +173,7 @@ describe('Utils', function() {
           data: [
             {name: 1531094400000, value: 6},
             {name: 1531180800000, value: 20},
-            {name: 1532070000000, value: null},
+            {name: 1532070000000, value: 0},
           ],
           seriesName: 'python,ZeroDivisionError',
         },
@@ -182,42 +182,75 @@ describe('Utils', function() {
       expect(getChartDataByDay(raw, query)).toEqual(expected);
     });
 
-    it('assumes null value as 0', function() {
+    it('returns chart data with zero filled dates', function() {
+      const zeroFilledRaw = [
+        {
+          'error.type': 'Type Error',
+          platform: 'javascript',
+          count: 5,
+          time: 1531465200,
+        },
+        {
+          'error.type': 'Exception',
+          platform: 'php',
+          count: 8,
+          time: 1531465200,
+        },
+        {
+          'error.type': 'SnubaError',
+          platform: 'python',
+          count: 30,
+          time: 1531465200,
+        },
+        {time: 1531378800},
+        {time: 1531292400},
+        ...raw.slice(-5),
+      ];
+
       const expected = [
         {
           data: [
             {name: 1531094400000, value: 14},
             {name: 1531180800000, value: 0},
-            {name: 1532070000000, value: 30},
+            {name: 1531292400000, value: 0},
+            {name: 1531378800000, value: 0},
+            {name: 1531465200000, value: 30},
           ],
           seriesName: 'python,SnubaError',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
             {name: 1531180800000, value: 0},
-            {name: 1532070000000, value: 8},
+            {name: 1531292400000, value: 0},
+            {name: 1531378800000, value: 0},
+            {name: 1531465200000, value: 8},
           ],
           seriesName: 'php,Exception',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
             {name: 1531180800000, value: 0},
-            {name: 1532070000000, value: 5},
+            {name: 1531292400000, value: 0},
+            {name: 1531378800000, value: 0},
+            {name: 1531465200000, value: 5},
           ],
           seriesName: 'javascript,Type Error',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
             {name: 1531180800000, value: 20},
-            {name: 1532070000000, value: 0},
+            {name: 1531292400000, value: 0},
+            {name: 1531378800000, value: 0},
+            {name: 1531465200000, value: 0},
           ],
           seriesName: 'python,ZeroDivisionError',
         },
       ];
-      expect(getChartDataByDay(raw, query, {assumeNullAsZero: true})).toEqual(expected);
+
+      expect(getChartDataByDay(zeroFilledRaw, query)).toEqual(expected);
     });
 
     it('shows only top 10 series by default', function() {
@@ -260,23 +293,23 @@ describe('Utils', function() {
         {
           data: [
             {name: 1531094400000, value: 14},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 30},
           ],
           seriesName: 'SNAKES,SnubaError',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 8},
           ],
           seriesName: 'PHP,Exception',
         },
         {
           data: [
             {name: 1531094400000, value: 6},
-            {name: 1531180800000, value: null},
+            {name: 1531180800000, value: 0},
             {name: 1532070000000, value: 5},
           ],
           seriesName: 'NOT JAVA,Type Error',
@@ -285,7 +318,7 @@ describe('Utils', function() {
           data: [
             {name: 1531094400000, value: 6},
             {name: 1531180800000, value: 20},
-            {name: 1532070000000, value: null},
+            {name: 1532070000000, value: 0},
           ],
           seriesName: 'SNAKES,ZeroDivisionError',
         },

tests/snuba/test_organization_discover_query.py

@@ -217,6 +217,44 @@ def test_groupby_project_name(self):
         assert(response.data['data'][0]['project.name']) == 'bar'
         assert(response.data['data'][0]['count']) == 1
 
+    def test_zerofilled_dates_when_rollup_relative(self):
+        with self.feature('organizations:discover'):
+            url = reverse('sentry-api-0-organization-discover-query', args=[self.org.slug])
+            response = self.client.post(url, {
+                'projects': [self.project.id],
+                'aggregations': [['count()', '', 'count']],
+                'fields': ['project.name'],
+                'groupby': ['time'],
+                'orderby': 'time',
+                'range': '5d',
+                'rollup': 86400,
+            })
+        assert response.status_code == 200, response.content
+        assert len(response.data['data']) == 7
+        assert(response.data['data'][6]['time']) > response.data['data'][5]['time']
+        assert(response.data['data'][6]['project.name']) == 'bar'
+        assert(response.data['data'][6]['count']) == 1
+
+    def test_zerofilled_dates_when_rollup_absolute(self):
+        with self.feature('organizations:discover'):
+            url = reverse('sentry-api-0-organization-discover-query', args=[self.org.slug])
+            response = self.client.post(url, {
+                'projects': [self.project.id],
+                'aggregations': [['count()', '', 'count']],
+                'fields': ['project.name'],
+                'groupby': ['time'],
+                'orderby': '-time',
+                'start': (datetime.now() - timedelta(seconds=60)).strftime('%Y-%m-%dT%H:%M:%S'),
+                'end': (datetime.now()).strftime('%Y-%m-%dT%H:%M:%S'),
+                'rollup': 10,
+            })
+
+        assert response.status_code == 200, response.content
+        assert len(response.data['data']) == 8
+        assert(response.data['data'][1]['time']) > response.data['data'][2]['time']
+        assert(response.data['data'][1]['project.name']) == 'bar'
+        assert(response.data['data'][1]['count']) == 1
+
     def test_uniq_project_name(self):
         with self.feature('organizations:discover'):
             url = reverse('sentry-api-0-organization-discover-query', args=[self.org.slug])