Browse Source

feat(insights): Adds insights alerts for http module (#77267)

Adds create alert buttons to http module charts.
Includes changes to `insightsMetricField` to support functions with non
MRI args.
Also organizes some constants better.
edwardgou-sentry 6 months ago
parent
commit
3c2f9e7c25

+ 3 - 0
static/app/components/metrics/metricSearchBar.tsx

@@ -46,6 +46,9 @@ const INSIGHTS_ADDITIONAL_TAG_FILTERS: MetricTag[] = [
   {
     key: SpanMetricsField.SPAN_MODULE,
   },
+  {
+    key: SpanMetricsField.FILE_EXTENSION,
+  },
 ];
 
 export function MetricSearchBar({

+ 15 - 4
static/app/utils/metrics/mri.tsx

@@ -7,6 +7,8 @@ import type {
   UseCase,
 } from 'sentry/types/metrics';
 import {parseFunction} from 'sentry/utils/discover/fields';
+import {SPAN_DURATION_MRI} from 'sentry/utils/metrics/constants';
+import {INSIGHTS_METRICS_OPERATIONS} from 'sentry/views/alerts/rules/metric/utils/isInsightsMetricAlert';
 
 export const DEFAULT_MRI: MRI = 'c:custom/sentry_metric@none';
 export const DEFAULT_SPAN_MRI: MRI = 'c:custom/span_attribute_0@none';
@@ -116,11 +118,20 @@ export function isMRIField(field: string): boolean {
 
 // convenience function to get the MRI from a field, returns defaut MRI if it fails
 export function getMRI(field: string): MRI {
-  // spm() doesn't take an argument and it always operates on the spans exclusive time mri
-  if (['spm()', 'cache_miss_rate()'].includes(field)) {
-    return 'd:spans/exclusive_time@millisecond';
-  }
   const parsed = parseField(field);
+  // Insights functions don't always take an MRI as an argument.
+  // In these cases, we need to default to a specific MRI.
+  if (parsed?.aggregation) {
+    const operation = INSIGHTS_METRICS_OPERATIONS.find(({value}) => {
+      return value === parsed?.aggregation;
+    });
+    if (operation) {
+      if (operation.mri) {
+        return operation.mri as MRI;
+      }
+      return SPAN_DURATION_MRI;
+    }
+  }
   return parsed?.mri ?? DEFAULT_MRI;
 }
 

+ 17 - 0
static/app/views/alerts/rules/metric/insightsMetricField.spec.tsx

@@ -89,4 +89,21 @@ describe('InsightsMetricField', () => {
     userEvent.click(await screen.findByText('spm'));
     await waitFor(() => expect(onChange).toHaveBeenCalledWith('spm()', {}));
   });
+
+  it('should call onChange using the http_response_rate function defaulting with argument 3 when switching to http_response_rate', async () => {
+    const {project} = initializeOrg();
+    const onChange = jest.fn();
+    render(
+      <InsightsMetricField
+        aggregate={'avg(d:spans/exclusive_time@millisecond)'}
+        onChange={onChange}
+        project={project}
+      />
+    );
+    userEvent.click(screen.getByText('avg'));
+    userEvent.click(await screen.findByText('http_response_rate'));
+    await waitFor(() =>
+      expect(onChange).toHaveBeenCalledWith('http_response_rate(3)', {})
+    );
+  });
 });

+ 88 - 43
static/app/views/alerts/rules/metric/insightsMetricField.tsx

@@ -5,16 +5,17 @@ import Tag from 'sentry/components/badge/tag';
 import SelectControl from 'sentry/components/forms/controls/selectControl';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {MetricMeta, ParsedMRI} from 'sentry/types/metrics';
+import type {MetricAggregation, MetricMeta, ParsedMRI} from 'sentry/types/metrics';
 import type {Project} from 'sentry/types/project';
+import {parseFunction} from 'sentry/utils/discover/fields';
 import {getDefaultAggregation} from 'sentry/utils/metrics';
 import {getReadableMetricType} from 'sentry/utils/metrics/formatters';
 import {
   DEFAULT_INSIGHTS_METRICS_ALERT_FIELD,
   DEFAULT_INSIGHTS_MRI,
   formatMRI,
+  isMRI,
   MRIToField,
-  parseField,
   parseMRI,
 } from 'sentry/utils/metrics/mri';
 import {useVirtualizedMetricsMeta} from 'sentry/utils/metrics/useMetricsMeta';
@@ -22,6 +23,8 @@ import {middleEllipsis} from 'sentry/utils/string/middleEllipsis';
 import {
   INSIGHTS_METRICS,
   INSIGHTS_METRICS_OPERATIONS,
+  INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS,
+  INSIGHTS_METRICS_OPERATIONS_WITHOUT_ARGS,
 } from 'sentry/views/alerts/rules/metric/utils/isInsightsMetricAlert';
 
 interface Props {
@@ -49,11 +52,19 @@ const OPERATIONS = [
     label: 'max',
     value: 'max',
   },
-  ...INSIGHTS_METRICS_OPERATIONS,
+  ...INSIGHTS_METRICS_OPERATIONS.map(({label, value}) => ({label, value})),
 ];
 
 function aggregateRequiresArgs(aggregation?: string) {
-  return !['spm', 'cache_miss_rate'].includes(aggregation ?? '');
+  return !INSIGHTS_METRICS_OPERATIONS_WITHOUT_ARGS.some(
+    ({value}) => value === aggregation
+  );
+}
+
+function aggregateHasCustomArgs(aggregation?: string) {
+  return INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS.some(
+    ({value}) => value === aggregation
+  );
 }
 
 function InsightsMetricField({aggregate, project, onChange}: Props) {
@@ -74,17 +85,31 @@ function InsightsMetricField({aggregate, project, onChange}: Props) {
       .filter(metric => INSIGHTS_METRICS.includes(metric.mri));
   }, [meta]);
 
-  const selectedValues = parseField(aggregate);
+  // We parse out the aggregation and field from the aggregate string.
+  // This only works for aggregates with <= 1 argument.
+  const {
+    name: aggregation,
+    arguments: [field],
+  } = parseFunction(aggregate) ?? {arguments: [undefined]};
 
   const selectedMriMeta = useMemo(() => {
-    return meta.find(metric => metric.mri === selectedValues?.mri);
-  }, [meta, selectedValues?.mri]);
+    return meta.find(metric => metric.mri === field);
+  }, [meta, field]);
 
   useEffect(() => {
-    if (!aggregateRequiresArgs(selectedValues?.aggregation)) {
+    if (!aggregateRequiresArgs(aggregation)) {
+      return;
+    }
+    if (aggregation && aggregateHasCustomArgs(aggregation)) {
+      const options = INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS.find(
+        ({value}) => value === aggregation
+      )?.options;
+      if (options && !options.some(({value}) => value === field)) {
+        onChange(`${aggregation}(${options?.[0].value})`, {});
+      }
       return;
     }
-    if (selectedValues?.mri && !selectedMriMeta && !isLoading) {
+    if (field && !selectedMriMeta && !isLoading) {
       const newSelection = metaArr[0];
       if (newSelection) {
         onChange(MRIToField(newSelection.mri, 'avg'), {});
@@ -92,15 +117,7 @@ function InsightsMetricField({aggregate, project, onChange}: Props) {
         onChange(DEFAULT_INSIGHTS_METRICS_ALERT_FIELD, {});
       }
     }
-  }, [
-    metaArr,
-    onChange,
-    isLoading,
-    aggregate,
-    selectedValues?.mri,
-    selectedMriMeta,
-    selectedValues?.aggregation,
-  ]);
+  }, [metaArr, onChange, isLoading, aggregate, selectedMriMeta, aggregation, field]);
 
   const handleMriChange = useCallback(
     option => {
@@ -110,13 +127,23 @@ function InsightsMetricField({aggregate, project, onChange}: Props) {
       }
       const newType = parseMRI(option.value)?.type;
       // If the type is the same, we can keep the current aggregate
-      if (newType === selectedMeta.type && selectedValues?.aggregation) {
-        onChange(MRIToField(option.value, selectedValues?.aggregation), {});
+      if (newType === selectedMeta.type && aggregation) {
+        onChange(MRIToField(option.value, aggregation as MetricAggregation), {});
       } else {
         onChange(MRIToField(option.value, getDefaultAggregation(option.value)), {});
       }
     },
-    [meta, onChange, selectedValues?.aggregation]
+    [meta, onChange, aggregation]
+  );
+
+  const handleOptionChange = useCallback(
+    option => {
+      if (!option || !aggregation) {
+        return;
+      }
+      onChange(`${aggregation}(${option.value})`, {});
+    },
+    [onChange, aggregation]
   );
 
   // As SelectControl does not support an options size limit out of the box
@@ -159,9 +186,9 @@ function InsightsMetricField({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 selectedMriOption = selectedValues?.mri && {
-    label: formatMRI(selectedValues.mri),
-    value: selectedValues.mri,
+  const selectedOption = field && {
+    label: isMRI(field) ? formatMRI(field) : field,
+    value: field,
   };
 
   return (
@@ -171,33 +198,51 @@ function InsightsMetricField({aggregate, project, onChange}: Props) {
         isDisabled={isLoading}
         placeholder={t('Select an operation')}
         options={OPERATIONS}
-        value={selectedValues?.aggregation}
+        value={aggregation}
         onChange={option => {
           if (!aggregateRequiresArgs(option.value)) {
             onChange(`${option.value}()`, {});
-          } else if (selectedValues?.mri) {
-            onChange(MRIToField(selectedValues.mri, option.value), {});
+          } else if (aggregateHasCustomArgs(option.value)) {
+            const options = INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS.find(
+              ({value}) => value === option.value
+            )?.options;
+            onChange(`${option.value}(${options?.[0].value})`, {});
+          } else if (field && isMRI(field)) {
+            onChange(MRIToField(field, option.value), {});
           } else {
             onChange(MRIToField(DEFAULT_INSIGHTS_MRI, option.value), {});
           }
         }}
       />
-      {aggregateRequiresArgs(selectedValues?.aggregation) && (
-        <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={selectedMriOption}
-          onChange={handleMriChange}
-        />
-      )}
+      {aggregateRequiresArgs(aggregation) &&
+        (aggregateHasCustomArgs(aggregation) ? (
+          <StyledSelectControl
+            searchable
+            placeholder={t('Select an option')}
+            options={
+              INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS.find(
+                ({value}) => value === aggregation
+              )?.options
+            }
+            value={selectedOption}
+            onChange={handleOptionChange}
+          />
+        ) : (
+          <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}
+            onChange={handleMriChange}
+          />
+        ))}
     </Wrapper>
   );
 }

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

@@ -57,6 +57,7 @@ import TriggersChart from 'sentry/views/alerts/rules/metric/triggers/chart';
 import {getEventTypeFilter} from 'sentry/views/alerts/rules/metric/utils/getEventTypeFilter';
 import hasThresholdValue from 'sentry/views/alerts/rules/metric/utils/hasThresholdValue';
 import {isCustomMetricAlert} from 'sentry/views/alerts/rules/metric/utils/isCustomMetricAlert';
+import {isInsightsMetricAlert} from 'sentry/views/alerts/rules/metric/utils/isInsightsMetricAlert';
 import {isOnDemandMetricAlert} from 'sentry/views/alerts/rules/metric/utils/onDemandMetricAlert';
 import {AlertRuleType} from 'sentry/views/alerts/types';
 import {ruleNeedsErrorMigration} from 'sentry/views/alerts/utils/migrationUi';
@@ -1197,7 +1198,8 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
     return (
       <Main fullWidth>
         <PermissionAlert access={['alerts:write']} project={project} />
-        {isCustomMetricAlert(rule.aggregate) && <MetricsBetaEndAlert />}
+        {isCustomMetricAlert(rule.aggregate) &&
+          !isInsightsMetricAlert(rule.aggregate) && <MetricsBetaEndAlert />}
 
         {eventView && <IncompatibleAlertQuery eventView={eventView} />}
         <Form

+ 34 - 1
static/app/views/alerts/rules/metric/utils/isInsightsMetricAlert.tsx

@@ -1,14 +1,47 @@
 import {parseField} from 'sentry/utils/metrics/mri';
 
-export const INSIGHTS_METRICS_OPERATIONS = [
+export const INSIGHTS_METRICS_OPERATIONS_WITHOUT_ARGS = [
   {
     label: 'spm',
     value: 'spm',
+    mri: 'd:spans/duration@millisecond',
   },
   {
     label: 'cache_miss_rate',
     value: 'cache_miss_rate',
+    mri: 'd:spans/duration@millisecond',
+  },
+];
+
+export const INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS = [
+  {
+    label: 'http_response_rate',
+    value: 'http_response_rate',
+    options: [
+      {label: '3', value: '3'},
+      {label: '4', value: '4'},
+      {label: '5', value: '5'},
+    ],
+    mri: 'd:spans/duration@millisecond',
   },
+  {
+    label: 'performance_score',
+    value: 'performance_score',
+    options: [
+      {label: 'measurements.score.lcp', value: 'measurements.score.lcp'},
+      {label: 'measurements.score.fcp', value: 'measurements.score.fcp'},
+      {label: 'measurements.score.inp', value: 'measurements.score.inp'},
+      {label: 'measurements.score.cls', value: 'measurements.score.cls'},
+      {label: 'measurements.score.ttfb', value: 'measurements.score.ttfb'},
+      {label: 'measurements.score.total', value: 'measurements.score.total'},
+    ],
+    mri: 'd:transactions/measurements.score.total@ratio',
+  },
+];
+
+export const INSIGHTS_METRICS_OPERATIONS = [
+  ...INSIGHTS_METRICS_OPERATIONS_WITH_CUSTOM_ARGS,
+  ...INSIGHTS_METRICS_OPERATIONS_WITHOUT_ARGS,
 ];
 
 export const INSIGHTS_METRICS = [

+ 29 - 0
static/app/views/insights/http/alerts.ts

@@ -0,0 +1,29 @@
+import type {AlertConfig} from 'sentry/views/insights/common/components/chartPanel';
+
+const QUERY = 'span.module:http span.op:http.client';
+
+export const ALERTS: Record<string, AlertConfig> = {
+  spm: {
+    aggregate: 'spm()',
+    query: QUERY,
+  },
+  duration: {
+    aggregate: 'avg(d:spans/duration@millisecond)',
+    query: QUERY,
+  },
+  threeHundreds: {
+    aggregate: 'http_response_rate(3)',
+    query: QUERY,
+    name: 'Create 3XX Response Rate Alert',
+  },
+  fourHundreds: {
+    aggregate: 'http_response_rate(4)',
+    query: QUERY,
+    name: 'Create 4XX Response Rate Alert',
+  },
+  fiveHundreds: {
+    aggregate: 'http_response_rate(5)',
+    query: QUERY,
+    name: 'Create 5XX Response Rate Alert',
+  },
+};

+ 12 - 1
static/app/views/insights/http/components/charts/durationChart.tsx

@@ -1,16 +1,20 @@
 import type {ComponentProps} from 'react';
 
 import type {EChartHighlightHandler, Series} from 'sentry/types/echarts';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {AVG_COLOR} from 'sentry/views/insights/colors';
 import Chart, {ChartType} from 'sentry/views/insights/common/components/chart';
 import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
 import {getDurationChartTitle} from 'sentry/views/insights/common/views/spans/types';
+import {ALERTS} from 'sentry/views/insights/http/alerts';
 import {CHART_HEIGHT} from 'sentry/views/insights/http/settings';
+import type {SpanMetricsQueryFilters} from 'sentry/views/insights/types';
 
 interface Props {
   isLoading: boolean;
   series: Series[];
   error?: Error | null;
+  filters?: SpanMetricsQueryFilters;
   onHighlight?: (highlights: Highlight[], event: Event) => void; // TODO: Correctly type this
   scatterPlot?: ComponentProps<typeof Chart>['scatterPlot'];
 }
@@ -26,6 +30,7 @@ export function DurationChart({
   isLoading,
   error,
   onHighlight,
+  filters,
 }: Props) {
   // TODO: This is duplicated from `DurationChart` in `SampleList`. Resolve the duplication
   const handleChartHighlight: EChartHighlightHandler = function (event) {
@@ -50,8 +55,14 @@ export function DurationChart({
     onHighlight?.(highlightedDataPoints, event);
   };
 
+  const filterString = filters && MutableSearch.fromQueryObject(filters).formatString();
+  const alertConfig = {
+    ...ALERTS.duration,
+    query: filterString ?? ALERTS.duration.query,
+  };
+
   return (
-    <ChartPanel title={getDurationChartTitle('http')}>
+    <ChartPanel title={getDurationChartTitle('http')} alertConfigs={[alertConfig]}>
       <Chart
         height={CHART_HEIGHT}
         grid={{

+ 12 - 2
static/app/views/insights/http/components/charts/responseRateChart.tsx

@@ -1,5 +1,6 @@
 import type {Series} from 'sentry/types/echarts';
 import {formatPercentage} from 'sentry/utils/number/formatPercentage';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {
   HTTP_RESPONSE_3XX_COLOR,
   HTTP_RESPONSE_4XX_COLOR,
@@ -8,17 +9,26 @@ import {
 import Chart, {ChartType} from 'sentry/views/insights/common/components/chart';
 import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
 import {DataTitles} from 'sentry/views/insights/common/views/spans/types';
+import {ALERTS} from 'sentry/views/insights/http/alerts';
 import {CHART_HEIGHT} from 'sentry/views/insights/http/settings';
+import type {SpanMetricsQueryFilters} from 'sentry/views/insights/types';
 
 interface Props {
   isLoading: boolean;
   series: [Series, Series, Series];
   error?: Error | null;
+  filters?: SpanMetricsQueryFilters;
 }
 
-export function ResponseRateChart({series, isLoading, error}: Props) {
+export function ResponseRateChart({series, isLoading, error, filters}: Props) {
+  const filterString = filters && MutableSearch.fromQueryObject(filters).formatString();
+  const alertConfig = [
+    {...ALERTS.threeHundreds, query: filterString ?? ALERTS.threeHundreds.query},
+    {...ALERTS.fourHundreds, query: filterString ?? ALERTS.fourHundreds.query},
+    {...ALERTS.fiveHundreds, query: filterString ?? ALERTS.fiveHundreds.query},
+  ];
   return (
-    <ChartPanel title={DataTitles.unsuccessfulHTTPCodes}>
+    <ChartPanel title={DataTitles.unsuccessfulHTTPCodes} alertConfigs={alertConfig}>
       <Chart
         showLegend
         height={CHART_HEIGHT}

+ 8 - 2
static/app/views/insights/http/components/charts/throughputChart.tsx

@@ -1,21 +1,27 @@
 import type {Series} from 'sentry/types/echarts';
 import {RateUnit} from 'sentry/utils/discover/fields';
 import {formatRate} from 'sentry/utils/formatters';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {THROUGHPUT_COLOR} from 'sentry/views/insights/colors';
 import Chart, {ChartType} from 'sentry/views/insights/common/components/chart';
 import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
 import {getThroughputChartTitle} from 'sentry/views/insights/common/views/spans/types';
+import {ALERTS} from 'sentry/views/insights/http/alerts';
 import {CHART_HEIGHT} from 'sentry/views/insights/http/settings';
+import type {SpanMetricsQueryFilters} from 'sentry/views/insights/types';
 
 interface Props {
   isLoading: boolean;
   series: Series;
   error?: Error | null;
+  filters?: SpanMetricsQueryFilters;
 }
 
-export function ThroughputChart({series, isLoading, error}: Props) {
+export function ThroughputChart({series, isLoading, error, filters}: Props) {
+  const filterString = filters && MutableSearch.fromQueryObject(filters).formatString();
+  const alertConfig = {...ALERTS.spm, query: filterString ?? ALERTS.spm.query};
   return (
-    <ChartPanel title={getThroughputChartTitle('http')}>
+    <ChartPanel title={getThroughputChartTitle('http')} alertConfigs={[alertConfig]}>
       <Chart
         height={CHART_HEIGHT}
         grid={{

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