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

fix(discover): Ensure span op breakdown has duration type when added as a column in Discover (#25374)

Alberto Leal 3 лет назад
Родитель
Сommit
13bc657d0c

+ 2 - 0
static/app/utils/discover/fields.tsx

@@ -742,6 +742,8 @@ export function aggregateFunctionOutputType(
     return FIELDS[firstArg];
   } else if (firstArg && isMeasurement(firstArg)) {
     return measurementType(firstArg);
+  } else if (firstArg && isSpanOperationBreakdownField(firstArg)) {
+    return 'duration';
   }
 
   return null;

+ 9 - 1
static/app/views/eventsV2/table/cellAction.tsx

@@ -10,6 +10,7 @@ import {t} from 'app/locale';
 import space from 'app/styles/space';
 import {TableDataRow} from 'app/utils/discover/discoverQuery';
 import {getAggregateAlias} from 'app/utils/discover/fields';
+import {getDuration} from 'app/utils/formatters';
 import {QueryResults} from 'app/utils/tokenizeSearch';
 
 import {TableColumn} from './types';
@@ -27,9 +28,16 @@ export enum Actions {
 export function updateQuery(
   results: QueryResults,
   action: Actions,
-  key: string,
+  column: TableColumn<keyof TableDataRow>,
   value: React.ReactText | string[]
 ) {
+  const key = column.name;
+
+  if (column.type === 'duration' && typeof value === 'number') {
+    // values are assumed to be in milliseconds
+    value = getDuration(value / 1000, 2, true);
+  }
+
   // De-duplicate array values
   if (Array.isArray(value)) {
     value = [...new Set(value)];

+ 3 - 2
static/app/views/eventsV2/table/tableView.tsx

@@ -400,8 +400,9 @@ class TableView extends React.Component<TableViewProps> {
 
           return;
         }
-        default:
-          updateQuery(query, action, column.name, value);
+        default: {
+          updateQuery(query, action, column, value);
+        }
       }
       nextView.query = stringifyQueryObject(query);
 

+ 3 - 0
static/app/views/eventsV2/utils.tsx

@@ -21,6 +21,7 @@ import {
   FIELDS,
   getAggregateAlias,
   isMeasurement,
+  isSpanOperationBreakdownField,
   measurementType,
   TRACING_FIELDS,
 } from 'app/utils/discover/fields';
@@ -76,6 +77,8 @@ export function decodeColumnOrder(
         column.type = FIELDS[col.field];
       } else if (isMeasurement(col.field)) {
         column.type = measurementType(col.field);
+      } else if (isSpanOperationBreakdownField(col.field)) {
+        column.type = 'duration';
       }
     }
     column.column = col;

+ 1 - 1
static/app/views/performance/table.tsx

@@ -77,7 +77,7 @@ class Table extends React.Component<Props, State> {
       // remove any event.type queries since it is implied to apply to only transactions
       searchConditions.removeTag('event.type');
 
-      updateQuery(searchConditions, action, column.name, value);
+      updateQuery(searchConditions, action, column, value);
 
       ReactRouter.browserHistory.push({
         pathname: location.pathname,

+ 1 - 1
static/app/views/performance/transactionSummary/content.tsx

@@ -119,7 +119,7 @@ class SummaryContent extends React.Component<Props, State> {
       // no need to include transaction as its already in the query params
       searchConditions.removeTag('transaction');
 
-      updateQuery(searchConditions, action, column.name, value);
+      updateQuery(searchConditions, action, column, value);
 
       browserHistory.push({
         pathname: location.pathname,

+ 1 - 1
static/app/views/performance/vitalDetail/table.tsx

@@ -109,7 +109,7 @@ class Table extends React.Component<Props, State> {
       // remove any event.type queries since it is implied to apply to only transactions
       searchConditions.removeTag('event.type');
 
-      updateQuery(searchConditions, action, column.name, value);
+      updateQuery(searchConditions, action, column, value);
 
       ReactRouter.browserHistory.push({
         pathname: location.pathname,

+ 64 - 26
tests/js/spec/views/eventsV2/table/cellAction.spec.jsx

@@ -351,81 +351,119 @@ describe('Discover -> CellAction', function () {
 });
 
 describe('updateQuery()', function () {
+  const columnA = {
+    key: 'a',
+    name: 'a',
+    type: 'number',
+    isSortable: false,
+    column: {
+      kind: 'field',
+      field: 'a',
+    },
+    width: -1,
+  };
+
+  const columnB = {
+    key: 'b',
+    name: 'b',
+    type: 'number',
+    isSortable: false,
+    column: {
+      kind: 'field',
+      field: 'b',
+    },
+    width: -1,
+  };
+
   it('modifies the query with has/!has', function () {
     let results = new QueryResults([]);
-    updateQuery(results, Actions.ADD, 'a', null);
+    updateQuery(results, Actions.ADD, columnA, null);
     expect(results.formatString()).toEqual('!has:a');
-    updateQuery(results, Actions.EXCLUDE, 'a', null);
+    updateQuery(results, Actions.EXCLUDE, columnA, null);
     expect(results.formatString()).toEqual('has:a');
-    updateQuery(results, Actions.ADD, 'a', null);
+    updateQuery(results, Actions.ADD, columnA, null);
     expect(results.formatString()).toEqual('!has:a');
 
     results = new QueryResults([]);
-    updateQuery(results, Actions.ADD, 'a', [null]);
+    updateQuery(results, Actions.ADD, columnA, [null]);
     expect(results.formatString()).toEqual('!has:a');
   });
 
   it('modifies the query with additions', function () {
     const results = new QueryResults([]);
-    updateQuery(results, Actions.ADD, 'a', '1');
+    updateQuery(results, Actions.ADD, columnA, '1');
     expect(results.formatString()).toEqual('a:1');
-    updateQuery(results, Actions.ADD, 'b', '1');
+    updateQuery(results, Actions.ADD, columnB, '1');
     expect(results.formatString()).toEqual('a:1 b:1');
-    updateQuery(results, Actions.ADD, 'a', '2');
+    updateQuery(results, Actions.ADD, columnA, '2');
     expect(results.formatString()).toEqual('b:1 a:2');
-    updateQuery(results, Actions.ADD, 'a', ['1', '2', '3']);
+    updateQuery(results, Actions.ADD, columnA, ['1', '2', '3']);
     expect(results.formatString()).toEqual('b:1 a:2 a:1 a:3');
   });
 
   it('modifies the query with exclusions', function () {
     const results = new QueryResults([]);
-    updateQuery(results, Actions.EXCLUDE, 'a', '1');
+    updateQuery(results, Actions.EXCLUDE, columnA, '1');
     expect(results.formatString()).toEqual('!a:1');
-    updateQuery(results, Actions.EXCLUDE, 'b', '1');
+    updateQuery(results, Actions.EXCLUDE, columnB, '1');
     expect(results.formatString()).toEqual('!a:1 !b:1');
-    updateQuery(results, Actions.EXCLUDE, 'a', '2');
+    updateQuery(results, Actions.EXCLUDE, columnA, '2');
     expect(results.formatString()).toEqual('!b:1 !a:1 !a:2');
-    updateQuery(results, Actions.EXCLUDE, 'a', ['1', '2', '3']);
+    updateQuery(results, Actions.EXCLUDE, columnA, ['1', '2', '3']);
     expect(results.formatString()).toEqual('!b:1 !a:1 !a:2 !a:3');
   });
 
   it('modifies the query with a mix of additions and exclusions', function () {
     const results = new QueryResults([]);
-    updateQuery(results, Actions.ADD, 'a', '1');
+    updateQuery(results, Actions.ADD, columnA, '1');
     expect(results.formatString()).toEqual('a:1');
-    updateQuery(results, Actions.ADD, 'b', '2');
+    updateQuery(results, Actions.ADD, columnB, '2');
     expect(results.formatString()).toEqual('a:1 b:2');
-    updateQuery(results, Actions.EXCLUDE, 'a', '3');
+    updateQuery(results, Actions.EXCLUDE, columnA, '3');
     expect(results.formatString()).toEqual('b:2 !a:3');
-    updateQuery(results, Actions.EXCLUDE, 'b', '4');
+    updateQuery(results, Actions.EXCLUDE, columnB, '4');
     expect(results.formatString()).toEqual('!a:3 !b:4');
-    updateQuery(results, Actions.ADD, 'a', '5');
+    updateQuery(results, Actions.ADD, columnA, '5');
     expect(results.formatString()).toEqual('!b:4 a:5');
-    updateQuery(results, Actions.ADD, 'b', '6');
+    updateQuery(results, Actions.ADD, columnB, '6');
     expect(results.formatString()).toEqual('a:5 b:6');
   });
 
   it('modifies the query with greater/less than', function () {
     const results = new QueryResults([]);
-    updateQuery(results, Actions.SHOW_GREATER_THAN, 'a', 1);
+    updateQuery(results, Actions.SHOW_GREATER_THAN, columnA, 1);
     expect(results.formatString()).toEqual('a:>1');
-    updateQuery(results, Actions.SHOW_GREATER_THAN, 'a', 2);
+    updateQuery(results, Actions.SHOW_GREATER_THAN, columnA, 2);
     expect(results.formatString()).toEqual('a:>2');
-    updateQuery(results, Actions.SHOW_LESS_THAN, 'a', 3);
+    updateQuery(results, Actions.SHOW_LESS_THAN, columnA, 3);
     expect(results.formatString()).toEqual('a:<3');
-    updateQuery(results, Actions.SHOW_LESS_THAN, 'a', 4);
+    updateQuery(results, Actions.SHOW_LESS_THAN, columnA, 4);
     expect(results.formatString()).toEqual('a:<4');
   });
 
+  it('modifies the query with greater/less than on duration fields', function () {
+    const columnADuration = {...columnA, type: 'duration'};
+
+    const results = new QueryResults([]);
+    updateQuery(results, Actions.SHOW_GREATER_THAN, columnADuration, 1);
+    expect(results.formatString()).toEqual('a:>1.00ms');
+    updateQuery(results, Actions.SHOW_GREATER_THAN, columnADuration, 2);
+    expect(results.formatString()).toEqual('a:>2.00ms');
+    updateQuery(results, Actions.SHOW_LESS_THAN, columnADuration, 3);
+    expect(results.formatString()).toEqual('a:<3.00ms');
+    updateQuery(results, Actions.SHOW_LESS_THAN, columnADuration, 4.1234);
+    expect(results.formatString()).toEqual('a:<4.12ms');
+  });
+
   it('does not error for special actions', function () {
     const results = new QueryResults([]);
-    updateQuery(results, Actions.TRANSACTION, '', '');
-    updateQuery(results, Actions.RELEASE, '', '');
-    updateQuery(results, Actions.DRILLDOWN, '', '');
+    updateQuery(results, Actions.TRANSACTION, columnA, '');
+    updateQuery(results, Actions.RELEASE, columnA, '');
+    updateQuery(results, Actions.DRILLDOWN, columnA, '');
   });
 
   it('errors for unknown actions', function () {
     const results = new QueryResults([]);
-    expect(() => updateQuery(results, 'unknown', '', '')).toThrow();
+    expect(() => updateQuery(results, 'unknown', columnA, '')).toThrow();
   });
 });

+ 54 - 0
tests/js/spec/views/eventsV2/utils.spec.jsx

@@ -53,6 +53,24 @@ describe('decodeColumnOrder', function () {
     });
   });
 
+  it('can decode span op breakdown fields', function () {
+    const results = decodeColumnOrder([{field: 'spans.foo', width: 123}]);
+
+    expect(Array.isArray(results)).toBeTruthy();
+
+    expect(results[0]).toEqual({
+      key: 'spans.foo',
+      name: 'spans.foo',
+      column: {
+        kind: 'field',
+        field: 'spans.foo',
+      },
+      width: 123,
+      isSortable: false,
+      type: 'duration',
+    });
+  });
+
   it('can decode aggregate functions with no arguments', function () {
     let results = decodeColumnOrder([{field: 'count()', width: 123}]);
 
@@ -149,6 +167,42 @@ describe('decodeColumnOrder', function () {
       type: 'duration',
     });
   });
+
+  it('can decode elements with aggregate functions using span op breakdowns', function () {
+    const results = decodeColumnOrder([{field: 'avg(spans.foo)'}]);
+
+    expect(Array.isArray(results)).toBeTruthy();
+
+    expect(results[0]).toEqual({
+      key: 'avg(spans.foo)',
+      name: 'avg(spans.foo)',
+      column: {
+        kind: 'function',
+        function: ['avg', 'spans.foo', undefined],
+      },
+      width: COL_WIDTH_UNDEFINED,
+      isSortable: true,
+      type: 'duration',
+    });
+  });
+
+  it('can decode elements with aggregate functions with multiple arguments using span op breakdowns', function () {
+    const results = decodeColumnOrder([{field: 'percentile(spans.lcp, 0.65)'}]);
+
+    expect(Array.isArray(results)).toBeTruthy();
+
+    expect(results[0]).toEqual({
+      key: 'percentile(spans.lcp, 0.65)',
+      name: 'percentile(spans.lcp, 0.65)',
+      column: {
+        kind: 'function',
+        function: ['percentile', 'spans.lcp', '0.65'],
+      },
+      width: COL_WIDTH_UNDEFINED,
+      isSortable: true,
+      type: 'duration',
+    });
+  });
 });
 
 describe('pushEventViewToLocation', function () {