Browse Source

feat(ddm-alerts): Handle custom metric alerts details (#60509)

Fix appearance of metric based alerts in metric details
Add typing for query params in ddm.
Add `Open in DDM` button to metric based alerts.

- closes https://github.com/getsentry/sentry/issues/60248
- closes https://github.com/getsentry/sentry/issues/60382
ArthurKnaus 1 year ago
parent
commit
597a6ed5a1

+ 61 - 7
static/app/utils/metrics/index.tsx

@@ -1,5 +1,6 @@
 import {InjectedRouter} from 'react-router';
 import moment from 'moment';
+import * as qs from 'query-string';
 
 import {
   DateTimeObject,
@@ -17,7 +18,7 @@ import {defined, formatBytesBase2, formatBytesBase10} from 'sentry/utils';
 import {parseFunction} from 'sentry/utils/discover/fields';
 import {formatPercentage, getDuration} from 'sentry/utils/formatters';
 
-import {PageFilters} from '../../types/core';
+import {DateString, PageFilters} from '../../types/core';
 
 export enum MetricDisplayType {
   LINE = 'line',
@@ -34,6 +35,31 @@ export type MetricTag = {
   key: string;
 };
 
+export type SortState = {
+  name: 'name' | 'avg' | 'min' | 'max' | 'sum' | undefined;
+  order: 'asc' | 'desc';
+};
+
+export interface MetricWidgetQueryParams
+  extends Pick<MetricsQuery, 'mri' | 'op' | 'query' | 'groupBy'> {
+  displayType: MetricDisplayType;
+  focusedSeries?: string;
+  position?: number;
+  powerUserMode?: boolean;
+  showSummaryTable?: boolean;
+  sort?: SortState;
+}
+
+export interface DdmQueryParams {
+  widgets: string; // stringified json representation of MetricWidgetQueryParams
+  end?: DateString;
+  environment?: string[];
+  project?: number[];
+  start?: DateString;
+  statsPeriod?: string | null;
+  utc?: boolean | null;
+}
+
 export type MetricsQuery = {
   datetime: PageFilters['datetime'];
   environments: PageFilters['environments'];
@@ -44,6 +70,36 @@ export type MetricsQuery = {
   query?: string;
 };
 
+export function getDdmUrl(
+  orgSlug: string,
+  {
+    widgets,
+    start,
+    end,
+    statsPeriod,
+    project,
+    ...otherParams
+  }: Omit<DdmQueryParams, 'project' | 'widgets'> & {
+    widgets: MetricWidgetQueryParams[];
+    project?: (string | number)[];
+  }
+) {
+  const urlParams: Partial<DdmQueryParams> = {
+    ...otherParams,
+    project: project?.map(id => (typeof id === 'string' ? parseInt(id, 10) : id)),
+    widgets: JSON.stringify(widgets),
+  };
+
+  if (statsPeriod) {
+    urlParams.statsPeriod = statsPeriod;
+  } else {
+    urlParams.start = start;
+    urlParams.end = end;
+  }
+
+  return `/organizations/${orgSlug}/ddm/?${qs.stringify(urlParams)}`;
+}
+
 export function getMetricsApiRequestQuery(
   {field, query, groupBy}: MetricsApiRequestMetric,
   {projects, environments, datetime}: PageFilters,
@@ -258,14 +314,11 @@ export function mriToField(mri: string, op: string): string {
   return `${op}(${mri})`;
 }
 
-export function fieldToMri(field: string) {
+export function fieldToMri(field: string): {mri?: string; op?: string} {
   const parsedFunction = parseFunction(field);
   if (!parsedFunction) {
     // We only allow aggregate functions for custom metric alerts
-    return {
-      mri: undefined,
-      op: undefined,
-    };
+    return {};
   }
   return {
     mri: parsedFunction.arguments[0],
@@ -286,7 +339,7 @@ export function groupByOp(metrics: MetricsMeta[]): Record<string, MetricsMeta[]>
   return groupedByOp;
 }
 // This is a workaround as the alert builder requires a valid aggregate to be set
-export const DEFAULT_METRIC_ALERT_AGGREGATE = 'sum(c:custom/my_metric@none)';
+export const DEFAULT_METRIC_ALERT_AGGREGATE = 'sum(c:custom/iolnqzyenoqugwm@none)';
 
 export const formatMriAggregate = (aggregate: string) => {
   if (aggregate === DEFAULT_METRIC_ALERT_AGGREGATE) {
@@ -296,6 +349,7 @@ export const formatMriAggregate = (aggregate: string) => {
   const {mri, op} = fieldToMri(aggregate);
   const parsed = parseMRI(mri);
 
+  // The field does not contain an MRI -> return the aggregate as is
   if (!parsed) {
     return aggregate;
   }

+ 4 - 1
static/app/utils/metrics/useMetricsMeta.tsx

@@ -63,6 +63,9 @@ export function useMetricsMeta(
 
   return {
     data: combinedMeta,
-    isLoading: sessionsMeta.isLoading || txnsMeta.isLoading || customMeta.isLoading,
+    isLoading:
+      (sessionsMeta.isLoading && sessionsMeta.fetchStatus !== 'idle') ||
+      (txnsMeta.isLoading && txnsMeta.fetchStatus !== 'idle') ||
+      (customMeta.isLoading && customMeta.fetchStatus !== 'idle'),
   };
 }

+ 3 - 2
static/app/views/alerts/rules/metric/details/body.tsx

@@ -32,6 +32,7 @@ import {
 } from 'sentry/views/alerts/utils/migrationUi';
 
 import {isCrashFreeAlert} from '../utils/isCrashFreeAlert';
+import {isCustomMetricAlert} from '../utils/isCustomMetricAlert';
 
 import {
   API_INTERVAL_POINTS_LIMIT,
@@ -89,9 +90,9 @@ export default function MetricDetailsBody({
       return null;
     }
 
-    const {dataset, query} = rule;
+    const {aggregate, dataset, query} = rule;
 
-    if (isCrashFreeAlert(dataset)) {
+    if (isCrashFreeAlert(dataset) || isCustomMetricAlert(aggregate)) {
       return query.trim().split(' ');
     }
 

+ 2 - 1
static/app/views/alerts/rules/metric/details/metricChart.tsx

@@ -45,6 +45,7 @@ import {ReactEchartsRef, Series} from 'sentry/types/echarts';
 import {getUtcDateString} from 'sentry/utils/dates';
 import {getDuration} from 'sentry/utils/formatters';
 import getDynamicText from 'sentry/utils/getDynamicText';
+import {formatMriAggregate} from 'sentry/utils/metrics';
 import {getForceMetricsLayerQueryExtras} from 'sentry/utils/metrics/features';
 import {shouldShowOnDemandMetricAlertUI} from 'sentry/utils/onDemandMetrics/features';
 import {MINUTES_THRESHOLD_TO_DISPLAY_SECONDS} from 'sentry/utils/sessions';
@@ -368,7 +369,7 @@ class MetricChart extends PureComponent<Props, State> {
           </ChartHeader>
           <ChartFilters>
             <StyledCircleIndicator size={8} />
-            <Filters>{rule.aggregate}</Filters>
+            <Filters>{formatMriAggregate(rule.aggregate)}</Filters>
             <Truncate value={queryFilter ?? ''} maxLength={truncateWidth} />
           </ChartFilters>
           {getDynamicText({

+ 5 - 1
static/app/views/alerts/rules/metric/details/metricChartOption.tsx

@@ -12,6 +12,7 @@ import ConfigStore from 'sentry/stores/configStore';
 import {space} from 'sentry/styles/space';
 import type {SessionApiResponse} from 'sentry/types';
 import type {Series} from 'sentry/types/echarts';
+import {formatMriAggregate} from 'sentry/utils/metrics';
 import {getCrashFreeRateSeries} from 'sentry/utils/sessions';
 import {lightTheme as theme} from 'sentry/utils/theme';
 import {
@@ -170,7 +171,10 @@ export function getMetricAlertChartOption({
     ({label}) => label === AlertRuleTriggerType.WARNING
   );
 
-  const series: AreaChartSeries[] = [...timeseriesData];
+  const series: AreaChartSeries[] = timeseriesData.map(s => ({
+    ...s,
+    seriesName: s.seriesName && formatMriAggregate(s.seriesName),
+  }));
   const areaSeries: AreaChartSeries[] = [];
   // Ensure series data appears below incident/mark lines
   series[0].z = 1;

+ 29 - 0
static/app/views/alerts/rules/metric/metricRulePresets.tsx

@@ -2,8 +2,10 @@ import type {LinkProps} from 'sentry/components/links/link';
 import {t} from 'sentry/locale';
 import type {Project} from 'sentry/types';
 import {DisplayModes} from 'sentry/utils/discover/types';
+import {fieldToMri, getDdmUrl, MetricDisplayType} from 'sentry/utils/metrics';
 import type {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
 import type {MetricRule} from 'sentry/views/alerts/rules/metric/types';
+import {isCustomMetricAggregate} from 'sentry/views/alerts/rules/metric/utils/isCustomMetricAggregate';
 import {getMetricRuleDiscoverUrl} from 'sentry/views/alerts/utils/getMetricRuleDiscoverUrl';
 
 interface PresetCta {
@@ -42,6 +44,33 @@ export function makeDefaultCta({
     };
   }
 
+  if (isCustomMetricAggregate(rule.aggregate)) {
+    const {mri, op} = fieldToMri(rule.aggregate);
+    return {
+      buttonText: t('Open in DDM'),
+      to: getDdmUrl(orgSlug, {
+        start: timePeriod.start,
+        end: timePeriod.end,
+        utc: timePeriod.utc,
+        // 7 days are 9999m in alerts as of a rounding error in the `events-stats` endpoint
+        // We need to round to 7d here to display it correctly in DDM
+        statsPeriod: timePeriod.period === '9999m' ? '7d' : timePeriod.period,
+        project: projects
+          .filter(({slug}) => rule.projects.includes(slug))
+          .map(project => project.id),
+        environment: rule.environment ? [rule.environment] : [],
+        widgets: [
+          {
+            mri: mri as string,
+            op: op as string,
+            query: rule.query,
+            displayType: MetricDisplayType.AREA,
+          },
+        ],
+      }),
+    };
+  }
+
   const extraQueryParams = {
     display: DisplayModes.TOP5,
   };

+ 9 - 5
static/app/views/alerts/rules/metric/mriField.tsx

@@ -25,7 +25,7 @@ function filterAndSortOperations(operations: string[]) {
 }
 
 function MriField({aggregate, project, onChange}: Props) {
-  const {data: meta} = useMetricsMeta([parseInt(project.id, 10)], {
+  const {data: meta, isLoading} = useMetricsMeta([parseInt(project.id, 10)], {
     useCases: ['custom'],
   });
   const metaArr = useMemo(() => {
@@ -44,7 +44,7 @@ function MriField({aggregate, project, onChange}: Props) {
         {}
       );
     }
-  }, [metaArr, onChange, selectedMriMeta]);
+  }, [metaArr, onChange, selectedMriMeta, isLoading]);
 
   const handleMriChange = useCallback(
     option => {
@@ -108,7 +108,7 @@ function MriField({aggregate, project, onChange}: Props) {
   );
 
   // When using the async variant of SelectControl, we need to pass in an option object instead of just the value
-  const selectedOption = selectedMriMeta && {
+  const selectedMriOption = selectedMriMeta && {
     label: selectedMriMeta.name,
     value: selectedMriMeta.mri,
   };
@@ -117,17 +117,21 @@ function MriField({aggregate, project, onChange}: Props) {
     <Wrapper>
       <StyledSelectControl
         searchable
+        isDisabled={isLoading}
         placeholder={t('Select a metric')}
+        noOptionsMessage={() =>
+          metaArr.length === 0 ? t('No metrics in this project') : t('No options')
+        }
         async
         defaultOptions={getMriOptions('')}
         loadOptions={searchText => Promise.resolve(getMriOptions(searchText))}
         filterOption={() => true}
-        value={selectedOption}
+        value={selectedMriOption}
         onChange={handleMriChange}
       />
       <StyledSelectControl
         searchable
-        disabled={!selectedValues.mri}
+        isDisabled={isLoading || !selectedMriMeta}
         placeholder={t('Select an operation')}
         options={operationOptions}
         value={selectedValues.op}

+ 7 - 3
static/app/views/alerts/rules/metric/ruleForm.tsx

@@ -150,10 +150,14 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
   }
 
   get chartQuery(): string {
-    const {query, eventTypes, dataset} = this.state;
+    const {alertType, query, eventTypes, dataset} = this.state;
     const eventTypeFilter = getEventTypeFilter(this.state.dataset, eventTypes);
     const queryWithTypeFilter = (
-      query ? `(${query}) AND (${eventTypeFilter})` : eventTypeFilter
+      alertType !== 'custom_metrics'
+        ? query
+          ? `(${query}) AND (${eventTypeFilter})`
+          : eventTypeFilter
+        : query
     ).trim();
     return isCrashFreeAlert(dataset) ? query : queryWithTypeFilter;
   }
@@ -911,7 +915,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
               <AlertInfo>
                 <StyledCircleIndicator size={8} />
                 <Aggregate>{formatMriAggregate(aggregate)}</Aggregate>
-                {!(hasDdmAlertsSupport(organization) && alertType === 'custom_metrics')
+                {alertType !== 'custom_metrics'
                   ? `event.type:${eventTypes?.join(',')}`
                   : ''}
               </AlertInfo>

+ 2 - 1
static/app/views/alerts/rules/metric/triggers/chart/index.tsx

@@ -34,6 +34,7 @@ import type {
 } from 'sentry/types';
 import type {Series} from 'sentry/types/echarts';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
+import {formatMriAggregate} from 'sentry/utils/metrics';
 import {getForceMetricsLayerQueryExtras} from 'sentry/utils/metrics/features';
 import {shouldShowOnDemandMetricAlertUI} from 'sentry/utils/onDemandMetrics/features';
 import {
@@ -505,7 +506,7 @@ class TriggersChart extends PureComponent<Props, State> {
         period={period}
         yAxis={aggregate}
         includePrevious={false}
-        currentSeriesNames={[aggregate]}
+        currentSeriesNames={[formatMriAggregate(aggregate)]}
         partial={false}
         queryExtras={queryExtras}
         dataLoadedCallback={onDataLoaded}

+ 8 - 0
static/app/views/alerts/rules/metric/utils/isCustomMetricAlert.tsx

@@ -0,0 +1,8 @@
+import {fieldToMri} from 'sentry/utils/metrics';
+
+/**
+ * Currently we can tell if an alert is a crash free alert by checking the aggregate for a MRI,
+ */
+export function isCustomMetricAlert(aggregate: string): boolean {
+  return fieldToMri(aggregate).mri !== undefined;
+}

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