diff --git a/src/sentry/api/endpoints/organization_discover_query.py b/src/sentry/api/endpoints/organization_discover_query.py index 9443aa78e697b1..56f04051701acc 100644 --- a/src/sentry/api/endpoints/organization_discover_query.py +++ b/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']: diff --git a/src/sentry/static/sentry/app/views/organizationDiscover/result/utils.jsx b/src/sentry/static/sentry/app/views/organizationDiscover/result/utils.jsx index 148362204a3e22..fa6d7b8b2ed4d2 100644 --- a/src/sentry/static/sentry/app/views/organizationDiscover/result/utils.jsx +++ b/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; diff --git a/src/sentry/utils/snuba.py b/src/sentry/utils/snuba.py index bf9ce33f7f6cbe..ae5ea54d2b97e4 100644 --- a/src/sentry/utils/snuba.py +++ b/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 diff --git a/tests/js/spec/views/organizationDiscover/result/utils.spec.jsx b/tests/js/spec/views/organizationDiscover/result/utils.spec.jsx index 530821332bd503..14ce73d543983d 100644 --- a/tests/js/spec/views/organizationDiscover/result/utils.spec.jsx +++ b/tests/js/spec/views/organizationDiscover/result/utils.spec.jsx @@ -148,7 +148,7 @@ describe('Utils', function() { { data: [ {name: 1531094400000, value: 14}, - {name: 1531180800000, value: null}, + {name: 1531180800000, value: 0}, {name: 1532070000000, value: 30}, ], seriesName: 'python,SnubaError', @@ -156,7 +156,7 @@ describe('Utils', function() { { data: [ {name: 1531094400000, value: 6}, - {name: 1531180800000, value: null}, + {name: 1531180800000, value: 0}, {name: 1532070000000, value: 8}, ], seriesName: 'php,Exception', @@ -164,7 +164,7 @@ describe('Utils', function() { { 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,13 +182,39 @@ 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', }, @@ -196,7 +222,9 @@ describe('Utils', function() { 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', }, @@ -204,7 +232,9 @@ describe('Utils', function() { 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', }, @@ -212,12 +242,15 @@ describe('Utils', function() { 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,7 +293,7 @@ describe('Utils', function() { { data: [ {name: 1531094400000, value: 14}, - {name: 1531180800000, value: null}, + {name: 1531180800000, value: 0}, {name: 1532070000000, value: 30}, ], seriesName: 'SNAKES,SnubaError', @@ -268,7 +301,7 @@ describe('Utils', function() { { data: [ {name: 1531094400000, value: 6}, - {name: 1531180800000, value: null}, + {name: 1531180800000, value: 0}, {name: 1532070000000, value: 8}, ], seriesName: 'PHP,Exception', @@ -276,7 +309,7 @@ describe('Utils', function() { { 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', }, diff --git a/tests/snuba/test_organization_discover_query.py b/tests/snuba/test_organization_discover_query.py index e293b729bb9bd0..79f328332e72aa 100644 --- a/tests/snuba/test_organization_discover_query.py +++ b/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])