feat(events2) Add metadata to responses and generalize column formatting

src/sentry/api/endpoints/organization_events.py

@@ -10,11 +10,7 @@
 from sentry.api.paginator import GenericOffsetPaginator
 from sentry.api.serializers import EventSerializer, serialize, SimpleEventSerializer
 from sentry.models import SnubaEvent
-from sentry.utils.snuba import (
-    raw_query,
-    SnubaError,
-    transform_aliases_and_query,
-)
+from sentry.utils import snuba
 from sentry import features
 from sentry.models.project import Project
 
@@ -49,8 +45,7 @@ def get(self, request, organization):
             }, status=400)
 
         data_fn = partial(
-            lambda **kwargs: transform_aliases_and_query(
-                skip_conditions=True, **kwargs)['data'],
+            lambda **kwargs: snuba.transform_aliases_and_query(skip_conditions=True, **kwargs),
             referrer='api.organization-events-v2',
             **snuba_args
         )
@@ -59,10 +54,10 @@ def get(self, request, organization):
             return self.paginate(
                 request=request,
                 paginator=GenericOffsetPaginator(data_fn=data_fn),
-                on_results=lambda results: self.handle_results(
+                on_results=lambda results: self.handle_results_with_meta(
                     request, organization, params['project_id'], results),
             )
-        except SnubaError as error:
+        except snuba.SnubaError as error:
             logger.info(
                 'organization.events.snuba-error',
                 extra={
@@ -105,7 +100,7 @@ def get_legacy(self, request, organization):
             snuba_cols = SnubaEvent.minimal_columns if full else SnubaEvent.selected_columns
             data_fn = partial(
                 # extract 'data' from raw_query result
-                lambda *args, **kwargs: raw_query(*args, **kwargs)['data'],
+                lambda *args, **kwargs: snuba.raw_query(*args, **kwargs)['data'],
                 selected_columns=snuba_cols,
                 orderby='-timestamp',
                 referrer='api.organization-events',
@@ -120,6 +115,21 @@ def get_legacy(self, request, organization):
             paginator=GenericOffsetPaginator(data_fn=data_fn)
         )
 
+    def handle_results_with_meta(self, request, organization, project_ids, results):
+        data = self.handle_results(request, organization, project_ids, results.get('data'))
+        if not data:
+            return {'data': [], 'meta': {}}
+
+        meta = {value['name']: snuba.get_json_type(value['type']) for value in results['meta']}
+        # Ensure all columns in the result have types.
+        for key in data[0]:
+            if key not in meta:
+                meta[key] = 'string'
+        return {
+            'meta': meta,
+            'data': data,
+        }
+
     def handle_results(self, request, organization, project_ids, results):
         if not results:
             return results

src/sentry/api/event_search.py

@@ -733,12 +733,35 @@ def validate_aggregate(field, match):
             "Invalid column '%s' in aggregate function '%s'" % (column, function_name))
 
 
-def validate_orderby(orderby, fields):
+def resolve_orderby(orderby, fields, aggregations):
+    """
+    We accept column names, aggregate functions, and aliases as order by
+    values. Aggregates and field aliases need to be resolve/validated.
+    """
     orderby = orderby if isinstance(orderby, (list, tuple)) else [orderby]
+    validated = []
     for column in orderby:
-        column = column.lstrip('-')
-        if column not in fields:
-            raise InvalidSearchQuery('Cannot order by an field that is not selected.')
+        bare_column = column.lstrip('-')
+        if bare_column in fields:
+            validated.append(column)
+            continue
+
+        match = AGGREGATE_PATTERN.search(bare_column)
+        if match:
+            bare_column = get_aggregate_alias(match)
+        found = [agg[2] for agg in aggregations if agg[2] == bare_column]
+        if found:
+            prefix = '-' if column.startswith('-') else ''
+            validated.append(prefix + bare_column)
+
+    if len(validated) == len(orderby):
+        return validated
+
+    raise InvalidSearchQuery('Cannot order by an field that is not selected.')
+
+
+def get_aggregate_alias(match):
+    return u'{}_{}'.format(match.group('function'), match.group('column')).rstrip('_')
 
 
 def resolve_field_list(fields, snuba_args):
@@ -780,7 +803,7 @@ def resolve_field_list(fields, snuba_args):
         aggregations.append([
             VALID_AGGREGATES[match.group('function')]['snuba_name'],
             match.group('column'),
-            u'{}_{}'.format(match.group('function'), match.group('column')).rstrip('_')
+            get_aggregate_alias(match)
         ])
 
     rollup = snuba_args.get('rollup')
@@ -804,7 +827,7 @@ def resolve_field_list(fields, snuba_args):
 
     orderby = snuba_args.get('orderby')
     if orderby:
-        validate_orderby(orderby, fields)
+        orderby = resolve_orderby(orderby, columns, aggregations)
 
     # If aggregations are present all columns
     # need to be added to the group by so that the query is valid.
@@ -815,4 +838,5 @@ def resolve_field_list(fields, snuba_args):
         'selected_columns': columns,
         'aggregations': aggregations,
         'groupby': groupby,
+        'orderby': orderby
     }

src/sentry/discover/endpoints/discover_query.py

@@ -1,6 +1,5 @@
 from __future__ import absolute_import
 
-import re
 from functools import partial
 from copy import deepcopy
 
@@ -25,34 +24,6 @@ class DiscoverQueryPermission(OrganizationPermission):
 class DiscoverQueryEndpoint(OrganizationEndpoint):
     permission_classes = (DiscoverQueryPermission, )
 
-    def get_json_type(self, snuba_type):
-        """
-        Convert Snuba/Clickhouse type to JSON type
-        Default is string
-        """
-
-        # Ignore Nullable part
-        nullable_match = re.search(r'^Nullable\((.+)\)$', snuba_type)
-
-        if nullable_match:
-            snuba_type = nullable_match.group(1)
-        # Check for array
-
-        array_match = re.search(r'^Array\(.+\)$', snuba_type)
-        if array_match:
-            return 'array'
-
-        types = {
-            'UInt8': 'boolean',
-            'UInt16': 'integer',
-            'UInt32': 'integer',
-            'UInt64': 'integer',
-            'Float32': 'number',
-            'Float64': 'number',
-        }
-
-        return types.get(snuba_type, 'string')
-
     def handle_results(self, snuba_results, requested_query, projects):
         if 'project.name' in requested_query['selected_columns']:
             project_name_index = requested_query['selected_columns'].index('project.name')
@@ -88,7 +59,7 @@ def handle_results(self, snuba_results, requested_query, projects):
 
         # Convert snuba types to json types
         for col in snuba_results['meta']:
-            col['type'] = self.get_json_type(col.get('type'))
+            col['type'] = snuba.get_json_type(col.get('type'))
 
         return snuba_results
 

src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx

@@ -89,6 +89,72 @@ export const ALL_VIEWS = deepFreeze([
   },
 ]);
 
