Browse Source

feat(alerts): common field support for on-demand metrics (#52613)

Ogi 1 year ago
parent
commit
dc7a4d7ac5

+ 6 - 0
static/app/utils/fields/index.ts

@@ -118,6 +118,7 @@ export enum FieldKey {
   USER_ID = 'user.id',
   USER_IP = 'user.ip',
   USER_USERNAME = 'user.username',
+  USER_SEGMENT = 'user.segment',
   APP_IN_FOREGROUND = 'app.in_foreground',
 }
 
@@ -983,6 +984,11 @@ const EVENT_FIELD_DEFINITIONS: Record<AllEventFieldKeys, FieldDefinition> = {
     kind: FieldKind.FIELD,
     valueType: FieldValueType.STRING,
   },
+  [FieldKey.USER_SEGMENT]: {
+    desc: t('Segment of the user'),
+    kind: FieldKind.FIELD,
+    valueType: FieldValueType.STRING,
+  },
   [FieldKey.APP_IN_FOREGROUND]: {
     desc: t('Indicates if the app is in the foreground or background'),
     kind: FieldKind.FIELD,

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

@@ -503,7 +503,7 @@ class MetricChart extends PureComponent<Props, State> {
       newAlertOrQuery: false,
     });
 
-    if (isOnDemandMetricAlert(rule.query)) {
+    if (isOnDemandMetricAlert(rule.dataset, rule.query)) {
       return this.renderEmpty(
         t(
           'This alert includes advanced conditions, which is a feature that is currently in early access. Charts are not yet supported at this time.'

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

@@ -360,6 +360,7 @@ describe('Incident Rules Form', () => {
       const invalidOnDemandMetricRule = TestStubs.MetricRule({
         aggregate: 'percentile()',
         query: 'transaction.duration:<1s',
+        dataset: 'generic_metrics',
       });
 
       const onSubmitSuccess = jest.fn();

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

@@ -37,6 +37,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 {
+  hasNonStandardMetricSearchFilters,
   isOnDemandMetricAlert,
   isValidOnDemandMetricAlert,
 } from 'sentry/views/alerts/rules/metric/utils/onDemandMetricAlert';
@@ -130,7 +131,10 @@ function determineAlertDataset(
     return selectedDataset;
   }
 
-  if (isOnDemandMetricAlert(query) && selectedDataset === Dataset.TRANSACTIONS) {
+  if (
+    hasNonStandardMetricSearchFilters(query) &&
+    selectedDataset === Dataset.TRANSACTIONS
+  ) {
     // for on-demand metrics extraction we want to override the dataset and use performance metrics instead
     return Dataset.GENERIC_METRICS;
   }
@@ -522,7 +526,11 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
   };
 
   validateOnDemandMetricAlert() {
-    return isValidOnDemandMetricAlert(this.state.aggregate, this.state.query);
+    return isValidOnDemandMetricAlert(
+      this.state.dataset,
+      this.state.aggregate,
+      this.state.query
+    );
   }
 
   handleSubmit = async (
@@ -844,7 +852,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
     const wizardBuilderChart = (
       <TriggersChart
         {...chartProps}
-        isOnDemandMetricAlert={isOnDemandMetricAlert(query)}
+        isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, query)}
         header={
           <ChartHeader>
             <AlertName>{AlertWizardAlertNames[alertType]}</AlertName>

+ 67 - 11
static/app/views/alerts/rules/metric/utils/onDemandMetricAlert.tsx

@@ -1,15 +1,13 @@
-import {AggregationKey} from 'sentry/utils/fields';
+import {ParseResult, parseSearch, Token} from 'sentry/components/searchSyntax/parser';
+import {AggregationKey, FieldKey} from 'sentry/utils/fields';
+import {Dataset} from 'sentry/views/alerts/rules/metric/types';
 
-/**
- * Currently we determine that an alert is an on-demand metric alert if the query contains
- * the string 'transaction.duration'. This should be extended in the future
- */
-export function isOnDemandMetricAlert(query: string): boolean {
-  return query.includes('transaction.duration');
-}
-
-export function isValidOnDemandMetricAlert(aggregate: string, query: string): boolean {
-  if (!isOnDemandMetricAlert(query)) {
+export function isValidOnDemandMetricAlert(
+  dataset: Dataset,
+  aggregate: string,
+  query: string
+): boolean {
+  if (!isOnDemandMetricAlert(dataset, query)) {
     return true;
   }
 
@@ -21,3 +19,61 @@ export function isValidOnDemandMetricAlert(aggregate: string, query: string): bo
 
   return !unsupportedAggregates.some(agg => aggregate.includes(agg));
 }
+
+const STANDARD_SEARCH_FIELD_KEYS = new Set([
+  FieldKey.RELEASE,
+  FieldKey.DIST,
+  FieldKey.ENVIRONMENT,
+  FieldKey.TRANSACTION,
+  FieldKey.PLATFORM,
+  FieldKey.TRANSACTION_OP,
+  FieldKey.TRANSACTION_STATUS,
+  FieldKey.HTTP_METHOD,
+  FieldKey.HTTP_STATUS_CODE,
+  FieldKey.BROWSER_NAME,
+  FieldKey.OS_NAME,
+  FieldKey.GEO_COUNTRY_CODE,
+]);
+
+/**
+ * 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 {
+  return dataset === Dataset.GENERIC_METRICS && hasNonStandardMetricSearchFilters(query);
+}
+
+export function hasNonStandardMetricSearchFilters(query: string): boolean {
+  const tokens = parseSearch(query);
+  if (!tokens) {
+    return false;
+  }
+
+  const searchFilterKeys = getSearchFilterKeys(tokens);
+  return searchFilterKeys.some(key => !STANDARD_SEARCH_FIELD_KEYS.has(key));
+}
+
+type SearchFilterKey = FieldKey | null;
+
+function getSearchFilterKeys(tokens: ParseResult): FieldKey[] {
+  try {
+    return getTokenKeys(tokens).filter(Boolean) as FieldKey[];
+  } catch (e) {
+    return [];
+  }
+}
+
+function getTokenKeys(tokens: ParseResult): SearchFilterKey[] {
+  return tokens.flatMap(getTokenKey);
+}
+
+function getTokenKey(token): SearchFilterKey[] | SearchFilterKey {
+  if (token.type === Token.LOGIC_GROUP) {
+    return getTokenKeys(token.inner);
+  }
+  if (token.type === Token.FILTER) {
+    return token.key.value;
+  }
+
+  return null;
+}

+ 39 - 1
static/app/views/alerts/wizard/options.tsx

@@ -233,6 +233,44 @@ const INDEXED_PERFORMANCE_ALERTS_OMITTED_TAGS = [
   ...Object.values(ReplayClickFieldKey),
 ];
 
+// This list matches currently supported tags in metrics extraction defined in
+// https://github.com/getsentry/sentry/blob/2fd2459c274dc81c079fd4c31b2755114602ef7c/src/sentry/snuba/metrics/extraction.py#L42
+export const ON_DEMAND_METRICS_SUPPORTED_TAGS = [
+  FieldKey.RELEASE,
+  FieldKey.DIST,
+  FieldKey.ENVIRONMENT,
+  FieldKey.TRANSACTION,
+  FieldKey.PLATFORM,
+
+  FieldKey.USER_EMAIL,
+  FieldKey.USER_ID,
+  FieldKey.USER_IP,
+  FieldKey.USER_USERNAME,
+  FieldKey.USER_SEGMENT,
+  FieldKey.GEO_CITY,
+  FieldKey.GEO_COUNTRY_CODE,
+  FieldKey.GEO_REGION,
+  FieldKey.GEO_SUBDIVISION,
+  FieldKey.HTTP_METHOD,
+
+  FieldKey.DEVICE_NAME,
+  FieldKey.DEVICE_FAMILY,
+  FieldKey.OS_NAME,
+  FieldKey.OS_KERNEL_VERSION,
+  FieldKey.BROWSER_NAME,
+  FieldKey.TRANSACTION_OP,
+  FieldKey.TRANSACTION_STATUS,
+  FieldKey.HTTP_STATUS_CODE,
+
+  FieldKey.TRANSACTION_DURATION,
+  FieldKey.RELEASE_BUILD,
+  FieldKey.RELEASE_PACKAGE,
+  FieldKey.RELEASE_VERSION,
+
+  ...Object.values(WebVital),
+  ...Object.values(MobileVital),
+] as FieldKey[];
+
 // Some data sets support a very limited number of tags. For these cases,
 // define all supported tags explicitly
 export function datasetSupportedTags(
@@ -259,7 +297,7 @@ export function datasetSupportedTags(
 
 function transactionSupportedTags(org: Organization) {
   if (org.features.includes('on-demand-metrics-extraction')) {
-    return [...TRANSACTION_SUPPORTED_TAGS, FieldKey.TRANSACTION_DURATION];
+    return ON_DEMAND_METRICS_SUPPORTED_TAGS;
   }
   return TRANSACTION_SUPPORTED_TAGS;
 }