Repository: getsentry/sentry
Pull request: feat(alerts): on demand support for apdex
Adds support for apdex alerts with arbitrary thresholds

Requires: https://github.com/getsentry/sentry/pull/55999
Closes: https://github.com/getsentry/sentry/issues/55767

Also fixes an issue where an alert info box was not shown after dismissal

Problem statement:
Support Apdex thresholds
**Problem**: If a user configures an alert based on Apdex where a threshold is different than the one configured in the Project Settings, this should be an on-demand metric.
```[tasklist]
### Tasks
- [ ] FE
- [ ] Extraction
- [ ] Querying
```

Hints from issue discussion:

Commit message: Merge branch 'master' into ogi/ref/on-demand-apdex-fe

Patch to review:
diff --git a/static/app/components/alerts/onDemandMetricAlert.tsx b/static/app/components/alerts/onDemandMetricAlert.tsx
index cef4cd5e7ef56b..3a401c385f66d0 100644
--- a/static/app/components/alerts/onDemandMetricAlert.tsx
+++ b/static/app/components/alerts/onDemandMetricAlert.tsx
@@ -47,7 +47,7 @@ export function OnDemandMetricAlert({
 }) {
   const {dismiss, isDismissed} = useDismissAlert({key: LOCAL_STORAGE_KEY});
 
-  if (isDismissed) {
+  if (dismissable && isDismissed) {
     return null;
   }
 
diff --git a/static/app/utils/onDemandMetrics/index.tsx b/static/app/utils/onDemandMetrics/index.tsx
index 2d1e6292152151..87bb8b37bc723e 100644
--- a/static/app/utils/onDemandMetrics/index.tsx
+++ b/static/app/utils/onDemandMetrics/index.tsx
@@ -7,7 +7,7 @@ import {
   TokenResult,
 } from 'sentry/components/searchSyntax/parser';
 import {Organization} from 'sentry/types';
-import {FieldKey, getFieldDefinition} from 'sentry/utils/fields';
+import {AggregationKey, FieldKey, getFieldDefinition} from 'sentry/utils/fields';
 import {
   ON_DEMAND_METRICS_UNSUPPORTED_TAGS,
   STANDARD_SEARCH_FIELD_KEYS,
@@ -38,6 +38,10 @@ export function createOnDemandFilterWarning(warning: React.ReactNode) {
   };
 }
 
+export function isOnDemandAggregate(aggregate: string): boolean {
+  return aggregate.includes(AggregationKey.APDEX);
+}
+
 export function isOnDemandQueryString(query: string): boolean {
   const searchFilterKeys = getSearchFilterKeys(query);
   const isStandardSearch = searchFilterKeys.every(isStandardSearchFilterKey);
diff --git a/static/app/views/alerts/rules/metric/details/body.tsx b/static/app/views/alerts/rules/metric/details/body.tsx
index 6cedff542033dd..c10dc4da0b52fe 100644
--- a/static/app/views/alerts/rules/metric/details/body.tsx
+++ b/static/app/views/alerts/rules/metric/details/body.tsx
@@ -128,7 +128,7 @@ export default function MetricDetailsBody({
     );
   }
 
-  const {query, dataset} = rule;
+  const {dataset, aggregate, query} = rule;
 
   const queryWithTypeFilter = `${query} ${extractEventTypeFilterFromRule(rule)}`.trim();
   const relativeOptions = {
@@ -139,7 +139,7 @@ export default function MetricDetailsBody({
   const isSnoozed = rule.snooze;
   const ruleActionCategory = getAlertRuleActionCategory(rule);
 
-  const isOnDemandAlert = isOnDemandMetricAlert(dataset, query);
+  const isOnDemandAlert = isOnDemandMetricAlert(dataset, aggregate, query);
 
   return (
     <Fragment>
@@ -227,7 +227,7 @@ export default function MetricDetailsBody({
         <Layout.Side>
           <MetricDetailsSidebar
             rule={rule}
-            isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, query)}
+            isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, aggregate, query)}
           />
         </Layout.Side>
       </Layout.Body>
diff --git a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx
index b485c22b2cfa69..b39321b2f40eab 100644
--- a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx
+++ b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx
@@ -510,7 +510,7 @@ class RuleConditionsForm extends PureComponent<Props, State> {
                     }}
                     hasRecentSearches={dataset !== Dataset.SESSIONS}
                   />
-                  {isExtrapolatedChartData && (
+                  {isExtrapolatedChartData && isOnDemandQueryString(value) && (
                     <OnDemandWarningIcon
                       color="gray500"
                       msg={tct(
diff --git a/static/app/views/alerts/rules/metric/ruleForm.tsx b/static/app/views/alerts/rules/metric/ruleForm.tsx
index 5dabb42b64f99e..443f51a5a1ac54 100644
--- a/static/app/views/alerts/rules/metric/ruleForm.tsx
+++ b/static/app/views/alerts/rules/metric/ruleForm.tsx
@@ -790,7 +790,9 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
     const {isExtrapolatedData} = data ?? {};
 
     this.setState({isExtrapolatedChartData: Boolean(isExtrapolatedData)});
-    if (!isOnDemandMetricAlert(this.state.dataset, this.state.query)) {
+
+    const {dataset, aggregate, query} = this.state;
+    if (!isOnDemandMetricAlert(dataset, aggregate, query)) {
       this.handleMEPAlertDataset(data);
     }
   };
@@ -832,8 +834,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
       location,
     } = this.state;
 
-    const onDemandMetricsAlert = isOnDemandMetricAlert(dataset, query);
-
     const chartProps = {
       organization,
       projects: [project],
@@ -850,7 +850,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
       comparisonDelta,
       comparisonType,
       isQueryValid,
-      isOnDemandMetricAlert: onDemandMetricsAlert,
+      isOnDemandMetricAlert: isOnDemandMetricAlert(dataset, aggregate, query),
       onDataLoaded: this.handleTimeSeriesDataFetched,
     };
 
diff --git a/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.spec.tsx b/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.spec.tsx
index e30210ffac7008..7cb69aadff156d 100644
--- a/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.spec.tsx
+++ b/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.spec.tsx
@@ -5,23 +5,37 @@ describe('isOnDemandMetricAlert', () => {
   it('should return true for an alert that contains non standard fields', () => {
     const dataset = Dataset.GENERIC_METRICS;
 
-    expect(isOnDemandMetricAlert(dataset, 'transaction.duration:>1')).toBeTruthy();
-    expect(isOnDemandMetricAlert(dataset, 'device.name:foo')).toBeTruthy();
-    expect(isOnDemandMetricAlert(dataset, 'geo.region:>US')).toBeTruthy();
+    expect(
+      isOnDemandMetricAlert(dataset, 'count()', 'transaction.duration:>1')
+    ).toBeTruthy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'device.name:foo')).toBeTruthy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'geo.region:>US')).toBeTruthy();
   });
 
   it('should return false for an alert that has only standard fields', () => {
     const dataset = Dataset.GENERIC_METRICS;
 
-    expect(isOnDemandMetricAlert(dataset, 'release:1.0')).toBeFalsy();
-    expect(isOnDemandMetricAlert(dataset, 'browser.name:chrome')).toBeFalsy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'release:1.0')).toBeFalsy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'browser.name:chrome')).toBeFalsy();
   });
 
   it('should return false if dataset is not generic_metrics', () => {
     const dataset = Dataset.TRANSACTIONS;
 
-    expect(isOnDemandMetricAlert(dataset, 'transaction.duration:>1')).toBeFalsy();
-    expect(isOnDemandMetricAlert(dataset, 'device.name:foo')).toBeFalsy();
-    expect(isOnDemandMetricAlert(dataset, 'geo.region:>US')).toBeFalsy();
+    expect(
+      isOnDemandMetricAlert(dataset, 'count()', 'transaction.duration:>1')
+    ).toBeFalsy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'device.name:foo')).toBeFalsy();
+    expect(isOnDemandMetricAlert(dataset, 'count()', 'geo.region:>US')).toBeFalsy();
+  });
+
+  it('should return true if aggregate is apdex', () => {
+    const dataset = Dataset.GENERIC_METRICS;
+
+    expect(isOnDemandMetricAlert(dataset, 'apdex(300)', '')).toBeTruthy();
+    expect(
+      isOnDemandMetricAlert(dataset, 'apdex(300)', 'transaction.duration:>1')
+    ).toBeTruthy();
+    expect(isOnDemandMetricAlert(dataset, 'apdex(300)', 'device.name:foo')).toBeTruthy();
   });
 });