+/**
+ * A mapping of field types to their rendering function.
+ * This mapping is used when a field is not defined in SPECIAL_FIELDS
+ * This mapping should match the output sentry.utils.snuba:get_json_type
+ */
+export const FIELD_FORMATTERS = {
+  boolean: {
+    sortField: true,
+    renderFunc: (field, data, {organization, location}) => {
+      const target = {
+        pathname: `/organizations/${organization.slug}/events/`,
+        query: {
+          ...location.query,
+          query: `${field}:${data[field]}`,
+        },
+      };
+      const value = data[field] ? t('yes') : t('no');
+      return <QueryLink to={target}>{value}</QueryLink>;
+    },
+  },
+  integer: {
+    sortField: true,
+    renderFunc: (field, data) => (
+      <NumberContainer>
+        {typeof data[field] === 'number' ? <Count value={data[field]} /> : null}
+      </NumberContainer>
+    ),
+  },
+  number: {
+    sortField: true,
+    renderFunc: (field, data) => (
+      <NumberContainer>
+        {typeof data[field] === 'number' ? <Count value={data[field]} /> : null}
+      </NumberContainer>
+    ),
+  },
+  date: {
+    sortField: true,
+    renderFunc: (field, data) => (
+      <Container>
+        {data[field] ? (
+          getDynamicText({
+            value: <StyledDateTime date={data[field]} />,
+            fixed: 'timestamp',
+          })
+        ) : (
+          <span>t('n/a')</span>
+        )}
+      </Container>
+    ),
+  },
+  string: {
+    sortField: false,
+    renderFunc: (field, data, {organization, location}) => {
+      const target = {
+        pathname: `/organizations/${organization.slug}/events/`,
+        query: {
+          ...location.query,
+          query: `${field}:${data[field]}`,
+        },
+      };
+      return <QueryLink to={target}>{data[field]}</QueryLink>;
+    },
+  },
+};
+
 /**
  * "Special fields" do not map 1:1 to an single column in the event database,
  * they are a UI concept that combines the results of multiple fields and
@@ -189,19 +255,6 @@ export const SPECIAL_FIELDS = {
       return <QueryLink to={target}>{badge}</QueryLink>;
     },
   },
-  timestamp: {
-    sortField: 'timestamp',
-    renderFunc: data => (
-      <Container>
-        {data.timestamp
-          ? getDynamicText({
-              value: <StyledDateTime date={data.timestamp} />,
-              fixed: 'time',
-            })
-          : null}
-      </Container>
-    ),
-  },
   issue_title: {
     sortField: 'issue_title',
     renderFunc: (data, {organization, location}) => {
@@ -221,25 +274,6 @@ export const SPECIAL_FIELDS = {
       );
     },
   },
-  // TODO generalize this.
-  'count(id)': {
-    sortField: 'count_id',
-    renderFunc: data => (
-      <NumberContainer>
-        {typeof data.count_id === 'number' ? <Count value={data.count_id} /> : null}
-      </NumberContainer>
-    ),
-  },
-  'count_unique(user)': {
-    sortField: 'unique_count_user',
-    renderFunc: data => (
-      <NumberContainer>
-        {typeof data.count_unique_user === 'number' ? (
-          <Count value={data.count_unique_user} />
-        ) : null}
-      </NumberContainer>
-    ),
-  },
   last_seen: {
     sortField: 'last_seen',
     renderFunc: data => {

src/sentry/static/sentry/app/views/organizationEventsV2/relatedEvents.jsx

@@ -62,7 +62,7 @@ class RelatedEvents extends AsyncComponent {
   renderBody() {
     const {location, projects, event} = this.props;
     const {events} = this.state;
-    if (!events) {
+    if (!events || !events.data) {
       return null;
     }
 
@@ -71,10 +71,10 @@ class RelatedEvents extends AsyncComponent {
         <Title>
           <InlineSvg src="icon-link" size="12px" /> {t('Related Events')}
         </Title>
-        {events.length < 1 ? (
+        {events.data.length < 1 ? (
           <Card>{t('No related events found.')}</Card>
         ) : (
-          events.map(item => {
+          events.data.map(item => {
             const eventUrl = {
               pathname: location.pathname,
               query: {

src/sentry/static/sentry/app/views/organizationEventsV2/table.jsx

@@ -10,26 +10,27 @@ import LoadingContainer from 'app/components/loading/loadingContainer';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
 
-import {SPECIAL_FIELDS} from './data';
-import {QueryLink} from './styles';
+import {FIELD_FORMATTERS, SPECIAL_FIELDS} from './data';
+import {getFieldRenderer} from './utils';
 import SortLink from './sortLink';
 
 export default class Table extends React.Component {
   static propTypes = {
     view: SentryTypes.EventView.isRequired,
-    data: PropTypes.arrayOf(PropTypes.object),
+    data: PropTypes.object,
     isLoading: PropTypes.bool.isRequired,
     organization: SentryTypes.Organization.isRequired,
     location: PropTypes.object,
   };
 
   renderBody() {
-    const {view, data, organization, location, isLoading} = this.props;
-    const {fields} = view.data;
+    const {view, organization, location, isLoading} = this.props;
 
-    if (!data || isLoading) {
+    if (!this.props.data || isLoading) {
       return null;
     }
+    const {fields} = view.data;
+    const {data, meta} = this.props.data;
 
     if (data.length === 0) {
       return (
@@ -42,22 +43,8 @@ export default class Table extends React.Component {
     return data.map((row, idx) => (
       <Row key={idx} className={getGridStyle(view)}>
         {fields.map(field => {
-          const target = {
-            pathname: `/organizations/${organization.slug}/events/`,
-            query: {
-              ...location.query,
-              query: `${field}:${row[field]}`,
-            },
-          };
-          return (
-            <Cell key={field}>
-              {SPECIAL_FIELDS.hasOwnProperty(field) ? (
-                SPECIAL_FIELDS[field].renderFunc(row, {organization, location})
-              ) : (
-                <QueryLink to={target}>{row[field]}</QueryLink>
-              )}
-            </Cell>
-          );
+          const fieldRenderer = getFieldRenderer(field, meta);
+          return <Cell key={field}>{fieldRenderer(row, {organization, location})}</Cell>;
         })}
       </Row>
     ));
@@ -73,9 +60,12 @@ export default class Table extends React.Component {
         <TableHeader className={getGridStyle(view)}>
           {fields.map((field, i) => {
             const title = columnNames[i] || field;
+
             let sortKey = field;
             if (SPECIAL_FIELDS.hasOwnProperty(field)) {
               sortKey = SPECIAL_FIELDS[field].sortField;
+            } else if (FIELD_FORMATTERS.hasOwnProperty(field)) {
+              sortKey = FIELD_FORMATTERS[field].sortField ? field : false;
             }
 
             if (sortKey === false) {