Browse Source

feat(alerts): fade out custom percentiles (#61926)

Ogi 1 year ago
parent
commit
f2f4fcf7f8

+ 0 - 1
static/app/views/alerts/rules/metric/constants.tsx

@@ -63,7 +63,6 @@ export const errorFieldConfig: OptionConfig = {
 
 const commonAggregations = [
   AggregationKey.AVG,
-  AggregationKey.PERCENTILE,
   AggregationKey.P50,
   AggregationKey.P75,
   AggregationKey.P90,

+ 1 - 2
static/app/views/alerts/rules/metric/metricField.spec.tsx

@@ -55,10 +55,9 @@ describe('MetricField', function () {
     );
     await openSelectMenu('(Required)');
 
-    // 10 error aggregate configs
+    // 9 error aggregate configs
     [
       'avg(…)',
-      'percentile(…)',
       'p50(…)',
       'p75(…)',
       'p95(…)',

+ 0 - 22
static/app/views/alerts/rules/metric/ruleForm.spec.tsx

@@ -414,28 +414,6 @@ describe('Incident Rules Form', () => {
       expect(onSubmitSuccess).toHaveBeenCalled();
     });
 
-    it('shows errors for an invalid on demand metric rule', async () => {
-      const invalidOnDemandMetricRule = TestStubs.MetricRule({
-        aggregate: 'percentile()',
-        query: 'transaction.duration:<1s',
-        dataset: 'generic_metrics',
-      });
-
-      const onSubmitSuccess = jest.fn();
-
-      createWrapper({
-        ruleId: invalidOnDemandMetricRule.id,
-        rule: {
-          ...invalidOnDemandMetricRule,
-          eventTypes: ['transaction'],
-        },
-        onSubmitSuccess,
-      });
-
-      await userEvent.click(screen.getByLabelText('Save Rule'), {delay: null});
-      expect(onSubmitSuccess).not.toHaveBeenCalled();
-    });
-
     it('hides fields when migrating error metric alerts to filter archived issues', async () => {
       const errorAlert = MetricRule({
         dataset: Dataset.ERRORS,

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

@@ -28,6 +28,7 @@ import {EventsStats, MultiSeriesEventsStats, Organization, Project} from 'sentry
 import {defined} from 'sentry/utils';
 import {metric, trackAnalytics} from 'sentry/utils/analytics';
 import type EventView from 'sentry/utils/discover/eventView';
+import {AggregationKey} from 'sentry/utils/fields';
 import {
   getForceMetricsLayerQueryExtras,
   hasDDMFeature,
@@ -47,10 +48,7 @@ import Triggers from 'sentry/views/alerts/rules/metric/triggers';
 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 {
-  isOnDemandMetricAlert,
-  isValidOnDemandMetricAlert,
-} from 'sentry/views/alerts/rules/metric/utils/onDemandMetricAlert';
+import {isOnDemandMetricAlert} from 'sentry/views/alerts/rules/metric/utils/onDemandMetricAlert';
 import {AlertRuleType} from 'sentry/views/alerts/types';
 import {
   hasIgnoreArchivedFeatureFlag,
@@ -553,11 +551,13 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
   };
 
   validateOnDemandMetricAlert() {
-    return isValidOnDemandMetricAlert(
-      this.state.dataset,
-      this.state.aggregate,
-      this.state.query
-    );
+    if (
+      !isOnDemandMetricAlert(this.state.dataset, this.state.aggregate, this.state.query)
+    ) {
+      return true;
+    }
+
+    return !this.state.aggregate.includes(AggregationKey.PERCENTILE);
   }
 
   handleSubmit = async (

+ 0 - 14
static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx

@@ -1,21 +1,7 @@
-import {AggregationKey} from 'sentry/utils/fields';
 import {isOnDemandAggregate, isOnDemandQueryString} from 'sentry/utils/onDemandMetrics';
 import {Dataset} from 'sentry/views/alerts/rules/metric/types';
 import {isCustomMetricField} from 'sentry/views/alerts/rules/metric/utils/isCustomMetricField';
 
-export function isValidOnDemandMetricAlert(
-  dataset: Dataset,
-  aggregate: string,
-  query: string
-): boolean {
-  if (!isOnDemandMetricAlert(dataset, aggregate, query)) {
-    return true;
-  }
-
-  // On demand metric alerts do not support generic percentile aggregations
-  return !aggregate.includes(AggregationKey.PERCENTILE);
-}
-
 /**
  * We determine that an alert is an on-demand metric alert if the query contains
  * one of the tags that are not supported by the standard metrics.

+ 56 - 2
static/app/views/alerts/rules/metric/wizardField.tsx

@@ -6,7 +6,11 @@ import FormField, {FormFieldProps} from 'sentry/components/forms/formField';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {Organization, Project} from 'sentry/types';
-import {explodeFieldString, generateFieldAsString} from 'sentry/utils/discover/fields';
+import {
+  explodeFieldString,
+  generateFieldAsString,
+  QueryFieldValue,
+} from 'sentry/utils/discover/fields';
 import {hasDDMFeature} from 'sentry/utils/metrics/features';
 import MriField from 'sentry/views/alerts/rules/metric/mriField';
 import {Dataset} from 'sentry/views/alerts/rules/metric/types';
@@ -144,7 +148,7 @@ export default function WizardField({
             alertType,
           });
         const fieldOptions = generateFieldOptions({organization, ...fieldOptionsConfig});
-        const fieldValue = explodeFieldString(aggregate ?? '');
+        const fieldValue = getFieldValue(aggregate ?? '', model);
 
         const fieldKey =
           fieldValue?.kind === FieldValueKind.FUNCTION
@@ -209,6 +213,56 @@ export default function WizardField({
   );
 }
 
+// swaps out custom percentile values for known percentiles, used while we fade out custom percentiles in metric alerts
+// TODO(telemetry-experience): remove once we migrate all custom percentile alerts
+const getFieldValue = (aggregate: string | undefined, model) => {
+  const fieldValue = explodeFieldString(aggregate ?? '');
+
+  if (fieldValue?.kind !== FieldValueKind.FUNCTION) {
+    return fieldValue;
+  }
+
+  if (fieldValue.function[0] !== 'percentile') {
+    return fieldValue;
+  }
+
+  const newFieldValue: QueryFieldValue = {
+    kind: FieldValueKind.FUNCTION,
+    function: [
+      getApproximateKnownPercentile(fieldValue.function[2] as string),
+      fieldValue.function[1],
+      undefined,
+      undefined,
+    ],
+    alias: fieldValue.alias,
+  };
+
+  model.setValue('aggregate', generateFieldAsString(newFieldValue));
+
+  return newFieldValue;
+};
+
+const getApproximateKnownPercentile = (customPercentile: string) => {
+  const percentile = parseFloat(customPercentile);
+
+  if (percentile <= 0.5) {
+    return 'p50';
+  }
+  if (percentile <= 0.75) {
+    return 'p75';
+  }
+  if (percentile <= 0.9) {
+    return 'p90';
+  }
+  if (percentile <= 0.95) {
+    return 'p95';
+  }
+  if (percentile <= 0.99) {
+    return 'p99';
+  }
+  return 'p100';
+};
+
 const Container = styled('div')<{hideGap: boolean}>`
   display: grid;
   grid-template-columns: 1fr auto;