-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Alerting V2] [ResponseOps] Recovery delay is configurable and persisted even when it has no effect on execution #288802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
cb02bad
bce43cc
a5aa772
00f662b
9b736cc
06a7cd9
57e01b6
d5f8df3
1eff571
d34739c
a3eeb71
7a88ad9
f26d7c5
350a627
e95a043
c6ea536
684bc24
11eae65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ import { | |
| deriveAlertDelayModeFromStateTransition, | ||
| deriveRecoveryDelayModeFromStateTransition, | ||
| } from '../../form/utils/state_transition_helpers'; | ||
| import { resolveRecoveryStrategy } from '../../form/utils/rule_request_mappers'; | ||
| import { isRecoveryEnabled, resolveRecoveryStrategy } from '../../form/utils/rule_request_mappers'; | ||
| import type { FormValues } from '../../form/types'; | ||
|
|
||
| const DELAY_IMMEDIATE = 'immediate'; | ||
|
|
@@ -42,15 +42,19 @@ const mapStateTransition = (formValues: FormValues) => { | |
| if (stateTransition?.pendingCount != null) out.pending_count = stateTransition.pendingCount; | ||
| } | ||
|
|
||
| if (recoveryMode === DELAY_IMMEDIATE) { | ||
| out.recovering_count = 0; | ||
| } else if (recoveryMode !== DELAY_DURATION && stateTransition?.recoveringCount != null) { | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } else if (recoveryMode === DELAY_DURATION) { | ||
| if (stateTransition?.recoveringTimeframe != null) | ||
| out.recovering_timeframe = stateTransition.recoveringTimeframe; | ||
| if (stateTransition?.recoveringCount != null) | ||
| // Recovering thresholds are only meaningful when recovery is enabled; emitting them | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: self explanatory
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed in this commit, e95a043 |
||
| // while recovery is disabled is inert and rejected by the write API. | ||
| if (isRecoveryEnabled(formValues)) { | ||
| if (recoveryMode === DELAY_IMMEDIATE) { | ||
| out.recovering_count = 0; | ||
| } else if (recoveryMode !== DELAY_DURATION && stateTransition?.recoveringCount != null) { | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } else if (recoveryMode === DELAY_DURATION) { | ||
| if (stateTransition?.recoveringTimeframe != null) | ||
| out.recovering_timeframe = stateTransition.recoveringTimeframe; | ||
| if (stateTransition?.recoveringCount != null) | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } | ||
| } | ||
|
|
||
| return Object.keys(out).length ? out : undefined; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,21 @@ export const resolveRecoveryStrategy = ( | |
| return formValues.query.recovery != null ? ('query' as const) : undefined; | ||
| }; | ||
|
|
||
| /** | ||
| * Recovery is enabled only for condition-based strategies (`no_breach` / `query`). | ||
| * `none`/unset means condition-based recovery never fires, so the recovering delay | ||
| * thresholds (`recovering_count` / `recovering_timeframe`) are inert and must not be | ||
| * emitted — the write API rejects them (see `isRecoveryTransitionConsistentWithStrategy`). | ||
| * `no_data_strategy: 'recover'` does NOT re-enable them: the director bypasses recovering | ||
| * gating for no-data recovery (see `count_timeframe_strategy`). | ||
| */ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably not needed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed in this commit, e95a043 |
||
| export const isRecoveryEnabled = ( | ||
| formValues: Pick<FormValues, 'kind' | 'recoveryStrategy' | 'query'> | ||
| ): boolean => { | ||
| const strategy = resolveRecoveryStrategy(formValues); | ||
| return strategy != null && strategy !== 'none'; | ||
| }; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // FormValues → API request | ||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -95,16 +110,20 @@ const mapStateTransition = (formValues: FormValues) => { | |
| } | ||
| } | ||
|
|
||
| if (recoveryMode === DELAY_MODE.immediate) { | ||
| out.recovering_count = 0; | ||
| } else if (recoveryMode !== DELAY_MODE.duration && stateTransition?.recoveringCount != null) { | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } else if (recoveryMode === DELAY_MODE.duration) { | ||
| if (stateTransition?.recoveringTimeframe != null) { | ||
| out.recovering_timeframe = stateTransition.recoveringTimeframe; | ||
| } | ||
| if (stateTransition?.recoveringCount != null) { | ||
| // Recovering thresholds are only meaningful when recovery is enabled; emitting them | ||
| // while recovery is disabled is inert and rejected by the write API. | ||
| if (isRecoveryEnabled(formValues)) { | ||
| if (recoveryMode === DELAY_MODE.immediate) { | ||
| out.recovering_count = 0; | ||
| } else if (recoveryMode !== DELAY_MODE.duration && stateTransition?.recoveringCount != null) { | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } else if (recoveryMode === DELAY_MODE.duration) { | ||
| if (stateTransition?.recoveringTimeframe != null) { | ||
| out.recovering_timeframe = stateTransition.recoveringTimeframe; | ||
| } | ||
| if (stateTransition?.recoveringCount != null) { | ||
| out.recovering_count = stateTransition.recoveringCount; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
nit: self-explanatory, comment not needed
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.
removed in this commit, e95a043