diff --git a/static/app/views/alerts/rules/metric/constants.tsx b/static/app/views/alerts/rules/metric/constants.tsx index 328a04752992aa..55b89617864df4 100644 --- a/static/app/views/alerts/rules/metric/constants.tsx +++ b/static/app/views/alerts/rules/metric/constants.tsx @@ -172,6 +172,7 @@ export function createDefaultRule( environment: null, resolveThreshold: '', thresholdType: AlertRuleThresholdType.ABOVE, + detectionType: AlertRuleComparisonType.COUNT, ...defaultRuleOptions, }; } diff --git a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx index 0ecb37df121bea..05a5e40de916eb 100644 --- a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx @@ -69,8 +69,7 @@ import {getProjectOptions} from '../utils'; import {isCrashFreeAlert} from './utils/isCrashFreeAlert'; import {DEFAULT_AGGREGATE, DEFAULT_TRANSACTION_AGGREGATE} from './constants'; -import type {AlertRuleComparisonType} from './types'; -import {Dataset, Datasource, TimeWindow} from './types'; +import {AlertRuleComparisonType, Dataset, Datasource, TimeWindow} from './types'; const TIME_WINDOW_MAP: Record = { [TimeWindow.ONE_MINUTE]: t('1 minute'), @@ -267,6 +266,14 @@ class RuleConditionsForm extends PureComponent { ]); } + if (this.props.comparisonType === AlertRuleComparisonType.DYNAMIC) { + options = pick(TIME_WINDOW_MAP, [ + TimeWindow.FIFTEEN_MINUTES, + TimeWindow.THIRTY_MINUTES, + TimeWindow.ONE_HOUR, + ]); + } + return Object.entries(options).map(([value, label]) => ({ value: parseInt(value, 10), label: tct('[timeWindow] interval', { diff --git a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx index 142c52fc122b00..0d1e16c8a8c1e6 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx @@ -12,7 +12,12 @@ import ProjectsStore from 'sentry/stores/projectsStore'; import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import {metric} from 'sentry/utils/analytics'; import RuleFormContainer from 'sentry/views/alerts/rules/metric/ruleForm'; -import {Dataset} from 'sentry/views/alerts/rules/metric/types'; +import { + AlertRuleComparisonType, + AlertRuleSeasonality, + AlertRuleSensitivity, + Dataset, +} from 'sentry/views/alerts/rules/metric/types'; import {permissionAlertText} from 'sentry/views/settings/project/permissionAlert'; jest.mock('sentry/actionCreators/indicator'); @@ -351,6 +356,52 @@ describe('Incident Rules Form', () => { ); }); + it('creates an anomaly detection rule', async () => { + organization.features = [...organization.features, 'anomaly-detection-alerts']; + const rule = MetricRuleFixture({ + detectionType: AlertRuleComparisonType.PERCENT, + sensitivity: AlertRuleSensitivity.MEDIUM, + seasonality: AlertRuleSeasonality.AUTO, + }); + createWrapper({ + rule: { + ...rule, + id: undefined, + aggregate: 'count()', + eventTypes: ['error'], + dataset: 'events', + }, + }); + await userEvent.click( + screen.getByText('Anomaly: when evaluated values are outside of expected bounds') + ); + + expect( + await screen.findByLabelText( + 'Anomaly: when evaluated values are outside of expected bounds' + ) + ).toBeChecked(); + expect( + await screen.findByRole('textbox', {name: 'Sensitivity'}) + ).toBeInTheDocument(); + await userEvent.click(screen.getByLabelText('Save Rule')); + + expect(createRule).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ + data: expect.objectContaining({ + aggregate: 'count()', + dataset: 'events', + environment: null, + eventTypes: ['error'], + detectionType: AlertRuleComparisonType.DYNAMIC, + sensitivity: AlertRuleSensitivity.MEDIUM, + seasonality: AlertRuleSeasonality.AUTO, + }), + }) + ); + }); + it('switches to custom metric and selects event.type:error', async () => { organization.features = [...organization.features, 'performance-view']; const rule = MetricRuleFixture(); diff --git a/static/app/views/alerts/rules/metric/ruleForm.tsx b/static/app/views/alerts/rules/metric/ruleForm.tsx index adb6f86022889f..52f78c3963a06b 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.tsx @@ -85,12 +85,14 @@ import { DEFAULT_COUNT_TIME_WINDOW, } from './constants'; import RuleConditionsForm from './ruleConditionsForm'; -import type { - EventTypes, - MetricActionTemplate, - MetricRule, - Trigger, - UnsavedMetricRule, +import { + AlertRuleSeasonality, + AlertRuleSensitivity, + type EventTypes, + type MetricActionTemplate, + type MetricRule, + type Trigger, + type UnsavedMetricRule, } from './types'; import { AlertRuleComparisonType, @@ -142,6 +144,7 @@ type State = { project: Project; query: string; resolveThreshold: UnsavedMetricRule['resolveThreshold']; + sensitivity: UnsavedMetricRule['sensitivity']; thresholdPeriod: UnsavedMetricRule['thresholdPeriod']; thresholdType: UnsavedMetricRule['thresholdType']; timeWindow: number; @@ -151,6 +154,7 @@ type State = { comparisonDelta?: number; isExtrapolatedChartData?: boolean; monitorType?: MonitorType; + seasonality?: AlertRuleSeasonality; } & DeprecatedAsyncComponent['state']; const isEmpty = (str: unknown): boolean => str === '' || !defined(str); @@ -233,6 +237,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { metricExtractionRules: null, triggers: triggersClone, resolveThreshold: rule.resolveThreshold, + sensitivity: null, thresholdType: rule.thresholdType, thresholdPeriod: rule.thresholdPeriod ?? 1, comparisonDelta: rule.comparisonDelta ?? undefined, @@ -455,7 +460,25 @@ class RuleFormContainer extends DeprecatedAsyncComponent { ) { const {comparisonType} = this.state; const triggerErrors = new Map(); - + // If we have an anomaly detection alert, then we don't need to validate the thresholds, but we do need to set them to 0 + if (comparisonType === AlertRuleComparisonType.DYNAMIC) { + // NOTE: we don't support warning triggers for anomaly detection alerts yet + // once we do, uncomment this code and delete 475-478: + // triggers.forEach(trigger => { + // trigger.alertThreshold = 0; + // }); + const criticalTriggerIndex = triggers.findIndex( + ({label}) => label === AlertRuleTriggerType.CRITICAL + ); + const warningTriggerIndex = criticalTriggerIndex ^ 1; + const triggersCopy = [...triggers]; + const criticalTrigger = triggersCopy[criticalTriggerIndex]; + const warningTrigger = triggersCopy[warningTriggerIndex]; + criticalTrigger.alertThreshold = 0; + warningTrigger.alertThreshold = ''; // we need to set this to empty + this.setState({triggers: triggersCopy}); + return triggerErrors; // return an empty map + } const requiredFields = ['label', 'alertThreshold']; triggers.forEach((trigger, triggerIndex) => { requiredFields.forEach(field => { @@ -725,6 +748,9 @@ class RuleFormContainer extends DeprecatedAsyncComponent { eventTypes, monitorType, activationCondition, + sensitivity, + seasonality, + comparisonType, } = this.state; // Remove empty warning trigger const sanitizedTriggers = triggers.filter( @@ -761,7 +787,12 @@ class RuleFormContainer extends DeprecatedAsyncComponent { activationCondition, }; } - + const detectionTypes = new Map([ + [AlertRuleComparisonType.COUNT, 'static'], + [AlertRuleComparisonType.CHANGE, 'percent'], + [AlertRuleComparisonType.DYNAMIC, 'dynamic'], + ]); + const detectionType = detectionTypes.get(comparisonType) ?? ''; const dataset = this.determinePerformanceDataset(); this.setState({loading: true}); // Add or update is just the PUT/POST to the org alert-rules api @@ -785,6 +816,9 @@ class RuleFormContainer extends DeprecatedAsyncComponent { eventTypes: isCrashFreeAlert(rule.dataset) ? undefined : eventTypes, dataset, queryType: DatasetMEPAlertQueryTypes[dataset], + sensitivity: sensitivity ?? null, + seasonality: seasonality ?? null, + detectionType: detectionType, }, { duplicateRule: this.isDuplicateRule ? 'true' : 'false', @@ -819,7 +853,13 @@ class RuleFormContainer extends DeprecatedAsyncComponent { ? err?.responseJSON : Object.values(err?.responseJSON) : []; - const apiErrors = errors.length > 0 ? `: ${errors.join(', ')}` : ''; + let apiErrors = ''; + if (typeof errors[0] === 'object') { + // NOTE: this occurs if we get a TimeoutError when attempting to hit the Seer API + apiErrors = ': ' + errors[0].message; + } else { + apiErrors = errors.length > 0 ? `: ${errors.join(', ')}` : ''; + } this.handleRuleSaveFailure(t('Unable to save alert%s', apiErrors)); } }); @@ -850,6 +890,10 @@ class RuleFormContainer extends DeprecatedAsyncComponent { }); }; + handleSensitivityChange = (sensitivity: AlertRuleSensitivity) => { + this.setState({sensitivity}); + }; + handleThresholdTypeChange = (thresholdType: AlertRuleThresholdType) => { const {triggers} = this.state; @@ -883,13 +927,25 @@ class RuleFormContainer extends DeprecatedAsyncComponent { handleComparisonTypeChange = (value: AlertRuleComparisonType) => { const comparisonDelta = - value === AlertRuleComparisonType.COUNT - ? undefined - : this.state.comparisonDelta ?? DEFAULT_CHANGE_COMP_DELTA; + value === AlertRuleComparisonType.CHANGE + ? this.state.comparisonDelta ?? DEFAULT_CHANGE_COMP_DELTA + : undefined; const timeWindow = this.state.comparisonDelta ? DEFAULT_COUNT_TIME_WINDOW : DEFAULT_CHANGE_TIME_WINDOW; - this.setState({comparisonType: value, comparisonDelta, timeWindow}); + const sensitivity = + value === AlertRuleComparisonType.DYNAMIC + ? this.state.sensitivity || AlertRuleSensitivity.MEDIUM + : undefined; + const seasonality = + value === AlertRuleComparisonType.DYNAMIC ? AlertRuleSeasonality.AUTO : undefined; // TODO: replace "auto" with the correct constant + this.setState({ + comparisonType: value, + comparisonDelta, + timeWindow, + sensitivity, + seasonality, + }); }; handleDeleteRule = async () => { @@ -1096,6 +1152,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { comparisonDelta, comparisonType, resolveThreshold, + sensitivity, loading, eventTypes, dataset, @@ -1120,6 +1177,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { aggregate={aggregate} isMigration={isMigration} resolveThreshold={resolveThreshold} + sensitivity={sensitivity} thresholdPeriod={thresholdPeriod} thresholdType={thresholdType} comparisonType={comparisonType} @@ -1130,6 +1188,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { onThresholdTypeChange={this.handleThresholdTypeChange} onThresholdPeriodChange={this.handleThresholdPeriodChange} onResolveThresholdChange={this.handleResolveThresholdChange} + onSensitivityChange={this.handleSensitivityChange} /> ); diff --git a/static/app/views/alerts/rules/metric/triggers/actionsPanel/index.tsx b/static/app/views/alerts/rules/metric/triggers/actionsPanel/index.tsx index d29bdd5e1d1b5c..e2e089c18e984b 100644 --- a/static/app/views/alerts/rules/metric/triggers/actionsPanel/index.tsx +++ b/static/app/views/alerts/rules/metric/triggers/actionsPanel/index.tsx @@ -24,11 +24,12 @@ import SentryAppRuleModal from 'sentry/views/alerts/rules/issue/sentryAppRuleMod import ActionSpecificTargetSelector from 'sentry/views/alerts/rules/metric/triggers/actionsPanel/actionSpecificTargetSelector'; import ActionTargetSelector from 'sentry/views/alerts/rules/metric/triggers/actionsPanel/actionTargetSelector'; import DeleteActionButton from 'sentry/views/alerts/rules/metric/triggers/actionsPanel/deleteActionButton'; -import type { - Action, - ActionType, - MetricActionTemplate, - Trigger, +import { + type Action, + type ActionType, + AlertRuleComparisonType, + type MetricActionTemplate, + type Trigger, } from 'sentry/views/alerts/rules/metric/types'; import { ActionLabel, @@ -39,6 +40,7 @@ import { type Props = { availableActions: MetricActionTemplate[] | null; + comparisonType: AlertRuleComparisonType; currentProject: string; disabled: boolean; error: boolean; @@ -311,6 +313,7 @@ class ActionsPanel extends PureComponent { organization, projects, triggers, + comparisonType, } = this.props; const project = projects.find(({slug}) => slug === currentProject); @@ -327,6 +330,10 @@ class ActionsPanel extends PureComponent { {value: 1, label: 'Warning Status'}, ]; + // NOTE: we don't support warning triggers for anomaly detection alerts yet + // once we do, this can be deleted + const anomalyDetectionLevels = [{value: 0, label: 'Critical Status'}]; + // Create single array of unsaved and saved trigger actions // Sorted by date created ascending const actions = triggers @@ -371,7 +378,11 @@ class ActionsPanel extends PureComponent { actionIdx )} value={triggerIndex} - options={levels} + options={ + comparisonType === AlertRuleComparisonType.DYNAMIC + ? anomalyDetectionLevels + : levels + } /> void; + onThresholdTypeChange: (thresholdType: AlertRuleThresholdType) => void; + sensitivity: UnsavedMetricRule['sensitivity']; + thresholdType: UnsavedMetricRule['thresholdType']; + /** + * Map of fieldName -> errorMessage + */ + error?: {[fieldName: string]: string}; + + hideControl?: boolean; +}; + +type SensitivityFormItemProps = { + onSensitivityChange: (sensitivity: AlertRuleSensitivity) => void; + sensitivity: UnsavedMetricRule['sensitivity']; +}; + +type DirectionFormItemProps = { + onThresholdTypeChange: (thresholdType: AlertRuleThresholdType) => void; + thresholdType: UnsavedMetricRule['thresholdType']; +}; + +function SensitivityFormItem({ + sensitivity, + onSensitivityChange, +}: SensitivityFormItemProps) { + return ( + + + { + onSensitivityChange(value); + }} + /> + + + ); +} + +function DirectionFormItem({ + thresholdType, + onThresholdTypeChange, +}: DirectionFormItemProps) { + return ( + + + { + onThresholdTypeChange(value); + }} + /> + + + ); +} + +class AnomalyDetectionFormField extends Component { + render() { + const {sensitivity, onSensitivityChange, thresholdType, onThresholdTypeChange} = + this.props; + + return ( + + + + + ); + } +} + +const StyledField = styled(FieldGroup)` + & > label > div:first-child > span { + display: flex; + flex-direction: row; + } +`; + +const SelectContainer = styled('div')` + flex: 1; +`; +export default AnomalyDetectionFormField; diff --git a/static/app/views/alerts/rules/metric/triggers/index.tsx b/static/app/views/alerts/rules/metric/triggers/index.tsx index 61f2d76fb03f9a..e1657b9ab163bf 100644 --- a/static/app/views/alerts/rules/metric/triggers/index.tsx +++ b/static/app/views/alerts/rules/metric/triggers/index.tsx @@ -7,11 +7,13 @@ import type {Project} from 'sentry/types/project'; import removeAtArrayIndex from 'sentry/utils/array/removeAtArrayIndex'; import replaceAtArrayIndex from 'sentry/utils/array/replaceAtArrayIndex'; import ActionsPanel from 'sentry/views/alerts/rules/metric/triggers/actionsPanel'; +import AnomalyDetectionFormField from 'sentry/views/alerts/rules/metric/triggers/anomalyAlertsForm'; import TriggerForm from 'sentry/views/alerts/rules/metric/triggers/form'; import { type Action, AlertRuleComparisonType, + type AlertRuleSensitivity, type AlertRuleThresholdType, type MetricActionTemplate, type Trigger, @@ -33,14 +35,16 @@ type Props = { onResolveThresholdChange: ( resolveThreshold: UnsavedMetricRule['resolveThreshold'] ) => void; + onSensitivityChange: (sensitivity: AlertRuleSensitivity) => void; onThresholdPeriodChange: (value: number) => void; onThresholdTypeChange: (thresholdType: AlertRuleThresholdType) => void; organization: Organization; projects: Project[]; resolveThreshold: UnsavedMetricRule['resolveThreshold']; - thresholdPeriod: UnsavedMetricRule['thresholdPeriod']; + sensitivity: UnsavedMetricRule['sensitivity']; + thresholdPeriod: UnsavedMetricRule['thresholdPeriod']; thresholdType: UnsavedMetricRule['thresholdType']; triggers: Trigger[]; isMigration?: boolean; @@ -107,9 +111,11 @@ class Triggers extends Component { comparisonType, resolveThreshold, isMigration, + onSensitivityChange, onThresholdTypeChange, onResolveThresholdChange, onThresholdPeriodChange, + sensitivity, } = this.props; // Note we only support 2 triggers max @@ -118,7 +124,13 @@ class Triggers extends Component { {comparisonType === AlertRuleComparisonType.DYNAMIC ? ( -
{'This is where the anomaly detection alert field choices go'}
+ ) : ( { triggers={triggers} onChange={this.handleChangeActions} onAdd={this.handleAddAction} + comparisonType={comparisonType} /> )} diff --git a/static/app/views/alerts/rules/metric/types.tsx b/static/app/views/alerts/rules/metric/types.tsx index ca274eda702b6c..dce46532a68556 100644 --- a/static/app/views/alerts/rules/metric/types.tsx +++ b/static/app/views/alerts/rules/metric/types.tsx @@ -12,6 +12,7 @@ import type {Incident} from '../../types'; export enum AlertRuleThresholdType { ABOVE = 0, BELOW = 1, + ABOVE_AND_BELOW = 2, } export enum AlertRuleTriggerType { @@ -56,6 +57,16 @@ export enum Datasource { TRANSACTION = 'transaction', } +export enum AlertRuleSensitivity { + LOW = 'low', + MEDIUM = 'medium', + HIGH = 'high', +} + +export enum AlertRuleSeasonality { + AUTO = 'auto', +} + /** * This is not a real aggregate as crash-free sessions/users can be only calculated on frontend by comparing the count of sessions broken down by status * It is here nevertheless to shoehorn sessions dataset into existing alerts codebase @@ -95,6 +106,7 @@ export type Trigger = Partial & UnsavedTrigger; export type UnsavedMetricRule = { aggregate: string; dataset: Dataset; + detectionType: string; environment: string | null; projects: string[]; query: string; @@ -110,6 +122,8 @@ export type UnsavedMetricRule = { monitorWindow?: number | null; owner?: string | null; queryType?: MEPAlertsQueryType | null; + seasonality?: AlertRuleSeasonality | null; + sensitivity?: AlertRuleSensitivity | null; }; // Form values for updating a metric alert rule diff --git a/tests/js/fixtures/metricRule.ts b/tests/js/fixtures/metricRule.ts index df12d87bd070d9..311641e8b1c48d 100644 --- a/tests/js/fixtures/metricRule.ts +++ b/tests/js/fixtures/metricRule.ts @@ -1,7 +1,7 @@ import {IncidentTriggerFixture} from 'sentry-fixture/incidentTrigger'; import type {SavedMetricRule as SavedMetricRule} from 'sentry/views/alerts/rules/metric/types'; -import {Dataset} from 'sentry/views/alerts/rules/metric/types'; +import {AlertRuleComparisonType, Dataset} from 'sentry/views/alerts/rules/metric/types'; export function MetricRuleFixture( params: Partial = {} @@ -24,6 +24,7 @@ export function MetricRuleFixture( thresholdType: 0, thresholdPeriod: 1, environment: null, + detectionType: AlertRuleComparisonType.COUNT, ...params, }; }