Browse Source

fix(ddm): polish dashboard integration (#60689)

Ogi 1 year ago
parent
commit
8326106a18

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

@@ -3,6 +3,7 @@ import isEqual from 'lodash/isEqual';
 import {RELEASE_ADOPTION_STAGES} from 'sentry/constants';
 import {MetricType, Organization, SelectValue} from 'sentry/types';
 import {assert} from 'sentry/types/utils';
+import {isMRI} from 'sentry/utils/metrics/mri';
 import {
   SESSIONS_FIELDS,
   SESSIONS_OPERATIONS,
@@ -1061,6 +1062,10 @@ export function aggregateFunctionOutputType(
     return STARFISH_AGGREGATION_FIELDS[funcName].defaultOutputType;
   }
 
+  if (firstArg && isMRI(firstArg)) {
+    return 'number';
+  }
+
   // If the function is an inherit type it will have a field as
   // the first parameter and we can use that to get the type.
   const fieldDef = getFieldDefinition(firstArg ?? '');

+ 15 - 0
static/app/utils/metrics/index.tsx

@@ -32,6 +32,21 @@ export enum MetricDisplayType {
 
 export const defaultMetricDisplayType = MetricDisplayType.LINE;
 
+export const getMetricDisplayType = (displayType: unknown): MetricDisplayType => {
+  if (
+    [
+      MetricDisplayType.AREA,
+      MetricDisplayType.BAR,
+      MetricDisplayType.LINE,
+      MetricDisplayType.TABLE,
+    ].includes(displayType as MetricDisplayType)
+  ) {
+    return displayType as MetricDisplayType;
+  }
+
+  return MetricDisplayType.LINE;
+};
+
 export type MetricTag = {
   key: string;
 };

+ 6 - 5
static/app/views/dashboards/datasetConfig/base.tsx

@@ -80,11 +80,6 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
     disableSortDirection: boolean;
     disableSortReason?: string;
   };
-  /**
-   * Used for mapping column names to more desirable
-   * values in tables.
-   */
-  fieldHeaderMap?: Record<string, string>;
   /**
    * Filter the options available to the parameters list
    * of an aggregate function in QueryField component on the
@@ -126,6 +121,11 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
     meta: MetaType,
     organization?: Organization
   ) => ReturnType<typeof getFieldRenderer> | null;
+  /**
+   * Generate field header used for mapping column
+   * names to more desirable values in tables.
+   */
+  getFieldHeaderMap?: (widgetQuery?: WidgetQuery) => Record<string, string>;
   /**
    * Field options to display in the Group by selector.
    */
@@ -200,6 +200,7 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
    * to reset the orderby of the widget query.
    */
   handleOrderByReset?: (widgetQuery: WidgetQuery, newFields: string[]) => WidgetQuery;
+
   /**
    * Transforms timeseries API results into series data that is
    * ingestable by echarts for timeseries visualizations.

+ 1 - 1
static/app/views/dashboards/datasetConfig/issues.tsx

@@ -57,7 +57,7 @@ export const IssuesConfig: DatasetConfig<never, Group[]> = {
   getTableSortOptions,
   getTableFieldOptions: (_organization: Organization) =>
     generateIssueWidgetFieldOptions(),
-  fieldHeaderMap: ISSUE_FIELD_TO_HEADER_MAP,
+  getFieldHeaderMap: () => ISSUE_FIELD_TO_HEADER_MAP,
   supportedDisplayTypes: [DisplayType.TABLE],
   transformTable: transformIssuesResponseToTable,
 };

+ 28 - 43
static/app/views/dashboards/datasetConfig/metrics.tsx

@@ -8,7 +8,12 @@ import {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/custo
 import {TableData} from 'sentry/utils/discover/discoverQuery';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import {getMetricsApiRequestQuery, getSeriesName, groupByOp} from 'sentry/utils/metrics';
-import {formatMRI, getMRI, getUseCaseFromMRI} from 'sentry/utils/metrics/mri';
+import {
+  formatMRI,
+  formatMRIField,
+  getMRI,
+  getUseCaseFromMRI,
+} from 'sentry/utils/metrics/mri';
 import {OnDemandControlContext} from 'sentry/utils/performance/contexts/onDemandControl';
 import {MetricSearchBar} from 'sentry/views/dashboards/widgetBuilder/buildSteps/filterResultsStep/metricSearchBar';
 import {FieldValueOption} from 'sentry/views/discover/table/queryField';
@@ -62,8 +67,21 @@ export const MetricsConfig: DatasetConfig<MetricsApiResponse, MetricsApiResponse
   filterAggregateParams: filterMetricMRIs,
   filterYAxisAggregateParams: () => option => filterMetricMRIs(option),
   getGroupByFieldOptions: getTagsForMetric,
+  getFieldHeaderMap: getFormattedMRIHeaders,
 };
 
+function getFormattedMRIHeaders(query?: WidgetQuery) {
+  if (!query) {
+    return {};
+  }
+
+  return (query.fields || []).reduce((acc, field, index) => {
+    const fieldAlias = query.fieldAliases?.[index];
+    acc[field] = fieldAlias || formatMRIField(field);
+    return acc;
+  }, {});
+}
+
 function getMetricTimeseriesSortOptions(_, widgetQuery) {
   if (!widgetQuery.columns) {
     return [];
@@ -220,15 +238,10 @@ function handleMetricTableOrderByReset(widgetQuery: WidgetQuery, newFields: stri
   return handleOrderByReset(widgetQuery, newFields);
 }
 
-export function transformMetricsResponseToTable(
-  data: MetricsApiResponse,
-  {aggregates}: WidgetQuery
-): TableData {
-  // TODO(ddm): get rid of this mapping, it is only needed because the API returns
-  // `op(metric_name)` instead of `op(mri)`
-  const rows = mapResponse(data, aggregates).groups.map((group, index) => {
-    const groupColumn = mapDerivedMetricsToFields(group.by);
-    const value = mapDerivedMetricsToFields(group.totals);
+export function transformMetricsResponseToTable(data: MetricsApiResponse): TableData {
+  const rows = data.groups.map((group, index) => {
+    const groupColumn = mapMetricGroupsToFields(group.by);
+    const value = mapMetricGroupsToFields(group.totals);
     return {
       id: String(index),
       ...groupColumn,
@@ -243,9 +256,8 @@ export function transformMetricsResponseToTable(
   return {meta, data: rows};
 }
 
-function mapDerivedMetricsToFields(
-  results: Record<string, number | string | null> | undefined,
-  mapToKey?: string
+function mapMetricGroupsToFields(
+  results: Record<string, number | string | null> | undefined
 ) {
   if (!results) {
     return {};
@@ -253,7 +265,7 @@ function mapDerivedMetricsToFields(
 
   const mappedResults: typeof results = {};
   for (const [key, value] of Object.entries(results)) {
-    mappedResults[mapToKey ?? key] = value;
+    mappedResults[key] = value;
   }
   return mappedResults;
 }
@@ -261,8 +273,8 @@ function mapDerivedMetricsToFields(
 function changeObjectValuesToTypes(
   obj: Record<string, number | string | null> | undefined
 ) {
-  return Object.keys(obj ?? {}).reduce((acc, key) => {
-    acc[key] = key.includes('@') ? 'number' : 'string';
+  return Object.entries(obj ?? {}).reduce((acc, [key, value]) => {
+    acc[key] = typeof value;
     return acc;
   }, {});
 }
@@ -352,30 +364,3 @@ function getMetricRequest(
     query: requestData,
   });
 }
-
-const mapResponse = (data: MetricsApiResponse, field: string[]): MetricsApiResponse => {
-  const mappedGroups = data.groups.map(group => {
-    return {
-      ...group,
-      by: group.by,
-      series: swapKeys(group.series, field),
-      totals: swapKeys(group.totals, field),
-    };
-  });
-
-  return {...data, groups: mappedGroups};
-};
-
-const swapKeys = (obj: Record<string, unknown> | undefined, newKeys: string[]) => {
-  if (!obj) {
-    return {};
-  }
-
-  const keys = Object.keys(obj);
-  const values = Object.values(obj);
-  const newObj = {};
-  keys.forEach((_, index) => {
-    newObj[newKeys[index]] = values[index];
-  });
-  return newObj;
-};

+ 2 - 3
static/app/views/dashboards/utils.tsx

@@ -44,7 +44,7 @@ import {DiscoverDatasets, DisplayModes} from 'sentry/utils/discover/types';
 import {getMeasurements} from 'sentry/utils/measurements/measurements';
 import {
   getDdmUrl,
-  MetricDisplayType,
+  getMetricDisplayType,
   MetricWidgetQueryParams,
 } from 'sentry/utils/metrics';
 import {parseField} from 'sentry/utils/metrics/mri';
@@ -427,8 +427,7 @@ export function getWidgetDDMUrl(
         op,
         groupBy: query.columns,
         query: query.conditions ?? '',
-        // TODO(oggi): Handle display type mismatch and remove cast
-        displayType: _widget.displayType as unknown as MetricDisplayType,
+        displayType: getMetricDisplayType(_widget.displayType),
       } satisfies MetricWidgetQueryParams;
     }),
   });

+ 5 - 3
static/app/views/dashboards/widgetBuilder/widgetBuilder.tsx

@@ -903,13 +903,15 @@ function WidgetBuilder({
             ...queryParamsWithoutSource,
             ...query,
           }
-        : undefined;
+        : {};
+
+    const sanitizedQuery = omit(pathQuery, ['defaultWidgetQuery', 'defaultTitle']);
 
     if (id === NEW_DASHBOARD_ID) {
       router.push(
         normalizeUrl({
           pathname: `/organizations/${organization.slug}/dashboards/new/`,
-          query: pathQuery,
+          query: sanitizedQuery,
         })
       );
       return;
@@ -918,7 +920,7 @@ function WidgetBuilder({
     router.push(
       normalizeUrl({
         pathname: `/organizations/${organization.slug}/dashboard/${id}/`,
-        query: pathQuery,
+        query: sanitizedQuery,
       })
     );
   }

+ 1 - 0
static/app/views/dashboards/widgetCard/chart.tsx

@@ -170,6 +170,7 @@ class WidgetCardChart extends Component<WidgetCardChartProps, State> {
           data={result.data}
           organization={organization}
           stickyHeaders
+          fieldHeaderMap={datasetConfig.getFieldHeaderMap?.(widget.queries[i])}
           getCustomFieldRenderer={datasetConfig.getCustomFieldRenderer}
         />
       );

+ 1 - 1
static/app/views/dashboards/widgetCard/issueWidgetCard.tsx

@@ -68,7 +68,7 @@ export function IssueWidgetCard({
       data={transformedResults}
       organization={organization}
       getCustomFieldRenderer={datasetConfig.getCustomFieldRenderer}
-      fieldHeaderMap={datasetConfig.fieldHeaderMap}
+      fieldHeaderMap={datasetConfig.getFieldHeaderMap?.()}
       stickyHeaders
     />
   );

+ 18 - 92
static/app/views/dashboards/widgetCard/metricWidgetQueries.tsx

@@ -1,19 +1,10 @@
 import {Component} from 'react';
-import cloneDeep from 'lodash/cloneDeep';
 import isEqual from 'lodash/isEqual';
 import omit from 'lodash/omit';
 
-import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {Client} from 'sentry/api';
 import {isSelectionEqual} from 'sentry/components/organizations/pageFilters/utils';
-import {t} from 'sentry/locale';
-import {
-  MetricsApiResponse,
-  Organization,
-  PageFilters,
-  Release,
-  SessionApiResponse,
-} from 'sentry/types';
+import {MetricsApiResponse, Organization, PageFilters} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
 import {TableDataWithTitle} from 'sentry/utils/discover/discoverQuery';
 import {TOP_N} from 'sentry/utils/discover/types';
@@ -44,73 +35,15 @@ type Props = {
 type State = {
   loading: boolean;
   errorMessage?: string;
-  releases?: Release[];
 };
 
-export function derivedMetricsToField(field: string): string {
-  return field;
-}
-
-export function resolveDerivedStatusFields(fields: string[]): {
-  aggregates: string[];
-  derivedStatusFields: string[];
-  injectedFields: string[];
-} {
-  return {aggregates: fields, derivedStatusFields: [], injectedFields: []};
-}
-
 class MetricWidgetQueries extends Component<Props, State> {
   state: State = {
     loading: true,
     errorMessage: undefined,
-    releases: undefined,
   };
 
-  componentDidMount() {
-    this._isMounted = true;
-  }
-
-  componentWillUnmount() {
-    this._isMounted = false;
-  }
-
   config = MetricsConfig;
-  private _isMounted: boolean = false;
-
-  fetchReleases = async () => {
-    this.setState({loading: true, errorMessage: undefined});
-    const {selection, api, organization} = this.props;
-    const {environments, projects} = selection;
-
-    try {
-      const releases = await api.requestPromise(
-        `/organizations/${organization.slug}/releases/`,
-        {
-          method: 'GET',
-          data: {
-            sort: 'date',
-            project: projects,
-            per_page: 50,
-            environment: environments,
-          },
-        }
-      );
-      if (!this._isMounted) {
-        return;
-      }
-      this.setState({releases, loading: false});
-    } catch (error) {
-      if (!this._isMounted) {
-        return;
-      }
-
-      const message = error.responseJSON
-        ? error.responseJSON.error
-        : t('Error sorting by releases');
-      this.setState({errorMessage: message, loading: false});
-      addErrorMessage(message);
-    }
-  };
 
   get limit() {
     const {limit} = this.props;
@@ -177,30 +110,12 @@ class MetricWidgetQueries extends Component<Props, State> {
     );
   };
 
-  transformWidget = (initialWidget: Widget): Widget => {
-    const widget = cloneDeep(initialWidget);
-
-    const releaseCondition = '';
-
-    widget.queries.forEach(query => {
-      query.conditions =
-        query.conditions + (releaseCondition === '' ? '' : ` ${releaseCondition}`);
+  afterFetchData = (data: MetricsApiResponse) => {
+    const fields = this.props.widget.queries[0].aggregates;
+    data.groups.forEach(group => {
+      group.series = swapKeys(group.series, fields);
+      group.totals = swapKeys(group.totals, fields);
     });
-
-    return widget;
-  };
-
-  afterFetchData = (data: SessionApiResponse | MetricsApiResponse) => {
-    const releasesArray: string[] = [];
-
-    if (releasesArray.length) {
-      data.groups.sort(function (group1, group2) {
-        const release1 = group1.by.release;
-        const release2 = group2.by.release;
-        return releasesArray.indexOf(release1) - releasesArray.indexOf(release2);
-      });
-      data.groups = data.groups.slice(0, this.limit);
-    }
   };
 
   render() {
@@ -222,7 +137,7 @@ class MetricWidgetQueries extends Component<Props, State> {
         api={api}
         organization={organization}
         selection={selection}
-        widget={this.transformWidget(widget)}
+        widget={widget}
         dashboardFilters={dashboardFilters}
         cursor={cursor}
         limit={this.limit}
@@ -244,3 +159,14 @@ class MetricWidgetQueries extends Component<Props, State> {
 }
 
 export default MetricWidgetQueries;
+
+const swapKeys = (obj: Record<string, unknown> | undefined, newKeys: string[]) => {
+  if (!obj) {
+    return {};
+  }
+
+  return Object.keys(obj).reduce((acc, key, index) => {
+    acc[newKeys[index]] = obj[key];
+    return acc;
+  }, {});
+};

Some files were not shown because too many files changed in this diff