[Alert Summaries] [BE] Move “Notify When” and throttle from rule to action #144130
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import Boom from '@hapi/boom'; | ||
| import { RuleTypeParams, RuleTypeParamsValidator } from '../types'; | ||
|
|
||
| export function validateRuleTypeParams<Params extends RuleTypeParams>( | ||
| params: Record<string, unknown>, | ||
| validator?: RuleTypeParamsValidator<Params> | ||
| ): Params { | ||
| if (!validator) { | ||
| return params as Params; | ||
| } | ||
|
|
||
| try { | ||
| return validator.validate(params); | ||
| } catch (err) { | ||
| throw Boom.badRequest(`params invalid: ${err.message}`); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { schema } from '@kbn/config-schema'; | ||
| import { validateDurationSchema } from '../../lib'; | ||
|
|
||
| export const actionsSchema = schema.arrayOf( | ||
| schema.object({ | ||
| group: schema.string(), | ||
| id: schema.string(), | ||
| params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }), | ||
| frequency: schema.maybe( | ||
| schema.object({ | ||
| summary: schema.boolean(), | ||
| notify_when: schema.oneOf([ | ||
| schema.literal('onActionGroupChange'), | ||
| schema.literal('onActiveAlert'), | ||
| schema.literal('onThrottleInterval'), | ||
| ]), | ||
| throttle: schema.nullable(schema.string({ validate: validateDurationSchema })), | ||
| }) | ||
| ), | ||
| }), | ||
| { defaultValue: [] } | ||
| ); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
| import { CamelToSnake, RewriteRequestCase } from './rewrite_request_case'; | ||
| import { RuleAction } from '../../types'; | ||
|
|
||
| type ReqRuleAction = Omit<RuleAction, 'actionTypeId' | 'frequency'> & { | ||
| frequency?: { | ||
| [K in keyof NonNullable<RuleAction['frequency']> as CamelToSnake<K>]: NonNullable< | ||
| RuleAction['frequency'] | ||
| >[K]; | ||
| }; | ||
| }; | ||
| export const rewriteActions: ( | ||
| actions?: ReqRuleAction[] | ||
| ) => Array<Omit<RuleAction, 'actionTypeId'>> = (actions) => { | ||
| const rewriteFrequency: RewriteRequestCase<NonNullable<RuleAction['frequency']>> = ({ | ||
| notify_when: notifyWhen, | ||
| ...rest | ||
| }) => ({ ...rest, notifyWhen }); | ||
| if (!actions) return []; | ||
| return actions.map( | ||
| (action) => | ||
| ({ | ||
| ...action, | ||
| ...(action.frequency ? { frequency: rewriteFrequency(action.frequency) } : {}), | ||
| } as RuleAction) | ||
| ); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -387,8 +387,8 @@ export interface UpdateOptions<Params extends RuleTypeParams> { | |
| schedule: IntervalSchedule; | ||
| actions: NormalizedAlertAction[]; | ||
| params: Params; | ||
| throttle?: string | null; | ||
| notifyWhen?: RuleNotifyWhenType | null; | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -574,7 +574,9 @@ export class RulesClient { | |
| throw Boom.badRequest(`Error creating rule: could not create API key - ${error.message}`); | ||
| } | ||
|
|
||
| const usesGlobalFreqParams = | ||
| typeof data.notifyWhen !== 'undefined' && typeof data.throttle !== 'undefined'; | ||
| await this.validateActions(ruleType, data.actions, usesGlobalFreqParams); | ||
|
|
||
| // Throw error if schedule interval is less than the minimum and we are enforcing it | ||
| const intervalInMs = parseDuration(data.schedule.interval); | ||
|
|
@@ -593,7 +595,8 @@ export class RulesClient { | |
|
|
||
| const createTime = Date.now(); | ||
| const legacyId = Semver.lt(this.kibanaVersion, '8.0.0') ? id : null; | ||
| const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); | ||
| const throttle = data.throttle ?? null; | ||
|
|
||
| const rawRule: RawRule = { | ||
| ...data, | ||
|
|
@@ -609,6 +612,7 @@ export class RulesClient { | |
| muteAll: false, | ||
| mutedInstanceIds: [], | ||
| notifyWhen, | ||
| throttle, | ||
| executionStatus: getRuleExecutionStatusPending(new Date().toISOString()), | ||
| monitoring: getDefaultRuleMonitoring(), | ||
| }; | ||
|
|
@@ -1711,7 +1715,9 @@ export class RulesClient { | |
|
|
||
| // Validate | ||
| const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params); | ||
| const usesGlobalFreqParams = | ||
| typeof data.notifyWhen !== 'undefined' && typeof data.throttle !== 'undefined'; | ||
| await this.validateActions(ruleType, data.actions, usesGlobalFreqParams); | ||
|
There was a problem hiding this comment. What if only one of them - After reading requirements in #143368 this is my assumption about valid and invalid inputs:
Is this assumption correct? There was a problem hiding this comment. Pushed a fix for this that will invalidate an XOR situation with There was a problem hiding this comment. @Zacqary Awesome, it looks correct to me after the fix. As a nit, I'd suggest adding this as a comment to the // check for actions using frequency params if the rule has global frequency params defined
// - Valid inputs
// 1. Old way, rule-level parameters: `rule.notifyWhen` !== undefined, `rule.throttle` !== undefined, all actions in a rule do not contain `frequency`
// 2. New way, action-level parameters: `rule.notifyWhen` === undefined, `rule.throttle` === undefined, all actions in a rule contain some `frequency` object
// - Invalid inputs: all the other combinations of the above parameters
// 1. `rule.notifyWhen` !== undefined, `rule.throttle` !== undefined, at least one action in a rule contains `frequency`
// 2. `rule.notifyWhen` === undefined, `rule.throttle` === undefined, at least one action in a rule doesn't contain `frequency`
// 3. Either of `rule.notifyWhen` and `rule.throttle` is undefined
if (usesGlobalFreqParams) {
const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency));
if (actionsWithFrequency.length) { |
||
|
|
||
| // Throw error if schedule interval is less than the minimum and we are enforcing it | ||
| const intervalInMs = parseDuration(data.schedule.interval); | ||
|
|
@@ -1740,7 +1746,7 @@ export class RulesClient { | |
| } | ||
|
|
||
| const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username); | ||
| const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); | ||
|
|
||
| let updatedObject: SavedObject<RawRule>; | ||
| const createAttributes = this.updateMeta({ | ||
|
|
@@ -2232,7 +2238,11 @@ export class RulesClient { | |
| for (const operation of operations) { | ||
| switch (operation.field) { | ||
| case 'actions': | ||
| const usesGlobalFreqParams = Boolean( | ||
| typeof attributes.notifyWhen !== 'undefined' && | ||
| typeof attributes.throttle !== 'undefined' | ||
| ); | ||
| await this.validateActions(ruleType, operation.value, usesGlobalFreqParams); | ||
| ruleActions = applyBulkEditOperation(operation, ruleActions); | ||
| break; | ||
| case 'snoozeSchedule': | ||
|
|
@@ -2342,7 +2352,7 @@ export class RulesClient { | |
|
|
||
| // get notifyWhen | ||
| const notifyWhen = getRuleNotifyWhenType( | ||
| attributes.notifyWhen ?? null, | ||
| attributes.throttle ?? null | ||
| ); | ||
|
|
||
|
|
@@ -3504,7 +3514,8 @@ export class RulesClient { | |
|
|
||
| private async validateActions( | ||
| alertType: UntypedNormalizedRuleType, | ||
| actions: NormalizedAlertAction[], | ||
| usesGlobalFreqParams: boolean | ||
| ): Promise<void> { | ||
| if (actions.length === 0) { | ||
| return; | ||
|
|
@@ -3548,6 +3559,34 @@ export class RulesClient { | |
| }) | ||
| ); | ||
| } | ||
|
|
||
| // check for actions using frequency params if the rule has global frequency params defined | ||
| if (usesGlobalFreqParams) { | ||
| const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency)); | ||
| if (actionsWithFrequency.length) { | ||
Zacqary marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| throw Boom.badRequest( | ||
| i18n.translate('xpack.alerting.rulesClient.validateActions.mixAndMatchFreqParams', { | ||
| defaultMessage: | ||
| 'Cannot mix and match per-action frequency params when notify_when or throttle are globally defined: {groups}', | ||
banderror marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| values: { | ||
| groups: actionsWithFrequency.map((a) => a.group).join(', '), | ||
| }, | ||
| }) | ||
| ); | ||
| } | ||
| } else { | ||
| const actionsWithoutFrequency = actions.filter((action) => !action.frequency); | ||
| if (actionsWithoutFrequency.length) { | ||
XavierM marked this conversation as resolved.
Show resolved
Hide resolved
XavierM marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| throw Boom.badRequest( | ||
| i18n.translate('xpack.alerting.rulesClient.validateActions.notAllActionsWithFreq', { | ||
| defaultMessage: 'Actions missing frequency parameters: {groups}', | ||
| values: { | ||
| groups: actionsWithoutFrequency.map((a) => a.group).join(', '), | ||
| }, | ||
| }) | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private async extractReferences< | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like total copy of
x-pack/plugins/alerting/server/lib/validate_rule_type_params.ts. With the same variables, types.Maybe better to reuse it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah looks like I committed this file by mistake