diff --git a/static/app/routes.tsx b/static/app/routes.tsx index 33baef54b61a34..91a296041a6bff 100644 --- a/static/app/routes.tsx +++ b/static/app/routes.tsx @@ -1212,6 +1212,7 @@ function buildRoutes() { ); const alertChildRoutes = ({forCustomerDomain}: {forCustomerDomain: boolean}) => { + // ALERT CHILD ROUTES return ( ); }; + // ALERT ROUTES const alertRoutes = ( {USING_CUSTOMER_DOMAIN && ( diff --git a/static/app/types/alerts.tsx b/static/app/types/alerts.tsx index 2748c00f9a57b0..a0dce16277d690 100644 --- a/static/app/types/alerts.tsx +++ b/static/app/types/alerts.tsx @@ -291,3 +291,8 @@ export enum MonitorType { CONTINUOUS = 0, ACTIVATED = 1, } + +export enum ActivationConditionType { + RELEASE_CREATION = 0, + DEPLOY_CREATION = 1, +} diff --git a/static/app/views/alerts/rules/metric/actions.tsx b/static/app/views/alerts/rules/metric/actions.tsx index 1d233595740d15..0cc3e3dedf3bcb 100644 --- a/static/app/views/alerts/rules/metric/actions.tsx +++ b/static/app/views/alerts/rules/metric/actions.tsx @@ -7,7 +7,7 @@ function isSavedRule(rule: MetricRule): rule is SavedMetricRule { } /** - * Add a new rule or update an existing rule + * Add a new alert rule or update an existing alert rule * * @param api API Client * @param orgId Organization slug @@ -16,7 +16,7 @@ function isSavedRule(rule: MetricRule): rule is SavedMetricRule { */ export function addOrUpdateRule( api: Client, - orgId: string, + orgId: string, // organization slug rule: MetricRule, query?: object | any ) { diff --git a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx index 4ea870920c711c..ae0897dad95d86 100644 --- a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx @@ -24,6 +24,7 @@ import {SearchInvalidTag} from 'sentry/components/smartSearchBar/searchInvalidTa import {t, tct} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {Environment, Organization, Project, SelectValue} from 'sentry/types'; +import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import {getDisplayName} from 'sentry/utils/environment'; import {hasCustomMetrics} from 'sentry/utils/metrics/features'; import {getMRI} from 'sentry/utils/metrics/mri'; @@ -69,6 +70,12 @@ type Props = { disabled: boolean; onComparisonDeltaChange: (value: number) => void; onFilterSearch: (query: string, isQueryValid) => void; + onMonitorTypeSelect: (activatedAlertFields: { + activationCondition?: ActivationConditionType | undefined; + monitorType?: MonitorType; + monitorWindowSuffix?: string | undefined; + monitorWindowValue?: number | undefined; + }) => void; onTimeWindowChange: (value: number) => void; organization: Organization; project: Project; @@ -76,6 +83,7 @@ type Props = { router: InjectedRouter; thresholdChart: React.ReactNode; timeWindow: number; + activationCondition?: ActivationConditionType; allowChangeEventTypes?: boolean; comparisonDelta?: number; disableProjectSelector?: boolean; @@ -83,6 +91,7 @@ type Props = { isExtrapolatedChartData?: boolean; isTransactionMigration?: boolean; loadingProjects?: boolean; + monitorType?: number; }; type State = { @@ -162,6 +171,20 @@ class RuleConditionsForm extends PureComponent { } } + get selectControlStyles() { + return { + control: (provided: {[x: string]: string | number | boolean}) => ({ + ...provided, + minWidth: 200, + maxWidth: 300, + }), + container: (provided: {[x: string]: string | number | boolean}) => ({ + ...provided, + margin: `${space(0.5)}`, + }), + }; + } + renderEventTypeFilter() { const {organization, disabled, alertType, isErrorMigration} = this.props; @@ -326,8 +349,15 @@ class RuleConditionsForm extends PureComponent { } renderInterval() { - const {organization, disabled, alertType, timeWindow, onTimeWindowChange, project} = - this.props; + const { + organization, + disabled, + alertType, + timeWindow, + onTimeWindowChange, + project, + monitorType, + } = this.props; return ( @@ -353,27 +383,107 @@ class RuleConditionsForm extends PureComponent { alertType={alertType} required /> - ({ - ...provided, - minWidth: 200, - maxWidth: 300, - }), - container: (provided: {[x: string]: string | number | boolean}) => ({ - ...provided, - margin: `${space(0.5)}`, - }), - }} - options={this.timeWindowOptions} - required - isDisabled={disabled} - value={timeWindow} - onChange={({value}) => onTimeWindowChange(value)} - inline={false} - flexibleControlStateSize - /> + {monitorType === MonitorType.CONTINUOUS && ( + onTimeWindowChange(value)} + inline={false} + flexibleControlStateSize + /> + )} + + + ); + } + + renderMonitorTypeSelect() { + const { + onMonitorTypeSelect, + monitorType, + activationCondition, + timeWindow, + onTimeWindowChange, + } = this.props; + + return ( + + + +
{t('Select Monitor Type')}
+ + + + + + onMonitorTypeSelect({ + monitorType: MonitorType.CONTINUOUS, + activationCondition, + }) + } + > + {t('Continuous')} +
{t('Continuously monitor trends for the metrics outlined below')}
+ + + onMonitorTypeSelect({ + monitorType: MonitorType.ACTIVATED, + }) + } + > + Conditional + {monitorType === MonitorType.ACTIVATED ? ( + + {`${t('Monitor')} `} + + onMonitorTypeSelect({activationCondition: value}) + } + inline={false} + flexibleControlStateSize + /> + {` ${t('for')} `} + onTimeWindowChange(value)} + inline={false} + flexibleControlStateSize + /> + + ) : ( +
+ {t('Temporarily monitor specified query given activation condition')} +
+ )} + + ); @@ -395,6 +505,7 @@ class RuleConditionsForm extends PureComponent { } = this.props; const {environments} = this.state; + const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); const environmentOptions: SelectValue[] = [ { @@ -425,6 +536,7 @@ class RuleConditionsForm extends PureComponent { )} /> )} + {hasActivatedAlerts && this.renderMonitorTypeSelect()} {!isErrorMigration && this.renderInterval()} {t('Filter events')} @@ -653,4 +765,46 @@ const FormRow = styled('div')<{columns?: number; noMargin?: boolean}>` `} `; +const MonitorSelect = styled('div')` + border-radius: ${p => p.theme.borderRadius}; + border: 1px solid ${p => p.theme.border}; + width: 100%; + display: grid; + grid-template-columns: 1fr 1fr; +`; + +type MonitorCardProps = { + isSelected: boolean; + /** + * Adds hover and focus states to the card + */ + position: 'left' | 'right'; +}; + +const MonitorCard = styled('div')` + padding: ${space(1)}; + display: flex; + flex-grow: 1; + flex-direction: column; + cursor: pointer; + + &:focus, + &:hover { + outline: 1px solid ${p => p.theme.purple200}; + background-color: ${p => p.theme.backgroundSecondary}; + } + + border-top-left-radius: ${p => (p.position === 'left' ? p.theme.borderRadius : 0)}; + border-bottom-left-radius: ${p => (p.position === 'left' ? p.theme.borderRadius : 0)}; + border-top-right-radius: ${p => (p.position !== 'left' ? p.theme.borderRadius : 0)}; + border-bottom-right-radius: ${p => (p.position !== 'left' ? p.theme.borderRadius : 0)}; + outline: ${p => (p.isSelected ? `1px solid ${p.theme.purple400}` : 'none')}; +`; + +const ActivatedAlertFields = styled('div')` + display: flex; + align-items: center; + justify-content: space-between; +`; + export default withApi(withProjects(RuleConditionsForm)); diff --git a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx index 308a0d7279a3a1..aaf2823b62f257 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx @@ -9,6 +9,7 @@ import selectEvent from 'sentry-test/selectEvent'; import {addErrorMessage} from 'sentry/actionCreators/indicator'; import type FormModel from 'sentry/components/forms/model'; 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'; @@ -254,6 +255,45 @@ describe('Incident Rules Form', () => { ); }); + it('creates a rule with an activation condition', async () => { + organization.features = [ + ...organization.features, + 'mep-rollout-flag', + 'activated-alert-rules', + ]; + const rule = MetricRuleFixture({ + monitorType: MonitorType.ACTIVATED, + activationCondition: ActivationConditionType.RELEASE_CREATION, + }); + createWrapper({ + rule: { + ...rule, + id: undefined, + aggregate: 'count()', + eventTypes: ['transaction'], + dataset: 'transactions', + }, + }); + + expect(await screen.findByTestId('alert-total-events')).toHaveTextContent('Total5'); + + await userEvent.click(screen.getByLabelText('Save Rule')); + + expect(createRule).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + data: expect.objectContaining({ + name: 'My Incident Rule', + projects: ['project-slug'], + aggregate: 'count()', + eventTypes: ['transaction'], + dataset: 'generic_metrics', + thresholdPeriod: 1, + }), + }) + ); + }); + 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 8a48fc1e7aba49..9350991cef4451 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.tsx @@ -32,6 +32,7 @@ import type { Organization, Project, } from 'sentry/types'; +import {type ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import {defined} from 'sentry/utils'; import {metric, trackAnalytics} from 'sentry/utils/analytics'; import type EventView from 'sentry/utils/discover/eventView'; @@ -137,8 +138,10 @@ type State = { timeWindow: number; triggerErrors: Map; triggers: Trigger[]; + activationCondition?: ActivationConditionType; comparisonDelta?: number; isExtrapolatedChartData?: boolean; + monitorType?: MonitorType; } & DeprecatedAsyncComponent['state']; const isEmpty = (str: unknown): boolean => str === '' || !defined(str); @@ -178,7 +181,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { } getDefaultState(): State { - const {rule, location} = this.props; + const {rule, location, organization} = this.props; const triggersClone = [...rule.triggers]; const { aggregate: _aggregate, @@ -205,6 +208,8 @@ class RuleFormContainer extends DeprecatedAsyncComponent { ? `is:unresolved ${rule.query ?? ''}` : rule.query ?? ''; + const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); + return { ...super.getDefaultState(), @@ -229,6 +234,10 @@ class RuleFormContainer extends DeprecatedAsyncComponent { project: this.props.project, owner: rule.owner, alertType: getAlertTypeFromAggregateDataset({aggregate, dataset}), + monitorType: hasActivatedAlerts + ? rule.monitorType || MonitorType.CONTINUOUS + : undefined, + activationCondition: rule.activationCondition, }; } @@ -560,6 +569,24 @@ class RuleFormContainer extends DeprecatedAsyncComponent { this.setState({query, dataset, isQueryValid}); }; + handleMonitorTypeSelect = (activatedAlertFields: { + activationCondition?: ActivationConditionType | undefined; + monitorType?: MonitorType; + monitorWindowSuffix?: string | undefined; + monitorWindowValue?: number | undefined; + }) => { + const {monitorType} = activatedAlertFields; + let updatedFields = activatedAlertFields; + if (monitorType === MonitorType.CONTINUOUS) { + updatedFields = { + ...updatedFields, + activationCondition: undefined, + monitorWindowValue: undefined, + }; + } + this.setState(updatedFields as State); + }; + validateOnDemandMetricAlert() { if ( !isOnDemandMetricAlert(this.state.dataset, this.state.aggregate, this.state.query) @@ -570,16 +597,24 @@ class RuleFormContainer extends DeprecatedAsyncComponent { return !this.state.aggregate.includes(AggregationKey.PERCENTILE); } - handleSubmit = async ( - _data: Partial, - _onSubmitSuccess, - _onSubmitError, - _e, - model: FormModel - ) => { + validateActivatedAlerts() { + const {organization} = this.props; + const {monitorType, activationCondition, timeWindow} = this.state; + + const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); + return ( + !hasActivatedAlerts || + (hasActivatedAlerts && + monitorType && + activationCondition !== undefined && + timeWindow) + ); + } + + validateSubmit = model => { if (!this.validateMri()) { addErrorMessage(t('You need to select a metric before you can save the alert')); - return; + return false; } // This validates all fields *except* for Triggers const validRule = model.validateForm(); @@ -588,6 +623,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { const triggerErrors = this.validateTriggers(); const validTriggers = Array.from(triggerErrors).length === 0; const validOnDemandAlert = this.validateOnDemandMetricAlert(); + const validActivatedAlerts = this.validateActivatedAlerts(); if (!validTriggers) { this.setState(state => ({ @@ -603,13 +639,34 @@ class RuleFormContainer extends DeprecatedAsyncComponent { ].filter(x => x); addErrorMessage(t('Alert not valid: missing %s', missingFields.join(' '))); - return; + return false; } if (!validOnDemandAlert) { addErrorMessage( t('%s is not supported for on-demand metric alerts', this.state.aggregate) ); + return false; + } + + if (!validActivatedAlerts) { + addErrorMessage( + t('Activation condition and monitor window must be set for activated alerts') + ); + return false; + } + + return true; + }; + + handleSubmit = async ( + _data: Partial, + _onSubmitSuccess, + _onSubmitError, + _e, + model: FormModel + ) => { + if (!this.validateSubmit(model)) { return; } @@ -631,6 +688,8 @@ class RuleFormContainer extends DeprecatedAsyncComponent { comparisonDelta, timeWindow, eventTypes, + monitorType, + activationCondition, } = this.state; // Remove empty warning trigger const sanitizedTriggers = triggers.filter( @@ -638,6 +697,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { trigger.label !== AlertRuleTriggerType.WARNING || !isEmpty(trigger.alertThreshold) ); + const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); // form model has all form state data, however we use local state to keep // track of the list of triggers (and actions within triggers) const loadingIndicator = IndicatorStore.addMessage( @@ -659,14 +719,25 @@ class RuleFormContainer extends DeprecatedAsyncComponent { metric.startSpan({name: 'saveAlertRule'}); + let activatedAlertFields = {}; + if (hasActivatedAlerts) { + activatedAlertFields = { + monitorType, + activationCondition, + }; + } + const dataset = this.determinePerformanceDataset(); this.setState({loading: true}); + // Add or update is just the PUT/POST to the org alert-rules api + // we're splatting the full rule in, then overwriting all the data? const [data, , resp] = await addOrUpdateRule( this.api, organization.slug, { - ...rule, - ...model.getTransformedData(), + ...rule, // existing rule + ...model.getTransformedData(), // form data + ...activatedAlertFields, projects: [project.slug], triggers: sanitizedTriggers, resolveThreshold: isEmpty(resolveThreshold) ? null : resolveThreshold, @@ -973,6 +1044,8 @@ class RuleFormContainer extends DeprecatedAsyncComponent { alertType, isExtrapolatedChartData, triggersHaveChanged, + activationCondition, + monitorType, } = this.state; const wizardBuilderChart = this.renderTriggerChart(); @@ -1029,6 +1102,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { hasIgnoreArchivedFeatureFlag(organization) && ruleNeedsErrorMigration(rule); + // Rendering the main form body return (
@@ -1108,6 +1182,9 @@ class RuleFormContainer extends DeprecatedAsyncComponent { onTimeWindowChange={value => this.handleFieldChange('timeWindow', value)} disableProjectSelector={disableProjectSelector} isExtrapolatedChartData={isExtrapolatedChartData} + monitorType={monitorType} + activationCondition={activationCondition} + onMonitorTypeSelect={this.handleMonitorTypeSelect} /> {t('Set thresholds')} {thresholdTypeForm(formDisabled)} diff --git a/static/app/views/alerts/rules/metric/types.tsx b/static/app/views/alerts/rules/metric/types.tsx index b0a97a72b76ea2..b0bc7147fd9b64 100644 --- a/static/app/views/alerts/rules/metric/types.tsx +++ b/static/app/views/alerts/rules/metric/types.tsx @@ -1,5 +1,5 @@ import {t} from 'sentry/locale'; -import type {MonitorType} from 'sentry/types/alerts'; +import type {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import type {MEPAlertsQueryType} from 'sentry/views/alerts/wizard/options'; import type {SchemaFormConfig} from 'sentry/views/settings/organizationIntegrations/sentryAppExternalForm'; @@ -85,11 +85,7 @@ export type SavedTrigger = Omit & { export type Trigger = Partial & UnsavedTrigger; -export enum ActivationCondition { - RELEASE_CONDITION = 0, - DEPLOY_CONDITION = 1, -} - +// Form values for creating a new metric alert rule export type UnsavedMetricRule = { aggregate: string; dataset: Dataset; @@ -101,14 +97,16 @@ export type UnsavedMetricRule = { thresholdType: AlertRuleThresholdType; timeWindow: TimeWindow; triggers: Trigger[]; - activationCondition?: ActivationCondition; + activationCondition?: ActivationConditionType; comparisonDelta?: number | null; eventTypes?: EventTypes[]; monitorType?: MonitorType; + monitorWindow?: number | null; owner?: string | null; queryType?: MEPAlertsQueryType | null; }; +// Form values for updating a metric alert rule export interface SavedMetricRule extends UnsavedMetricRule { dateCreated: string; dateModified: string; diff --git a/static/app/views/releases/thresholdsList/thresholdGroupRows.tsx b/static/app/views/releases/thresholdsList/thresholdGroupRows.tsx index cf41c7be432b23..f8f8f1bd55a6ee 100644 --- a/static/app/views/releases/thresholdsList/thresholdGroupRows.tsx +++ b/static/app/views/releases/thresholdsList/thresholdGroupRows.tsx @@ -9,7 +9,7 @@ import Input from 'sentry/components/input'; import {IconAdd, IconClose, IconDelete, IconEdit} from 'sentry/icons'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import {MonitorType} from 'sentry/types/alerts'; +import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import type {Project} from 'sentry/types/project'; import {getExactDuration, parseLargestSuffix} from 'sentry/utils/formatters'; import {capitalize} from 'sentry/utils/string/capitalize'; @@ -17,7 +17,6 @@ import useApi from 'sentry/utils/useApi'; import useOrganization from 'sentry/utils/useOrganization'; import { // ActionType, - ActivationCondition, AlertRuleThresholdType, AlertRuleTriggerType, Dataset, @@ -196,6 +195,10 @@ export function ThresholdGroupRows({ ) => { const slug = project.slug; + const windowMinutes = + moment + .duration(thresholdData.windowValue, thresholdData.windowSuffix) + .as('seconds') / 60; /* Convert threshold data structure to metric alert data structure */ const metricAlertData: UnsavedMetricRule & {name: string} = { name: `Release Alert Rule for ${slug} in ${thresholdData.environmentName}`, @@ -208,7 +211,7 @@ export function ThresholdGroupRows({ resolveThreshold: null, thresholdPeriod: 1, thresholdType: AlertRuleThresholdType.ABOVE, - timeWindow: thresholdData.windowValue, + timeWindow: windowMinutes, triggers: [ { label: AlertRuleTriggerType.CRITICAL, @@ -221,7 +224,7 @@ export function ThresholdGroupRows({ eventTypes: [EventTypes.ERROR], owner: null, queryType: MEPAlertsQueryType.ERROR, - activationCondition: ActivationCondition.RELEASE_CONDITION, + activationCondition: ActivationConditionType.RELEASE_CREATION, }; let apiUrl = `/organizations/${organization.slug}/alert-rules/`;