diff --git a/src/sentry/api/bases/organization_events.py b/src/sentry/api/bases/organization_events.py index 0f54759c910681..1b078de2c3f72d 100644 --- a/src/sentry/api/bases/organization_events.py +++ b/src/sentry/api/bases/organization_events.py @@ -1,38 +1,19 @@ from __future__ import absolute_import -from copy import deepcopy -from rest_framework.exceptions import PermissionDenied import six +from rest_framework.exceptions import PermissionDenied from enum import Enum from sentry import features from sentry.api.bases import OrganizationEndpoint, OrganizationEventsError -from sentry.api.event_search import get_snuba_query_args, InvalidSearchQuery +from sentry.api.event_search import ( + get_snuba_query_args, + resolve_field_list, + InvalidSearchQuery +) from sentry.models.project import Project from sentry.utils import snuba -# We support 4 "special fields" on the v2 events API which perform some -# additional calculations over aggregated event data -SPECIAL_FIELDS = { - 'issue_title': { - 'aggregations': [['anyHeavy', 'title', 'issue_title']], - }, - 'last_seen': { - 'aggregations': [['max', 'timestamp', 'last_seen']], - }, - 'event_count': { - 'aggregations': [['uniq', 'id', 'event_count']], - }, - 'user_count': { - 'aggregations': [['uniq', 'user', 'user_count']], - }, - 'latest_event': { - 'fields': [ - ['argMax', ['id', 'timestamp'], 'latest_event'], - ], - }, -} - class Direction(Enum): NEXT = 0 @@ -67,47 +48,29 @@ def get_snuba_query_args(self, request, organization, params): except InvalidSearchQuery as exc: raise OrganizationEventsError(exc.message) - fields = request.GET.getlist('field')[:] - aggregations = [] - groupby = request.GET.getlist('groupby') - special_fields = set() - - if fields: - # If project.name is requested, get the project.id from Snuba so we - # can use this to look up the name in Sentry - if 'project.name' in fields: - fields.remove('project.name') - if 'project.id' not in fields: - fields.append('project.id') - - for field in fields[:]: - if field in SPECIAL_FIELDS: - special_fields.add(field) - special_field = deepcopy(SPECIAL_FIELDS[field]) - fields.remove(field) - fields.extend(special_field.get('fields', [])) - aggregations.extend(special_field.get('aggregations', [])) - groupby.extend(special_field.get('groupby', [])) - - snuba_args['selected_columns'] = fields - - self._filter_unspecified_special_fields_in_conditions(snuba_args, special_fields) - if aggregations: - snuba_args['aggregations'] = aggregations - - if groupby: - snuba_args['groupby'] = groupby - sort = request.GET.getlist('sort') - if sort and snuba.valid_orderby(sort, SPECIAL_FIELDS): + if sort: snuba_args['orderby'] = sort # Deprecated. `sort` should be used as it is supported by # more endpoints. orderby = request.GET.getlist('orderby') - if orderby and snuba.valid_orderby(orderby, SPECIAL_FIELDS) and 'orderby' not in snuba_args: + if orderby and 'orderby' not in snuba_args: snuba_args['orderby'] = orderby + if request.GET.get('rollup'): + try: + snuba_args['rollup'] = int(request.GET.get('rollup')) + except ValueError: + raise OrganizationEventsError('rollup must be an integer.') + + fields = request.GET.getlist('field')[:] + if fields: + try: + snuba_args.update(resolve_field_list(fields, snuba_args)) + 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', @@ -147,9 +110,6 @@ def get_snuba_query_args_legacy(self, request, organization): except InvalidSearchQuery as exc: raise OrganizationEventsError(exc.message) - # Filter out special aggregates. - self._filter_unspecified_special_fields_in_conditions(snuba_args, set()) - # TODO(lb): remove once boolean search is fully functional has_boolean_op_flag = features.has( 'organizations:boolean-search', @@ -212,17 +172,3 @@ def _get_next_or_prev_id(self, direction, request, organization, snuba_args, eve return None return six.text_type(result['data'][0]['event_id']) - - def _filter_unspecified_special_fields_in_conditions(self, snuba_args, special_fields): - conditions = [] - for condition in snuba_args['conditions']: - field = condition[0] - if ( - not isinstance(field, (list, tuple)) - and field in SPECIAL_FIELDS - and field not in special_fields - ): - # skip over special field. - continue - conditions.append(condition) - snuba_args['conditions'] = conditions diff --git a/src/sentry/api/endpoints/organization_events.py b/src/sentry/api/endpoints/organization_events.py index a2fec67a37bb76..7cd3e3a1d68cf2 100644 --- a/src/sentry/api/endpoints/organization_events.py +++ b/src/sentry/api/endpoints/organization_events.py @@ -18,7 +18,6 @@ from sentry import features from sentry.models.project import Project -ALLOWED_GROUPINGS = frozenset(('issue.id', 'project.id', 'transaction')) logger = logging.getLogger(__name__) @@ -31,16 +30,8 @@ def get(self, request, organization): try: params = self.get_filter_params(request, organization) snuba_args = self.get_snuba_query_args(request, organization, params) - fields = snuba_args.get('selected_columns') - groupby = snuba_args.get('groupby', []) - - if not fields and not groupby: - return Response({'detail': 'No fields or groupings provided'}, status=400) - - if any(field for field in groupby if field not in ALLOWED_GROUPINGS): - message = ('Invalid groupby value requested. Allowed values are ' + - ', '.join(ALLOWED_GROUPINGS)) - return Response({'detail': message}, status=400) + if not snuba_args.get('selected_columns') and not snuba_args.get('aggregations'): + return Response({'detail': 'No fields provided'}, status=400) except OrganizationEventsError as exc: return Response({'detail': exc.message}, status=400) @@ -130,16 +121,22 @@ def get_legacy(self, request, organization): ) def handle_results(self, request, organization, project_ids, results): + if not results: + return results + + first_row = results[0] + if not ('project.id' in first_row or 'projectid' in first_row): + return results + + fields = request.GET.getlist('field') projects = {p['id']: p['slug'] for p in Project.objects.filter( organization=organization, id__in=project_ids).values('id', 'slug')} - - fields = request.GET.getlist('field') - - if 'project.name' in fields: - for result in results: - result['project.name'] = projects[result['project.id']] - if 'project.id' not in fields: - del result['project.id'] + for result in results: + for key in ('projectid', 'project.id'): + if key in result: + result['project.name'] = projects[result[key]] + if key not in fields: + del result[key] return results diff --git a/src/sentry/api/event_search.py b/src/sentry/api/event_search.py index 9a9b05f11db2bc..c8464144462dfc 100644 --- a/src/sentry/api/event_search.py +++ b/src/sentry/api/event_search.py @@ -2,6 +2,7 @@ import re from collections import namedtuple, defaultdict +from copy import deepcopy from datetime import datetime import six @@ -142,9 +143,8 @@ def translate(pat): 'first_seen': 'first_seen', 'last_seen': 'last_seen', 'times_seen': 'times_seen', - # OrganizationEvents aggregations - 'event_count': 'event_count', - 'user_count': 'user_count', + # TODO(mark) figure out how to safelist aggregate functions/field aliases + # so they can be used in conditions }, **SENTRY_SNUBA_MAP) no_conversion = set(['project_id', 'start', 'end']) @@ -215,9 +215,8 @@ class SearchVisitor(NodeVisitor): 'device.battery_level', 'device.charging', 'device.online', 'device.simulator', 'error.handled', 'issue.id', 'stack.colno', 'stack.in_app', 'stack.lineno', 'stack.stack_level', - # OrganizationEvents aggregations - 'event_count', 'user_count', - + # TODO(mark) figure out how to safelist aggregate functions/field aliases + # so they can be used in conditions ]) date_keys = set([ 'start', 'end', 'first_seen', 'last_seen', 'time', 'timestamp', @@ -662,3 +661,158 @@ def get_snuba_query_args(query=None, params=None): kwargs['has_boolean_terms'] = True kwargs['conditions'].append(convert_search_boolean_to_snuba_query(term)) return kwargs + + +FIELD_ALIASES = { + 'issue_title': { + 'aggregations': [['anyHeavy', 'title', 'issue_title']], + }, + 'last_seen': { + 'aggregations': [['max', 'timestamp', 'last_seen']], + }, + 'latest_event': { + 'aggregations': [ + # TODO(mark) This is a hack to work around jsonschema limitations + # in snuba. + ['argMax(event_id, timestamp)', '', 'latest_event'], + ], + }, + 'project': { + 'fields': ['project.id'], + }, + 'user': { + 'fields': ['user.id', 'user.name', 'user.username', 'user.email', 'user.ip'], + } + # TODO(mark) Add rpm alias. +} + +VALID_AGGREGATES = { + 'count_unique': { + 'snuba_name': 'uniq', + 'fields': '*', + }, + 'count': { + 'snuba_name': 'count', + 'fields': '*' + }, + 'avg': { + 'snuba_name': 'avg', + 'fields': ['duration'], + }, + 'min': { + 'snuba_name': 'min', + 'fields': ['timestamp', 'duration'], + }, + 'max': { + 'snuba_name': 'max', + 'fields': ['timestamp', 'duration'], + }, + 'sum': { + 'snuba_name': 'sum', + 'fields': ['duration'], + }, + # This doesn't work yet, but is an illustration of how it could work + 'p75': { + 'snuba_name': 'quantileTiming(0.75)', + 'fields': ['duration'], + }, +} + +AGGREGATE_PATTERN = re.compile(r'^(?P[^\(]+)\((?P[a-z\._]*)\)$') + + +def validate_aggregate(field, match): + function_name = match.group('function') + if function_name not in VALID_AGGREGATES: + raise InvalidSearchQuery("Unknown aggregate function '%s'" % field) + + function_data = VALID_AGGREGATES[function_name] + column = match.group('column') + if column not in function_data['fields'] and function_data['fields'] != '*': + raise InvalidSearchQuery( + "Invalid column '%s' in aggregate function '%s'" % (column, function_name)) + + +def validate_orderby(orderby, fields): + orderby = orderby if isinstance(orderby, (list, tuple)) else [orderby] + for column in orderby: + column = column.lstrip('-') + if column not in fields: + raise InvalidSearchQuery('Cannot order by an field that is not selected.') + + +def resolve_field_list(fields, snuba_args): + """ + Expand a list of fields based on aliases and aggregate functions. + + Returns a dist of aggregations, selected_columns, and + groupby that can be merged into the result of get_snuba_query_args() + to build a more complete snuba query based on event search conventions. + """ + # If project.name is requested, get the project.id from Snuba so we + # can use this to look up the name in Sentry + if 'project.name' in fields: + fields.remove('project.name') + if 'project.id' not in fields: + fields.append('project.id') + + aggregations = [] + groupby = [] + columns = [] + for field in fields: + if not isinstance(field, six.string_types): + raise InvalidSearchQuery('Field names must be strings') + + if field in FIELD_ALIASES: + special_field = deepcopy(FIELD_ALIASES[field]) + columns.extend(special_field.get('fields', [])) + aggregations.extend(special_field.get('aggregations', [])) + continue + + # Basic fields don't require additional validation. They could be tag + # names which we have no way of validating at this point. + match = AGGREGATE_PATTERN.search(field) + if not match: + columns.append(field) + continue + + validate_aggregate(field, match) + aggregations.append([ + VALID_AGGREGATES[match.group('function')]['snuba_name'], + match.group('column'), + u'{}_{}'.format(match.group('function'), match.group('column')).rstrip('_') + ]) + + rollup = snuba_args.get('rollup') + if not rollup: + # Ensure fields we require to build a functioning interface + # are present. We don't add fields when using a rollup as the additional fields + # would be aggregated away. When there are aggregations + # we use argMax to get the latest event/projectid so we can create links. + # The `projectid` output name is not a typo, using `project_id` triggers + # generates invalid queries. + if not aggregations and 'id' not in columns: + columns.append('id') + columns.append('project.id') + if aggregations and 'latest_event' not in fields: + aggregations.extend(deepcopy(FIELD_ALIASES['latest_event']['aggregations'])) + if aggregations and 'project.id' not in columns: + aggregations.append(['argMax(project_id, timestamp)', '', 'projectid']) + + if rollup and columns and not aggregations: + raise InvalidSearchQuery('You cannot use rollup without an aggregate field.') + + orderby = snuba_args.get('orderby') + if orderby: + validate_orderby(orderby, fields) + + # If aggregations are present all columns + # need to be added to the group by so that the query is valid. + if aggregations: + groupby.extend(columns) + + return { + 'selected_columns': columns, + 'aggregations': aggregations, + 'groupby': groupby, + } diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx index 75162bb704d369..f2c023c092085c 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx @@ -17,14 +17,16 @@ import {QueryLink} from './styles'; export const MODAL_QUERY_KEYS = ['eventSlug']; export const PIN_ICON = `image://${pinIcon}`; +export const AGGREGATE_ALIASES = ['issue_title', 'last_seen', 'latest_event']; export const ALL_VIEWS = deepFreeze([ { id: 'all', name: t('All Events'), data: { - fields: ['event', 'type', 'project', 'user', 'time'], - sort: ['-timestamp', '-id'], + fields: ['title', 'event.type', 'project', 'user', 'timestamp'], + columnNames: ['title', 'type', 'project', 'user', 'time'], + sort: ['-timestamp'], }, tags: [ 'event.type', @@ -40,9 +42,9 @@ export const ALL_VIEWS = deepFreeze([ id: 'errors', name: t('Errors'), data: { - fields: ['error', 'event_count', 'user_count', 'project', 'last_seen'], - groupby: ['issue.id', 'project.id'], - sort: ['-last_seen', '-issue.id'], + fields: ['issue_title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'], + columnNames: ['error', 'events', 'users', 'project', 'last seen'], + sort: ['-last_seen', '-issue_title'], query: 'event.type:error', }, tags: ['error.type', 'project.name'], @@ -52,9 +54,9 @@ export const ALL_VIEWS = deepFreeze([ id: 'csp', name: t('CSP'), data: { - fields: ['csp', 'event_count', 'user_count', 'project', 'last_seen'], - groupby: ['issue.id', 'project.id'], - sort: ['-last_seen', '-issue.id'], + fields: ['issue_title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'], + columnNames: ['csp', 'events', 'users', 'project', 'last seen'], + sort: ['-last_seen', '-issue_title'], query: 'event.type:csp', }, tags: [ @@ -70,8 +72,8 @@ export const ALL_VIEWS = deepFreeze([ id: 'transactions', name: t('Transactions'), data: { - fields: ['transaction', 'project'], - groupby: ['transaction', 'project.id'], + fields: ['transaction', 'project', 'count(id)'], + columnNames: ['transaction', 'project', 'volume'], sort: ['-transaction'], query: 'event.type:transaction', }, @@ -83,7 +85,7 @@ export const ALL_VIEWS = deepFreeze([ 'user.ip', 'environment', ], - columnWidths: ['3fr', '2fr'], + columnWidths: ['3fr', '1fr', '70px'], }, ]); @@ -94,7 +96,6 @@ export const ALL_VIEWS = deepFreeze([ */ export const SPECIAL_FIELDS = { transaction: { - fields: ['project.name', 'transaction', 'latest_event'], sortField: 'transaction', renderFunc: (data, {organization, location}) => { const target = { @@ -113,8 +114,7 @@ export const SPECIAL_FIELDS = { ); }, }, - event: { - fields: ['title', 'id', 'project.name'], + title: { sortField: 'title', renderFunc: (data, {organization, location}) => { const target = { @@ -131,7 +131,6 @@ export const SPECIAL_FIELDS = { }, }, type: { - fields: ['event.type'], sortField: 'event.type', renderFunc: (data, {location, organization}) => { const target = { @@ -146,7 +145,6 @@ export const SPECIAL_FIELDS = { }, }, project: { - fields: ['project.name'], sortField: false, renderFunc: (data, {organization}) => { const project = organization.projects.find(p => p.slug === data['project.name']); @@ -162,8 +160,7 @@ export const SPECIAL_FIELDS = { }, }, user: { - fields: ['user', 'user.name', 'user.username', 'user.email', 'user.ip', 'user.id'], - sortField: 'user', + sortField: 'user.id', renderFunc: (data, {organization, location}) => { const userObj = { id: data['user.id'], @@ -192,8 +189,7 @@ export const SPECIAL_FIELDS = { return {badge}; }, }, - time: { - fields: ['timestamp'], + timestamp: { sortField: 'timestamp', renderFunc: data => ( @@ -206,8 +202,7 @@ export const SPECIAL_FIELDS = { ), }, - error: { - fields: ['issue_title', 'project.name', 'latest_event'], + issue_title: { sortField: 'issue_title', renderFunc: (data, {organization, location}) => { const target = { @@ -226,49 +221,26 @@ export const SPECIAL_FIELDS = { ); }, }, - csp: { - fields: ['issue_title', 'project.name', 'latest_event'], - sortField: 'issue_title', - renderFunc: (data, {organization, location}) => { - const target = { - pathname: `/organizations/${organization.slug}/events/`, - query: { - ...location.query, - eventSlug: `${data['project.name']}:${data.latest_event}`, - }, - }; - return ( - - - {data.issue_title} - - - ); - }, - }, - event_count: { - title: 'events', - fields: ['event_count'], - sortField: 'event_count', + // TODO generalize this. + 'count(id)': { + sortField: 'count_id', renderFunc: data => ( - {typeof data.event_count === 'number' ? : null} + {typeof data.count_id === 'number' ? : null} ), }, - user_count: { - title: 'users', - fields: ['user_count'], - sortField: 'user_count', + 'count_unique(user)': { + sortField: 'unique_count_user', renderFunc: data => ( - {typeof data.user_count === 'number' ? : null} + {typeof data.count_unique_user === 'number' ? ( + + ) : null} ), }, last_seen: { - title: 'last seen', - fields: ['last_seen'], sortField: 'last_seen', renderFunc: data => { return ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx index bcdcb406df9b9d..e9901feda933fa 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx @@ -17,6 +17,7 @@ import ModalPagination from './modalPagination'; import ModalLineGraph from './modalLineGraph'; import RelatedEvents from './relatedEvents'; import TagsTable from './tagsTable'; +import {AGGREGATE_ALIASES} from './data'; import TransanctionView from './transactionView'; /** @@ -25,7 +26,11 @@ import TransanctionView from './transactionView'; */ const EventModalContent = props => { const {event, projectId, organization, location, view} = props; - const isGroupedView = !!view.data.groupby; + + // Known aggregate aliases and functions indicated grouped views. + const isGroupedView = !!view.data.fields.find( + field => AGGREGATE_ALIASES.includes(field) || field.match(/[a-z_]+\([a-z_\.]+\)/) + ); const eventJsonUrl = `/api/0/projects/${organization.slug}/${projectId}/events/${ event.eventID }/json/`; diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/table.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/table.jsx index 2a90023bbc7ccf..45b0e4b538a028 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/table.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/table.jsx @@ -49,7 +49,6 @@ export default class Table extends React.Component { query: `${field}:${row[field]}`, }, }; - return ( {SPECIAL_FIELDS.hasOwnProperty(field) ? ( @@ -66,17 +65,16 @@ export default class Table extends React.Component { render() { const {isLoading, location, view} = this.props; - const {fields, sort} = view.data; + const {fields, columnNames, sort} = view.data; const defaultSort = sort.length ? sort[0] : null; return ( - {fields.map(field => { - let title = field; + {fields.map((field, i) => { + const title = columnNames[i] || field; let sortKey = field; if (SPECIAL_FIELDS.hasOwnProperty(field)) { - title = SPECIAL_FIELDS[field].title || field; sortKey = SPECIAL_FIELDS[field].sortField; } diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/utils.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/utils.jsx index 827ef7293052d1..f4f880453541b0 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/utils.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/utils.jsx @@ -2,7 +2,7 @@ import {pick, get} from 'lodash'; import {DEFAULT_PER_PAGE} from 'app/constants'; import {URL_PARAM} from 'app/constants/globalSelectionHeader'; -import {ALL_VIEWS, SPECIAL_FIELDS} from './data'; +import {ALL_VIEWS} from './data'; /** * Given a view id, return the corresponding view object @@ -22,25 +22,8 @@ export function getCurrentView(requestedView) { * @returns {Object} */ export function getQuery(view, location) { - const fields = []; const groupby = view.data.groupby ? [...view.data.groupby] : []; - - const viewFields = get(view, 'data.fields', []); - - viewFields.forEach(field => { - if (SPECIAL_FIELDS.hasOwnProperty(field)) { - const specialField = SPECIAL_FIELDS[field]; - - if (specialField.hasOwnProperty('fields')) { - fields.push(...specialField.fields); - } - if (specialField.hasOwnProperty('groupby')) { - groupby.push(...specialField.groupby); - } - } else { - fields.push(field); - } - }); + const fields = get(view, 'data.fields', []); const data = pick(location.query, [ 'project', diff --git a/tests/js/spec/views/organizationEventsV2/index.spec.jsx b/tests/js/spec/views/organizationEventsV2/index.spec.jsx index 1c594939d55eef..f7fc728f72c30a 100644 --- a/tests/js/spec/views/organizationEventsV2/index.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/index.spec.jsx @@ -94,9 +94,9 @@ describe('OrganizationEventsV2', function() { // Sort link should reverse. expect(timestamp.props().to.query).toEqual({sort: 'timestamp'}); - const userlink = findLink('user'); + const userlink = findLink('user.id'); // User link should be descending. - expect(userlink.props().to.query).toEqual({sort: '-user'}); + expect(userlink.props().to.query).toEqual({sort: '-user.id'}); }); it('generates links to modals', async function() { diff --git a/tests/js/spec/views/organizationEventsV2/utils.spec.jsx b/tests/js/spec/views/organizationEventsV2/utils.spec.jsx index cb7fbdf59978ca..2b6d23a549687e 100644 --- a/tests/js/spec/views/organizationEventsV2/utils.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/utils.spec.jsx @@ -20,30 +20,6 @@ describe('getCurrentView()', function() { }); describe('getQuery()', function() { - it('expands special "event" and "user" fields', function() { - const view = { - id: 'test', - name: 'test view', - data: { - fields: ['event', 'user', 'issue.id'], - }, - tags: [], - }; - - expect(getQuery(view, {}).field).toEqual([ - 'title', - 'id', - 'project.name', - 'user', - 'user.name', - 'user.username', - 'user.email', - 'user.ip', - 'user.id', - 'issue.id', - ]); - }); - it('appends any additional conditions defined for view', function() { const view = { id: 'test', diff --git a/tests/sentry/api/test_event_search.py b/tests/sentry/api/test_event_search.py index 189c1ee0b7b161..046706ed9667d6 100644 --- a/tests/sentry/api/test_event_search.py +++ b/tests/sentry/api/test_event_search.py @@ -3,6 +3,7 @@ import datetime import pytest import six +import unittest from datetime import timedelta from django.utils import timezone @@ -10,13 +11,14 @@ from sentry.api.event_search import ( convert_endpoint_params, event_search_grammar, get_snuba_query_args, - parse_search_query, InvalidSearchQuery, SearchBoolean, SearchFilter, SearchKey, + resolve_field_list, parse_search_query, + InvalidSearchQuery, SearchBoolean, SearchFilter, SearchKey, SearchValue, SearchVisitor, ) from sentry.testutils import TestCase -class ParseSearchQueryTest(TestCase): +class ParseSearchQueryTest(unittest.TestCase): def test_simple(self): # test with raw search query at the end assert parse_search_query('user.email:foo@example.com release:1.2.1 hello') == [ @@ -691,7 +693,7 @@ def test_empty_string(self): assert parse_search_query('') == [] -class ParseBooleanSearchQueryTest(TestCase): +class ParseBooleanSearchQueryTest(unittest.TestCase): def setUp(self): super(ParseBooleanSearchQueryTest, self).setUp() self.term1 = SearchFilter( @@ -1163,7 +1165,7 @@ def test_project_name(self): } -class ConvertEndpointParamsTests(TestCase): +class ConvertEndpointParamsTests(unittest.TestCase): def test_simple(self): assert convert_endpoint_params({ 'project_id': [1, 2, 3], @@ -1204,3 +1206,146 @@ def test_simple(self): ) ), ] + + +class ResolveFieldListTest(unittest.TestCase): + def test_non_string_field_error(self): + fields = [['any', 'thing', 'lol']] + with pytest.raises(InvalidSearchQuery) as err: + resolve_field_list(fields, {}) + assert 'Field names' in six.text_type(err) + + def test_automatic_fields_no_aggregates(self): + fields = ['event.type', 'message'] + result = resolve_field_list(fields, {}) + assert result['selected_columns'] == ['event.type', 'message', 'id', 'project.id'] + assert result['aggregations'] == [] + assert result['groupby'] == [] + + def test_automatic_fields_with_aggregate_aliases(self): + fields = ['issue_title', 'message'] + result = resolve_field_list(fields, {}) + # Automatic fields should be inserted + assert result['selected_columns'] == [ + 'message', + ] + assert result['aggregations'] == [ + ['anyHeavy', 'title', 'issue_title'], + ['argMax(event_id, timestamp)', '', 'latest_event'], + ['argMax(project_id, timestamp)', '', 'projectid'], + ] + assert result['groupby'] == ['message'] + + def test_field_alias_expansion(self): + fields = ['issue_title', 'last_seen', 'latest_event', 'project', 'user', 'message'] + result = resolve_field_list(fields, {}) + assert result['selected_columns'] == [ + 'project.id', + 'user.id', + 'user.name', + 'user.username', + 'user.email', + 'user.ip', + 'message', + ] + assert result['aggregations'] == [ + ['anyHeavy', 'title', 'issue_title'], + ['max', 'timestamp', 'last_seen'], + ['argMax(event_id, timestamp)', '', 'latest_event'], + ] + assert result['groupby'] == [ + 'project.id', + 'user.id', + 'user.name', + 'user.username', + 'user.email', + 'user.ip', + 'message', + ] + + def test_aggregate_function_expansion(self): + fields = ['count_unique(user)', 'count(id)', 'avg(duration)'] + result = resolve_field_list(fields, {}) + # Automatic fields should be inserted + assert result['selected_columns'] == [] + assert result['aggregations'] == [ + ['uniq', 'user', 'count_unique_user'], + ['count', 'id', 'count_id'], + ['avg', 'duration', 'avg_duration'], + ['argMax(event_id, timestamp)', '', 'latest_event'], + ['argMax(project_id, timestamp)', '', 'projectid'], + ] + assert result['groupby'] == [] + + def test_aggregate_function_invalid_name(self): + with pytest.raises(InvalidSearchQuery) as err: + fields = ['derp(user)'] + resolve_field_list(fields, {}) + assert 'Unknown aggregate' in six.text_type(err) + + def test_aggregate_function_case_sensitive(self): + with pytest.raises(InvalidSearchQuery) as err: + fields = ['MAX(user)'] + resolve_field_list(fields, {}) + assert 'Unknown aggregate' in six.text_type(err) + + def test_aggregate_function_invalid_column(self): + with pytest.raises(InvalidSearchQuery) as err: + fields = ['p75(message)'] + resolve_field_list(fields, {}) + assert 'Invalid column' in six.text_type(err) + + def test_rollup_with_unaggregated_fields(self): + with pytest.raises(InvalidSearchQuery) as err: + fields = ['message'] + snuba_args = {'rollup': 15} + resolve_field_list(fields, snuba_args) + assert 'rollup without an aggregate' in six.text_type(err) + + def test_rollup_with_basic_and_aggregated_fields(self): + fields = ['message', 'count()'] + snuba_args = {'rollup': 15} + result = resolve_field_list(fields, snuba_args) + + assert result['aggregations'] == [ + ['count', '', 'count'] + ] + assert result['selected_columns'] == ['message'] + assert result['groupby'] == ['message'] + + def test_rollup_with_aggregated_fields(self): + fields = ['count_unique(user)'] + snuba_args = {'rollup': 15} + result = resolve_field_list(fields, snuba_args) + assert result['aggregations'] == [ + ['uniq', 'user', 'count_unique_user'] + ] + assert result['selected_columns'] == [] + assert result['groupby'] == [] + + def test_orderby_unselected_field(self): + fields = ['message'] + snuba_args = {'orderby': 'timestamp'} + with pytest.raises(InvalidSearchQuery) as err: + resolve_field_list(fields, snuba_args) + assert 'Cannot order' in six.text_type(err) + + def test_orderby_basic_field(self): + fields = ['message'] + snuba_args = {'orderby': '-message'} + result = resolve_field_list(fields, snuba_args) + assert result['selected_columns'] == ['message', 'id', 'project.id'] + assert result['aggregations'] == [] + assert result['groupby'] == [] + + def test_orderby_field_alias(self): + fields = ['issue_title'] + snuba_args = {'orderby': '-issue_title'} + result = resolve_field_list(fields, snuba_args) + assert result['selected_columns'] == [] + assert result['aggregations'] == [ + ['anyHeavy', 'title', 'issue_title'], + ['argMax(event_id, timestamp)', '', 'latest_event'], + ['argMax(project_id, timestamp)', '', 'projectid'], + ] + assert result['groupby'] == [] diff --git a/tests/snuba/api/endpoints/test_organization_events_stats.py b/tests/snuba/api/endpoints/test_organization_events_stats.py index 7bcfba9590d99c..2444f8fd3974bf 100644 --- a/tests/snuba/api/endpoints/test_organization_events_stats.py +++ b/tests/snuba/api/endpoints/test_organization_events_stats.py @@ -164,25 +164,3 @@ def test_with_event_count_flag(self): [{'count': 1}], [{'count': 2}], ] - - def test_special_fields_ignored(self): - url = reverse( - 'sentry-api-0-organization-events-stats', - kwargs={ - 'organization_slug': self.project.organization.slug, - } - ) - response = self.client.get('%s?%s' % (url, urlencode({ - 'start': self.day_ago.isoformat()[:19], - 'end': (self.day_ago + timedelta(hours=1, minutes=59)).isoformat()[:19], - 'interval': '1h', - 'yAxis': 'event_count', - 'query': 'event_count:>5' - })), format='json') - - assert response.status_code == 200, response.content - assert [attrs for time, attrs in response.data['data']] == [ - [], - [{'count': 1}], - [{'count': 2}], - ] diff --git a/tests/snuba/api/endpoints/test_organization_events_v2.py b/tests/snuba/api/endpoints/test_organization_events_v2.py index 80f3984b8a3b09..dfedbef93eb5e4 100644 --- a/tests/snuba/api/endpoints/test_organization_events_v2.py +++ b/tests/snuba/api/endpoints/test_organization_events_v2.py @@ -6,6 +6,7 @@ from django.core.urlresolvers import reverse from sentry.testutils import APITestCase, SnubaTestCase +import pytest class OrganizationEventsTestBase(APITestCase, SnubaTestCase): @@ -119,7 +120,7 @@ def test_raw_data(self): self.url, format='json', data={ - 'field': ['id', 'project.id', 'user.email', 'user.ip', 'time'], + 'field': ['id', 'project.id', 'user.email', 'user.ip', 'timestamp'], 'orderby': '-timestamp', }, ) @@ -157,18 +158,18 @@ def test_project_name(self): assert 'project.id' not in response.data[0] assert response.data[0]['environment'] == 'staging' - def test_groupby(self): + def test_implicit_groupby(self): self.login_as(user=self.user) project = self.create_project() - event1 = self.store_event( + self.store_event( data={ 'event_id': 'a' * 32, - 'timestamp': self.min_ago, + 'timestamp': self.two_min_ago, 'fingerprint': ['group_1'], }, project_id=project.id, ) - self.store_event( + event1 = self.store_event( data={ 'event_id': 'b' * 32, 'timestamp': self.min_ago, @@ -190,18 +191,64 @@ def test_groupby(self): self.url, format='json', data={ - 'field': ['project.id', 'issue.id'], - 'groupby': ['project.id', 'issue.id'], + 'field': ['count(id)', 'project.id', 'issue.id'], 'orderby': 'issue.id', }, ) assert response.status_code == 200, response.content assert len(response.data) == 2 - assert response.data[0]['project.id'] == project.id - assert response.data[0]['issue.id'] == event1.group_id - assert response.data[1]['project.id'] == project.id - assert response.data[1]['issue.id'] == event2.group_id + assert response.data[0] == { + 'project.id': project.id, + 'project.name': project.slug, + 'issue.id': event1.group_id, + 'count_id': 2, + 'latest_event': event1.event_id, + } + assert response.data[1] == { + 'project.id': project.id, + 'project.name': project.slug, + 'issue.id': event2.group_id, + 'count_id': 1, + 'latest_event': event2.event_id, + } + + def test_automatic_id_and_project(self): + self.login_as(user=self.user) + project = self.create_project() + self.store_event( + data={ + 'event_id': 'a' * 32, + 'timestamp': self.two_min_ago, + 'fingerprint': ['group_1'], + }, + project_id=project.id, + ) + event = self.store_event( + data={ + 'event_id': 'b' * 32, + 'timestamp': self.min_ago, + 'fingerprint': ['group_1'], + }, + project_id=project.id, + ) + + with self.feature('organizations:events-v2'): + response = self.client.get( + self.url, + format='json', + data={ + 'field': ['count(id)'], + }, + ) + + assert response.status_code == 200, response.content + assert len(response.data) == 1 + assert response.data[0] == { + 'project.name': project.slug, + 'count_id': 2, + 'latest_event': event.event_id, + } def test_orderby(self): self.login_as(user=self.user) @@ -232,11 +279,12 @@ def test_orderby(self): self.url, format='json', data={ - 'field': ['id'], + 'field': ['id', 'timestamp'], 'orderby': ['-timestamp', '-id'] }, ) + assert response.status_code == 200, response.content assert response.data[0]['id'] == 'c' * 32 assert response.data[1]['id'] == 'b' * 32 assert response.data[2]['id'] == 'a' * 32 @@ -273,16 +321,17 @@ def test_sort_title(self): self.url, format='json', data={ - 'field': ['id'], + 'field': ['id', 'title'], 'sort': 'title' }, ) + assert response.status_code == 200, response.content assert response.data[0]['id'] == 'c' * 32 assert response.data[1]['id'] == 'b' * 32 assert response.data[2]['id'] == 'a' * 32 - def test_sort_ignore_invalid(self): + def test_sort_invalid(self): self.login_as(user=self.user) project = self.create_project() self.store_event( @@ -301,9 +350,10 @@ def test_sort_ignore_invalid(self): 'sort': 'garbage' }, ) - assert response.status_code == 200 + assert response.status_code == 400 + assert 'order by' in response.content - def test_special_fields(self): + def test_aliased_fields(self): self.login_as(user=self.user) project = self.create_project() event1 = self.store_event( @@ -345,8 +395,7 @@ def test_special_fields(self): self.url, format='json', data={ - 'field': ['issue_title', 'event_count', 'user_count'], - 'groupby': ['issue.id', 'project.id'], + 'field': ['issue.id', 'issue_title', 'count(id)', 'count_unique(user)'], 'orderby': 'issue.id' }, ) @@ -354,12 +403,17 @@ def test_special_fields(self): assert response.status_code == 200, response.content assert len(response.data) == 2 assert response.data[0]['issue.id'] == event1.group_id - assert response.data[0]['event_count'] == 1 - assert response.data[0]['user_count'] == 1 + assert response.data[0]['count_id'] == 1 + assert response.data[0]['count_unique_user'] == 1 + assert 'latest_event' in response.data[0] + assert 'project.name' in response.data[0] + assert 'projectid' not in response.data[0] + assert 'project.id' not in response.data[0] assert response.data[1]['issue.id'] == event2.group_id - assert response.data[1]['event_count'] == 2 - assert response.data[1]['user_count'] == 2 + assert response.data[1]['count_id'] == 2 + assert response.data[1]['count_unique_user'] == 2 + @pytest.mark.xfail(reason='aggregate comparisons need parser improvements') def test_aggregation_comparison(self): self.login_as(user=self.user) project = self.create_project() @@ -424,10 +478,9 @@ def test_aggregation_comparison(self): self.url, format='json', data={ - 'field': ['issue_title', 'event_count', 'user_count'], - 'query': 'event_count:>1 user_count:>1', - 'groupby': ['issue.id', 'project.id'], - 'orderby': 'issue.id' + 'field': ['issue_title', 'count(id)', 'count_unique(user)'], + 'query': 'count_id:>1 count_unique_user:>1', + 'orderby': 'issue_title' }, ) @@ -435,9 +488,10 @@ def test_aggregation_comparison(self): assert len(response.data) == 1 assert response.data[0]['issue.id'] == event.group_id - assert response.data[0]['event_count'] == 2 - assert response.data[0]['user_count'] == 2 + assert response.data[0]['count_id'] == 2 + assert response.data[0]['count_unique_user'] == 2 + @pytest.mark.xfail(reason='aggregate comparisons need parser improvements') def test_aggregation_comparison_with_conditions(self): self.login_as(user=self.user) project = self.create_project() @@ -495,10 +549,9 @@ def test_aggregation_comparison_with_conditions(self): self.url, format='json', data={ - 'field': ['issue_title', 'event_count'], - 'query': 'event_count:>1 user.email:foo@example.com environment:prod', - 'groupby': ['issue.id', 'project.id'], - 'orderby': 'issue.id' + 'field': ['issue_title', 'count(id)'], + 'query': 'count_id:>1 user.email:foo@example.com environment:prod', + 'orderby': 'issue_title' }, ) @@ -506,56 +559,7 @@ def test_aggregation_comparison_with_conditions(self): assert len(response.data) == 1 assert response.data[0]['issue.id'] == event.group_id - assert response.data[0]['event_count'] == 2 - - def test_invalid_groupby(self): - self.login_as(user=self.user) - - project = self.create_project() - self.store_event( - data={ - 'event_id': 'a' * 32, - 'message': 'how to make fast', - 'timestamp': self.min_ago, - }, - project_id=project.id - ) - - with self.feature('organizations:events-v2'): - response = self.client.get( - self.url, - format='json', - data={ - 'groupby': ['id'], - }, - ) - assert response.status_code == 400, response.content - assert response.data['detail'] == 'Invalid groupby value requested. Allowed values are transaction, project.id, issue.id' - - def test_non_aggregated_fields_with_groupby(self): - self.login_as(user=self.user) - - project = self.create_project() - self.store_event( - data={ - 'event_id': 'a' * 32, - 'message': 'how to make fast', - 'timestamp': self.min_ago, - }, - project_id=project.id - ) - - with self.feature('organizations:events-v2'): - response = self.client.get( - self.url, - format='json', - data={ - 'field': ['project.id'], - 'groupby': ['issue.id'], - }, - ) - assert response.status_code == 400, response.content - assert response.data['detail'] == 'Invalid query.' + assert response.data[0]['count_id'] == 2 def test_nonexistent_fields(self): self.login_as(user=self.user) @@ -579,7 +583,7 @@ def test_nonexistent_fields(self): }, ) assert response.status_code == 200, response.content - assert response.data == [{u'issue_world.id': u''}] + assert response.data[0]['issue_world.id'] == '' def test_no_requested_fields_or_grouping(self): self.login_as(user=self.user) @@ -603,33 +607,11 @@ def test_no_requested_fields_or_grouping(self): }, ) assert response.status_code == 400, response.content - assert response.data['detail'] == 'No fields or groupings provided' + assert response.data['detail'] == 'No fields provided' - def test_irrelevant_special_field_in_query(self): + def test_condition_on_aggregate_fails(self): self.login_as(user=self.user) project = self.create_project() - event1 = self.store_event( - data={ - 'event_id': 'a' * 32, - 'timestamp': self.min_ago, - 'fingerprint': ['group_1'], - 'user': { - 'email': 'foo@example.com', - }, - }, - project_id=project.id, - ) - event2 = self.store_event( - data={ - 'event_id': 'b' * 32, - 'timestamp': self.min_ago, - 'fingerprint': ['group_2'], - 'user': { - 'email': 'foo@example.com', - }, - }, - project_id=project.id, - ) self.store_event( data={ 'event_id': 'c' * 32, @@ -648,16 +630,13 @@ def test_irrelevant_special_field_in_query(self): format='json', data={ 'field': ['issue.id'], - 'query': 'event_count:>1', + 'query': 'event_count:>0', 'orderby': 'issue.id' }, ) assert response.status_code == 200, response.content - assert len(response.data) == 3 - assert response.data[0]['issue.id'] == event1.group_id - assert response.data[1]['issue.id'] == event2.group_id - assert response.data[2]['issue.id'] == event2.group_id + assert len(response.data) == 0 def test_group_filtering(self): user = self.create_user()