fix(metrics): default aggregation (#71240)

static/app/types/metrics.tsx

@@ -1,6 +1,6 @@
 import type {DateString} from 'sentry/types/core';
 
-export type MetricsOperation =
+export type MetricsAggregate =
   | 'sum'
   | 'count_unique'
   | 'avg'
@@ -102,7 +102,7 @@ export type MetricMeta = {
   mri: MRI;
   // name is returned by the API but should not be used, use parseMRI(mri).name instead
   // name: string;
-  operations: MetricsOperation[];
+  operations: MetricsAggregate[];
   projectIds: number[];
   type: MetricType;
   unit: string;

static/app/utils/metrics/index.spec.tsx

@@ -1,13 +1,15 @@
 import {resetMockDate, setMockDate} from 'sentry-test/utils';
 
-import type {MetricsOperation} from 'sentry/types/metrics';
+import type {MetricsAggregate, MetricType, MRI} from 'sentry/types/metrics';
 import {
   getAbsoluteDateTimeRange,
   getDateTimeParams,
+  getDefaultAggregate,
   getFormattedMQL,
   getMetricsInterval,
   isFormattedMQL,
 } from 'sentry/utils/metrics';
+import {DEFAULT_AGGREGATES} from 'sentry/utils/metrics/constants';
 
 describe('getDDMInterval', () => {
   it('should return the correct interval for non-"1m" intervals', () => {
@@ -77,7 +79,7 @@ describe('getFormattedMQL', () => {
 
   it('defaults to an empty string', () => {
     const result = getFormattedMQL({
-      op: '' as MetricsOperation,
+      op: '' as MetricsAggregate,
       mri: 'd:custom/sentry.process_profile.symbolicate.process@second',
       groupBy: [],
       query: '',
@@ -155,3 +157,18 @@ describe('getAbsoluteDateTimeRange', () => {
     });
   });
 });
+
+describe('getDefaultAggregate', () => {
+  it.each(['c', 'd', 'g', 's'])(
+    'should give default aggregate - metric type %s',
+    metricType => {
+      const mri = `${metricType as MetricType}:custom/xyz@test` as MRI;
+
+      expect(getDefaultAggregate(mri)).toBe(DEFAULT_AGGREGATES[metricType]);
+    }
+  );
+
+  it('should fallback to sum', () => {
+    expect(getDefaultAggregate('b:roken/MRI@none' as MRI)).toBe('sum');
+  });
+});

static/app/utils/metrics/index.tsx

@@ -23,8 +23,8 @@ import {t} from 'sentry/locale';
 import type {PageFilters} from 'sentry/types/core';
 import type {
   MetricMeta,
+  MetricsAggregate,
   MetricsDataIntervalLadder,
-  MetricsOperation,
   MetricsQueryApiResponse,
   MetricsQueryApiResponseLastMeta,
   MRI,
@@ -33,6 +33,7 @@ import type {
 import {statsPeriodToDays} from 'sentry/utils/dates';
 import {isMeasurement} from 'sentry/utils/discover/fields';
 import {getMeasurements} from 'sentry/utils/measurements/measurements';
+import {DEFAULT_AGGREGATES} from 'sentry/utils/metrics/constants';
 import {formatMRI, formatMRIField, MRIToField, parseMRI} from 'sentry/utils/metrics/mri';
 import type {
   MetricsQuery,
@@ -149,18 +150,16 @@ export function getDateTimeParams({start, end, period}: PageFilters['datetime'])
     : {start: moment(start).toISOString(), end: moment(end).toISOString()};
 }
 
-export function getDefaultMetricOp(mri: MRI): MetricsOperation {
+export function getDefaultAggregate(mri: MRI): MetricsAggregate {
   const parsedMRI = parseMRI(mri);
-  switch (parsedMRI?.type) {
-    case 'd':
-    case 'g':
-      return 'avg';
-    case 's':
-      return 'count_unique';
-    case 'c':
-    default:
-      return 'sum';
+
+  const fallbackAggregate = 'sum';
+
+  if (!parsedMRI) {
+    return fallbackAggregate;
   }
+
+  return DEFAULT_AGGREGATES[parsedMRI.type] || fallbackAggregate;
 }
 
 export function isAllowedOp(op: string) {

static/app/utils/metrics/mri.spec.tsx

@@ -1,12 +1,5 @@
 import type {MetricType, MRI, ParsedMRI, UseCase} from 'sentry/types/metrics';
-import {DEFAULT_AGGREGATES} from 'sentry/utils/metrics/constants';
-import {
-  defaultAggregateForMRI,
-  getUseCaseFromMRI,
-  parseField,
-  parseMRI,
-  toMRI,
-} from 'sentry/utils/metrics/mri';
+import {getUseCaseFromMRI, parseField, parseMRI, toMRI} from 'sentry/utils/metrics/mri';
 
 describe('parseMRI', () => {
   it('should handle falsy values', () => {
@@ -211,18 +204,3 @@ describe('toMRI', () => {
     }
   );
 });
-
-describe('defaultAggregateForMRI', () => {
-  it.each(['c', 'd', 'g', 's'])(
-    'should give default aggregate - metric type %s',
-    metricType => {
-      const mri = `${metricType as MetricType}:custom/xyz@test` as MRI;
-
-      expect(defaultAggregateForMRI(mri)).toBe(DEFAULT_AGGREGATES[metricType]);
-    }
-  );
-
-  it('should fallback to sum', () => {
-    expect(defaultAggregateForMRI('b:roken/MRI@none' as MRI)).toBe('sum');
-  });
-});

static/app/utils/metrics/mri.tsx

@@ -1,7 +1,6 @@
 import {t} from 'sentry/locale';
 import type {MetricType, MRI, ParsedMRI, UseCase} from 'sentry/types/metrics';
 import {parseFunction} from 'sentry/utils/discover/fields';
-import {DEFAULT_AGGREGATES} from 'sentry/utils/metrics/constants';
 
 export const DEFAULT_MRI: MRI = 'c:custom/sentry_metric@none';
 // This is a workaround as the alert builder requires a valid aggregate to be set
@@ -112,15 +111,3 @@ export function formatMRIField(aggregate: string) {
 
   return `${parsed.op}(${formatMRI(parsed.mri)})`;
 }
-
-export function defaultAggregateForMRI(mri: MRI) {
-  const parsedMRI = parseMRI(mri);
-
-  const fallbackAggregate = 'sum';
-
-  if (!parsedMRI) {
-    return fallbackAggregate;
-  }
-
-  return DEFAULT_AGGREGATES[parsedMRI.type] || fallbackAggregate;
-}

static/app/views/metrics/customMetricsEventData.tsx

@@ -23,7 +23,7 @@ import type {
   Organization,
 } from 'sentry/types';
 import {defined} from 'sentry/utils';
-import {getDefaultMetricOp, getMetricsUrl} from 'sentry/utils/metrics';
+import {getDefaultAggregate, getMetricsUrl} from 'sentry/utils/metrics';
 import {hasCustomMetrics} from 'sentry/utils/metrics/features';
 import {formatMetricUsingUnit} from 'sentry/utils/metrics/formatters';
 import {formatMRI, parseMRI} from 'sentry/utils/metrics/mri';
@@ -97,7 +97,7 @@ export function CustomMetricsEventData({
       metricsSummaryEntries.map((entry, index) => ({
         mri: entry.mri,
         name: index.toString(),
-        op: getDefaultMetricOp(entry.mri),
+        op: getDefaultAggregate(entry.mri),
         query: Object.entries(entry.item.tags ?? {})
           .map(([tagKey, tagValue]) => tagToQuery(tagKey, tagValue))
           .join(' '),
@@ -215,7 +215,7 @@ export function CustomMetricsEventData({
                 {
                   mri: mri,
                   displayType: MetricDisplayType.LINE,
-                  op: getDefaultMetricOp(mri),
+                  op: getDefaultAggregate(mri),
                   query: Object.entries(summaryItem.tags ?? {})
                     .map(([tagKey, tagValue]) => tagToQuery(tagKey, tagValue))
                     .join(' '),
@@ -288,7 +288,7 @@ function MetricRenderer({
             {
               mri: mri,
               displayType: MetricDisplayType.LINE,
-              op: getDefaultMetricOp(mri),
+              op: getDefaultAggregate(mri),
               query: Object.entries(summaryItem.tags ?? {})
                 .map(([tagKey, tagValue]) => tagToQuery(tagKey, tagValue))
                 .join(' '),

static/app/views/metrics/queryBuilder.tsx

@@ -11,9 +11,10 @@ import {Tooltip} from 'sentry/components/tooltip';
 import {IconLightning, IconReleases, IconWarning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {MetricMeta, MetricsOperation, MRI} from 'sentry/types';
+import type {MetricMeta, MRI} from 'sentry/types/metrics';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {
+  getDefaultAggregate,
   isAllowedOp,
   isCustomMetric,
   isSpanMeasurement,
@@ -53,10 +54,6 @@ const isShownByDefault = (metric: MetricMeta) =>
   isVisibleTransactionMetric(metric) ||
   isVisibleSpanMetric(metric);
 
-function getOpsForMRI(mri: MRI, meta: MetricMeta[]) {
-  return meta.find(metric => metric.mri === mri)?.operations.filter(isAllowedOp) ?? [];
-}
-
 function useMriMode() {
   const [mriMode, setMriMode] = useState(false);
   const mriModeKeyPressed = useKeyPress('`', undefined, true);
@@ -153,24 +150,34 @@ export const QueryBuilder = memo(function QueryBuilder({
 
   const handleMRIChange = useCallback(
     ({value}) => {
-      const availableOps = getOpsForMRI(value, meta);
-      const selectedOp = availableOps.includes(
-        (metricsQuery.op ?? '') as MetricsOperation
-      )
-        ? metricsQuery.op
-        : availableOps?.[0];
-
-      const queryChanges = {
-        mri: value,
-        op: selectedOp,
-        groupBy: undefined,
-      };
+      const currentMRI = parseMRI(metricsQuery.mri);
+      const newMRI = parseMRI(value);
+
+      if (!currentMRI || !newMRI) {
+        return;
+      }
+
+      let queryChanges = {};
+
+      // If the type is the same, we can keep the current aggregate
+      if (currentMRI.type === newMRI.type) {
+        queryChanges = {
+          mri: value,
+          groupBy: undefined,
+        };
+      } else {
+        queryChanges = {
+          mri: value,
+          op: getDefaultAggregate(value),
+          groupBy: undefined,
+        };
+      }
 
       trackAnalytics('ddm.widget.metric', {organization});
       incrementQueryMetric('ddm.widget.metric', queryChanges);
       onChange(queryChanges);
     },
-    [incrementQueryMetric, meta, metricsQuery.op, onChange, organization]
+    [incrementQueryMetric, metricsQuery.mri, onChange, organization]
   );
 
   const handleOpChange = useCallback(

static/app/views/metrics/utils/parseMetricWidgetsQueryParam.tsx

@@ -1,11 +1,11 @@
-import type {MRI} from 'sentry/types';
-import {getDefaultMetricOp} from 'sentry/utils/metrics';
+import type {MRI} from 'sentry/types/metrics';
+import {getDefaultAggregate} from 'sentry/utils/metrics';
 import {
   DEFAULT_SORT_STATE,
   emptyMetricsQueryWidget,
   NO_QUERY_ID,
 } from 'sentry/utils/metrics/constants';
-import {defaultAggregateForMRI, isMRI} from 'sentry/utils/metrics/mri';
+import {isMRI} from 'sentry/utils/metrics/mri';
 import {
   type BaseWidgetParams,
   type FocusedMetricsSeries,
@@ -141,7 +141,7 @@ function parseQueryWidget(
 
   return {
     mri,
-    op: parseStringParam(widget, 'op') ?? getDefaultMetricOp(mri),
+    op: parseStringParam(widget, 'op') ?? getDefaultAggregate(mri),
     query: parseStringParam(widget, 'query') ?? '',
     groupBy: parseArrayParam(widget, 'groupBy', entry =>
       typeof entry === 'string' ? entry : undefined
@@ -291,7 +291,7 @@ export function parseMetricWidgetsQueryParam(
     queries.push({
       ...emptyMetricsQueryWidget,
       mri,
-      op: defaultAggregateForMRI(mri),
+      op: getDefaultAggregate(mri),
     });
   }
 

static/app/views/settings/projectMetrics/projectMetricsDetails.tsx

@@ -16,7 +16,7 @@ import {CHART_PALETTE} from 'sentry/constants/chartPalette';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {
-  MetricsOperation,
+  MetricsAggregate,
   MetricType,
   MRI,
   Organization,
@@ -38,7 +38,7 @@ import {TextAlignRight} from 'sentry/views/starfish/components/textAlign';
 
 import {useProjectMetric} from '../../../utils/metrics/useMetricsMeta';
 
-function getSettingsOperationForType(type: MetricType): MetricsOperation {
+function getSettingsOperationForType(type: MetricType): MetricsAggregate {
   switch (type) {
     case 'c':
       return 'sum';