diff --git a/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx b/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx
index fa6cc7b859e1c7..c38e2fd97068f4 100644
--- a/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx
+++ b/static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx
@@ -1,5 +1,5 @@
 import {AggregationKey} from 'sentry/utils/fields';
-import {isOnDemandQueryString} from 'sentry/utils/onDemandMetrics';
+import {isOnDemandAggregate, isOnDemandQueryString} from 'sentry/utils/onDemandMetrics';
 import {Dataset} from 'sentry/views/alerts/rules/metric/types';
 
 export function isValidOnDemandMetricAlert(
@@ -7,7 +7,7 @@ export function isValidOnDemandMetricAlert(
   aggregate: string,
   query: string
 ): boolean {
-  if (!isOnDemandMetricAlert(dataset, query)) {
+  if (!isOnDemandMetricAlert(dataset, aggregate, query)) {
     return true;
   }
 
@@ -19,6 +19,13 @@ export function isValidOnDemandMetricAlert(
  * 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.
  */
-export function isOnDemandMetricAlert(dataset: Dataset, query: string): boolean {
+export function isOnDemandMetricAlert(
+  dataset: Dataset,
+  aggregate: string,
+  query: string
+): boolean {
+  if (isOnDemandAggregate(aggregate)) {
+    return true;
+  }
   return dataset === Dataset.GENERIC_METRICS && isOnDemandQueryString(query);
 }


Code review comments:
diff_hunk: @@ -227,7 +227,7 @@ export default function MetricDetailsBody({
         <Layout.Side>
           <MetricDetailsSidebar
             rule={rule}
-            isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, query)}
+            isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, aggregate, query)}
path: static/app/views/alerts/rules/metric/details/body.tsx
text: @user1:
```suggestion
            isOnDemandMetricAlert={isOnDemandAlert}
```