Browse Source

feat(dashboard): dashboard widget builder only allow cm on supported functions (#36570)

updates widget builder yaxis and column function arg dropdown to only show custom measurements when supported by the function
edwardgou-sentry 2 years ago
parent
commit
7a3cf28999

+ 6 - 3
static/app/views/dashboardsV2/datasetConfig/base.tsx

@@ -4,11 +4,11 @@ import {Client, ResponseMeta} from 'sentry/api';
 import {SearchBarProps} from 'sentry/components/events/searchBar';
 import {Organization, PageFilters, SelectValue, TagCollection} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
+import {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/customMeasurements';
 import {TableData} from 'sentry/utils/discover/discoverQuery';
 import {MetaType} from 'sentry/utils/discover/eventView';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import {isEquation, QueryFieldValue} from 'sentry/utils/discover/fields';
-import {MeasurementCollection} from 'sentry/utils/measurements/measurements';
 import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
 import {FieldValue} from 'sentry/views/eventsV2/table/types';
 
@@ -46,7 +46,7 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
   getTableFieldOptions: (
     organization: Organization,
     tags?: TagCollection,
-    customMeasurements?: MeasurementCollection
+    customMeasurements?: CustomMeasurementCollection
   ) => Record<string, SelectValue<FieldValue>>;
   /**
    * List of supported display types for dataset.
@@ -81,7 +81,10 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
    * of an aggregate function in QueryField component on the
    * Widget Builder.
    */
-  filterAggregateParams?: (option: FieldValueOption) => boolean;
+  filterAggregateParams?: (
+    option: FieldValueOption,
+    fieldValue?: QueryFieldValue
+  ) => boolean;
   /**
    * Refine the options available in the sort options for timeseries
    * displays on the 'Sort by' step of the Widget Builder.

+ 23 - 8
static/app/views/dashboardsV2/datasetConfig/errorsAndTransactions.tsx

@@ -15,6 +15,7 @@ import {
   TagCollection,
 } from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
+import {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/customMeasurements';
 import {EventsTableData, TableData} from 'sentry/utils/discover/discoverQuery';
 import {MetaType} from 'sentry/utils/discover/eventView';
 import {
@@ -41,10 +42,7 @@ import {
   generateEventSlug,
 } from 'sentry/utils/discover/urls';
 import {getShortEventId} from 'sentry/utils/events';
-import {
-  getMeasurements,
-  MeasurementCollection,
-} from 'sentry/utils/measurements/measurements';
+import {getMeasurements} from 'sentry/utils/measurements/measurements';
 import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
 import {FieldValue, FieldValueKind} from 'sentry/views/eventsV2/table/types';
 import {generateFieldOptions} from 'sentry/views/eventsV2/utils';
@@ -153,6 +151,7 @@ export const ErrorsAndTransactionsConfig: DatasetConfig<
   transformSeries: transformEventsResponseToSeries,
   transformTable: transformEventsResponseToTable,
   filterTableOptions,
+  filterAggregateParams,
 };
 
 function getTableSortOptions(_organization: Organization, widgetQuery: WidgetQuery) {
@@ -236,7 +235,7 @@ function getTimeseriesSortOptions(
 function getEventsTableFieldOptions(
   organization: Organization,
   tags?: TagCollection,
-  customMeasurements?: MeasurementCollection
+  customMeasurements?: CustomMeasurementCollection
 ) {
   const measurements = getMeasurements();
 
@@ -244,12 +243,15 @@ function getEventsTableFieldOptions(
     organization,
     tagKeys: Object.values(tags ?? {}).map(({key}) => key),
     measurementKeys: Object.values(measurements).map(({key}) => key),
-    customMeasurementKeys: organization.features.includes(
+    spanOperationBreakdownKeys: SPAN_OP_BREAKDOWN_FIELDS,
+    customMeasurements: organization.features.includes(
       'dashboard-custom-measurement-widgets'
     )
-      ? Object.values(customMeasurements ?? {}).map(({key}) => key)
+      ? Object.values(customMeasurements ?? {}).map(({key, functions}) => ({
+          key,
+          functions,
+        }))
       : undefined,
-    spanOperationBreakdownKeys: SPAN_OP_BREAKDOWN_FIELDS,
   });
 }
 
@@ -569,3 +571,16 @@ function getEventsSeriesRequest(
 function filterTableOptions(option: FieldValueOption) {
   return option.value.kind !== FieldValueKind.CUSTOM_MEASUREMENT;
 }
+
+// Checks fieldValue to see what function is being used and only allow supported custom measurements
+function filterAggregateParams(option: FieldValueOption, fieldValue?: QueryFieldValue) {
+  if (
+    option.value.kind === FieldValueKind.CUSTOM_MEASUREMENT &&
+    fieldValue?.kind === 'function' &&
+    fieldValue?.function &&
+    !option.value.meta.functions.includes(fieldValue.function[0])
+  ) {
+    return false;
+  }
+  return true;
+}

+ 8 - 2
static/app/views/eventsV2/table/queryField.tsx

@@ -66,7 +66,10 @@ type Props = {
   /**
    * Function to filter the options that are used as parameters for function/aggregate.
    */
-  filterAggregateParameters?: (option: FieldValueOption) => boolean;
+  filterAggregateParameters?: (
+    option: FieldValueOption,
+    fieldValue?: QueryFieldValue
+  ) => boolean;
   /**
    * Filter the options in the primary selector. Useful if you only want to
    * show a subset of selectable items.
@@ -420,6 +423,7 @@ class QueryField extends Component<Props> {
       filterAggregateParameters,
       hideParameterSelector,
       skipParameterPlaceholder,
+      fieldValue,
     } = this.props;
 
     const inputs = parameters.map((descriptor: ParameterDescription, index: number) => {
@@ -428,7 +432,9 @@ class QueryField extends Component<Props> {
           return null;
         }
         const aggregateParameters = filterAggregateParameters
-          ? descriptor.options.filter(filterAggregateParameters)
+          ? descriptor.options.filter(option =>
+              filterAggregateParameters(option, fieldValue)
+            )
           : descriptor.options;
 
         aggregateParameters.forEach(opt => {

+ 1 - 0
static/app/views/eventsV2/table/types.tsx

@@ -61,6 +61,7 @@ export type FieldValueColumns =
       kind: FieldValueKind.CUSTOM_MEASUREMENT;
       meta: {
         dataType: ColumnType;
+        functions: string[];
         name: string;
       };
     }

+ 14 - 8
static/app/views/eventsV2/utils.tsx

@@ -439,7 +439,7 @@ function generateExpandedConditions(
 type FieldGeneratorOpts = {
   organization: OrganizationSummary;
   aggregations?: Record<string, Aggregation>;
-  customMeasurementKeys?: string[] | null;
+  customMeasurements?: {functions: string[]; key: string}[] | null;
   fields?: Record<string, ColumnType>;
   measurementKeys?: string[] | null;
   spanOperationBreakdownKeys?: string[];
@@ -450,8 +450,8 @@ export function generateFieldOptions({
   organization,
   tagKeys,
   measurementKeys,
-  customMeasurementKeys,
   spanOperationBreakdownKeys,
+  customMeasurements,
   aggregations = AGGREGATIONS,
   fields = FIELDS,
 }: FieldGeneratorOpts) {
@@ -519,14 +519,20 @@ export function generateFieldOptions({
     });
   }
 
-  if (customMeasurementKeys !== undefined && customMeasurementKeys !== null) {
-    customMeasurementKeys.sort();
-    customMeasurementKeys.forEach(customMeasurement => {
-      fieldOptions[`measurement:${customMeasurement}`] = {
-        label: customMeasurement,
+  if (customMeasurements !== undefined && customMeasurements !== null) {
+    customMeasurements.sort(({key: currentKey}, {key: nextKey}) =>
+      currentKey > nextKey ? 1 : currentKey === nextKey ? 0 : -1
+    );
+    customMeasurements.forEach(({key, functions: supportedFunctions}) => {
+      fieldOptions[`measurement:${key}`] = {
+        label: key,
         value: {
           kind: FieldValueKind.CUSTOM_MEASUREMENT,
-          meta: {name: customMeasurement, dataType: measurementType(customMeasurement)},
+          meta: {
+            name: key,
+            dataType: measurementType(key),
+            functions: supportedFunctions,
+          },
         },
       };
     });

+ 32 - 0
tests/js/spec/views/dashboardsV2/widgetBuilder/widgetBuilder.spec.tsx

@@ -3537,6 +3537,38 @@ describe('WidgetBuilder', function () {
         screen.getByText('You have inputs that are incompatible with');
         expect(screen.getByText('Add Widget').closest('button')).toBeDisabled();
       });
+
+      it('only displays custom measurements in supported functions', async function () {
+        measurementsMetaMock = MockApiClient.addMockResponse({
+          url: '/organizations/org-slug/measurements-meta/',
+          method: 'GET',
+          body: {
+            'measurements.custom.measurement': {functions: ['p99']},
+            'measurements.another.custom.measurement': {functions: ['p95']},
+          },
+        });
+
+        renderTestComponent({
+          query: {source: DashboardWidgetSource.DISCOVERV2},
+          dashboard: testDashboard,
+          orgFeatures: [...defaultOrgFeatures, 'discover-frontend-use-events-endpoint'],
+        });
+
+        expect(await screen.findAllByText('Custom Widget')).toHaveLength(2);
+
+        await selectEvent.select(screen.getAllByText('count()')[1], ['p99(…)']);
+        userEvent.click(screen.getByText('transaction.duration'));
+        screen.getByText('measurements.custom.measurement');
+        expect(
+          screen.queryByText('measurements.another.custom.measurement')
+        ).not.toBeInTheDocument();
+        await selectEvent.select(screen.getAllByText('p99(…)')[0], ['p95(…)']);
+        userEvent.click(screen.getByText('transaction.duration'));
+        screen.getByText('measurements.another.custom.measurement');
+        expect(
+          screen.queryByText('measurements.custom.measurement')
+        ).not.toBeInTheDocument();
+      });
     });
   });
 });

+ 4 - 1
tests/js/spec/views/eventsV2/utils.spec.jsx

@@ -792,7 +792,9 @@ describe('generateFieldOptions', function () {
     expect(
       generateFieldOptions({
         organization: initializeOrg().organization,
-        customMeasurementKeys: ['measurements.custom.measurement'],
+        customMeasurements: [
+          {functions: ['p99'], key: 'measurements.custom.measurement'},
+        ],
       })['measurement:measurements.custom.measurement']
     ).toEqual({
       label: 'measurements.custom.measurement',
@@ -800,6 +802,7 @@ describe('generateFieldOptions', function () {
         kind: 'custom_measurement',
         meta: {
           dataType: 'number',
+          functions: ['p99'],
           name: 'measurements.custom.measurement',
         },
       },