Skip to content

Commit 5a5a2d0

Browse files
authored
[Alerting] Preserve rule type payload across delayed-to-active graduation (elastic#266012)
## Summary Resolves elastic#259886. Architectural alternative to elastic#265588. When a delayed alert is reactivated by `delayRecoveredFlappingAlerts` (flap-hold) and crosses `alertDelay` on a run where the executor does **not** report it, the alert builder used to dispatch to `buildNewAlert` with an empty payload — producing an active AAD doc with blank rule type fields (e.g. `kibana.alert.reason`). Rather than skip the graduation in that case, this PR makes the framework own the `delayed -> active` transition explicitly so the resulting active doc is always complete. ### Fix (code) - **`buildDelayedAlert`** now stores the full executor payload on the delayed AAD doc. Previously the delayed doc only carried framework fields. Persisting the rule type payload turns each delayed doc into a usable predecessor. - **`buildGraduatedAlert`** is a new builder dedicated to `delayed -> active` transitions. It deep-merges the predecessor delayed doc with the current run's payload (per-field precedence: current wins, predecessor fills gaps), sets `event.action: 'open'` and `kibana.alert.status: active`, and treats the alert as user-visible for the first time (`severity_improving: false`, no `previous_action_group`). - **`AlertBuilder.buildActiveAlerts`** now branches on `trackedActive` vs `trackedDelayed` to dispatch to ongoing / graduated / new respectively, instead of the previous status check on a single tracked alert. The per-field merge means: | Run shape on graduation | `cleanedPayload[K]` | Resulting field | | --- | --- | --- | | Executor reports `K` | present | fresh value (predecessor shadowed) | | Flap-hold reactivation, no executor report | absent | predecessor's value preserved | | Partial report (some `K` reported) | present for some | executor where present, predecessor where absent | This matches the long-standing semantics of `buildOngoingAlert`, just sourced from the delayed predecessor instead of an active one. How to reproduce the issue (on the 6th execution we see an alert without context): Run | pattern | flappingHistory | active | recovered | activeCount | pending recovered | flapping | AAD status -- | -- | -- | -- | -- | -- | -- | -- | -- 1 | a | T | x |   | 1 | 0 | FALSE | delayed 2 | a | T,F | x |   | 2 | 0 | FALSE | active 3 | - | T,F,T |   | x | 0 | - | FALSE | recovered 4 | - | T,F,T,F |   | x | 0 | - | FALSE | recovered 5 | a | T,F,T,F,T | x |   | 1 | 0 | FALSE | delayed 6 | - | T,F,T,F,T,T | x |   | 2 | 1 | TRUE | active
1 parent f737970 commit 5a5a2d0

9 files changed

Lines changed: 940 additions & 39 deletions

File tree

x-pack/platform/plugins/shared/alerting/server/alerts_client/lib/alert_builder/alert_builder.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import type { IIndexPatternString } from '../../../alerts_service/resource_insta
2727
import type { TrackedAADAlerts } from '../../types';
2828
import { buildOngoingAlert } from './build_ongoing_alert';
2929
import { buildNewAlert } from './build_new_alert';
30+
import { buildGraduatedAlert } from './build_graduated_alert';
3031
import { buildRecoveredAlert } from './build_recovered_alert';
3132
import { buildUpdatedRecoveredAlert } from './build_updated_recovered_alert';
3233
import { buildDelayedAlert } from './build_delayed_alert';
@@ -184,18 +185,21 @@ export class AlertBuilder<
184185
continue;
185186
}
186187
if (activeAlert) {
187-
const trackedAlert = this.trackedAlerts.get(activeAlert.getUuid());
188-
if (!!trackedAlert && get(trackedAlert, ALERT_STATUS) === ALERT_STATUS_ACTIVE) {
188+
const uuid = activeAlert.getUuid();
189+
const trackedActive = this.trackedAlerts.active[uuid];
190+
const trackedDelayed = this.trackedAlerts.delayed[uuid];
191+
192+
if (trackedActive) {
189193
const isImproving = isAlertImproving<
190194
AlertData,
191195
State,
192196
Context,
193197
ActionGroupIds,
194198
RecoveryActionGroupId
195-
>(trackedAlert, activeAlert, this.ruleType.actionGroups);
199+
>(trackedActive, activeAlert, this.ruleType.actionGroups);
196200
activeAlertsToIndex.push(
197201
buildOngoingAlert<AlertData, State, Context, ActionGroupIds, RecoveryActionGroupId>({
198-
alert: trackedAlert,
202+
alert: trackedActive,
199203
legacyAlert: activeAlert,
200204
rule: this.rule,
201205
ruleData: this.alertRuleData,
@@ -207,6 +211,26 @@ export class AlertBuilder<
207211
dangerouslyCreateAlertsInAllSpaces: this.createAlertsInAllSpaces,
208212
})
209213
);
214+
} else if (trackedDelayed) {
215+
// The alert is graduating from `delayed` to `active`. The delayed
216+
// predecessor doc carries the rule type fields reported during the
217+
// delayed runs, which the graduated builder merges in. This keeps
218+
// the resulting active doc complete even when the executor does
219+
// not report a fresh payload on the run that triggers graduation
220+
// (e.g. flap-hold reactivation).
221+
activeAlertsToIndex.push(
222+
buildGraduatedAlert<AlertData, State, Context, ActionGroupIds, RecoveryActionGroupId>({
223+
alert: trackedDelayed,
224+
legacyAlert: activeAlert,
225+
rule: this.rule,
226+
ruleData: this.alertRuleData,
227+
runTimestamp: this.runTimestampString,
228+
timestamp: this.currentTime,
229+
payload: this.reportedAlerts[id],
230+
kibanaVersion: this.kibanaVersion,
231+
dangerouslyCreateAlertsInAllSpaces: this.createAlertsInAllSpaces,
232+
})
233+
);
210234
} else {
211235
activeAlertsToIndex.push(
212236
buildNewAlert<AlertData, State, Context, ActionGroupIds, RecoveryActionGroupId>({
@@ -277,8 +301,13 @@ export class AlertBuilder<
277301
delayedAlertsToIndex.push(
278302
buildDelayedAlert<AlertData, State, Context, ActionGroupIds, RecoveryActionGroupId>({
279303
legacyAlert: delayedAlert,
280-
timestamp: this.currentTime,
281304
rule: this.rule,
305+
ruleData: this.alertRuleData,
306+
payload: this.reportedAlerts[delayedAlert.getId()],
307+
runTimestamp: this.runTimestampString,
308+
timestamp: this.currentTime,
309+
kibanaVersion: this.kibanaVersion,
310+
dangerouslyCreateAlertsInAllSpaces: this.createAlertsInAllSpaces,
282311
})
283312
);
284313
}

x-pack/platform/plugins/shared/alerting/server/alerts_client/lib/alert_builder/build_delayed_alert.test.ts

Lines changed: 130 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,36 @@
77
import { Alert as LegacyAlert } from '../../../alert/alert';
88
import {
99
ALERT_ACTION_GROUP,
10+
ALERT_CONSECUTIVE_MATCHES,
11+
ALERT_DURATION,
12+
ALERT_FLAPPING,
13+
ALERT_FLAPPING_HISTORY,
1014
ALERT_INSTANCE_ID,
15+
ALERT_MAINTENANCE_WINDOW_IDS,
16+
ALERT_MAINTENANCE_WINDOW_NAMES,
17+
ALERT_MUTED,
18+
ALERT_PENDING_RECOVERED_COUNT,
19+
ALERT_RULE_EXECUTION_TIMESTAMP,
20+
ALERT_RULE_EXECUTION_UUID,
21+
ALERT_RULE_UUID,
22+
ALERT_START,
1123
ALERT_STATUS,
24+
ALERT_STATUS_DELAYED,
25+
ALERT_TIME_RANGE,
1226
ALERT_UUID,
27+
ALERT_WORKFLOW_STATUS,
28+
EVENT_KIND,
29+
SPACE_IDS,
30+
TAGS,
1331
TIMESTAMP,
14-
ALERT_CONSECUTIVE_MATCHES,
15-
ALERT_RULE_UUID,
16-
ALERT_RULE_EXECUTION_UUID,
17-
ALERT_STATUS_DELAYED,
32+
VERSION,
1833
} from '@kbn/rule-data-utils';
1934
import { alertRule } from '../test_fixtures';
2035
import { buildDelayedAlert } from './build_delayed_alert';
2136
import { get } from 'lodash';
2237

2338
describe('buildDelayedAlert', () => {
24-
test('should build alert document with info from legacy alert', () => {
39+
test('should build alert document with framework fields from legacy alert', () => {
2540
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
2641
legacyAlert.scheduleActions('default');
2742

@@ -31,18 +46,124 @@ describe('buildDelayedAlert', () => {
3146
expect(
3247
buildDelayedAlert<{}, {}, {}, 'default', 'recovered'>({
3348
legacyAlert,
34-
timestamp,
3549
rule: alertRule,
50+
timestamp,
51+
kibanaVersion: '8.9.0',
3652
})
3753
).toEqual({
38-
[ALERT_UUID]: legacyAlert.getUuid(),
54+
...alertRule,
55+
[TIMESTAMP]: timestamp,
56+
[EVENT_KIND]: 'signal',
57+
[ALERT_RULE_EXECUTION_TIMESTAMP]: timestamp,
3958
[ALERT_RULE_UUID]: get(alertRule, ALERT_RULE_UUID),
4059
[ALERT_RULE_EXECUTION_UUID]: get(alertRule, ALERT_RULE_EXECUTION_UUID),
41-
[TIMESTAMP]: timestamp,
4260
[ALERT_ACTION_GROUP]: legacyAlert.getScheduledActionOptions()?.actionGroup,
61+
[ALERT_FLAPPING]: false,
62+
[ALERT_FLAPPING_HISTORY]: [],
4363
[ALERT_INSTANCE_ID]: alertInstanceId,
44-
[ALERT_CONSECUTIVE_MATCHES]: legacyAlert.getActiveCount(),
64+
[ALERT_MAINTENANCE_WINDOW_IDS]: [],
65+
[ALERT_MAINTENANCE_WINDOW_NAMES]: [],
66+
[ALERT_CONSECUTIVE_MATCHES]: 0,
67+
[ALERT_PENDING_RECOVERED_COUNT]: 0,
68+
[ALERT_MUTED]: false,
69+
[ALERT_STATUS]: ALERT_STATUS_DELAYED,
70+
[ALERT_UUID]: legacyAlert.getUuid(),
71+
[ALERT_WORKFLOW_STATUS]: 'open',
72+
[SPACE_IDS]: ['default'],
73+
[VERSION]: '8.9.0',
74+
[TAGS]: ['rule-', '-tags'],
75+
});
76+
});
77+
78+
test('should include start, duration and time range when set', () => {
79+
const now = Date.now();
80+
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
81+
legacyAlert.scheduleActions('default').replaceState({ start: now, duration: '0' });
82+
83+
const built = buildDelayedAlert<{}, {}, {}, 'default', 'recovered'>({
84+
legacyAlert,
85+
rule: alertRule,
86+
timestamp: '2023-03-28T12:27:28.159Z',
87+
kibanaVersion: '8.9.0',
88+
});
89+
90+
expect(built).toMatchObject({
91+
[ALERT_DURATION]: 0,
92+
[ALERT_START]: now,
93+
[ALERT_TIME_RANGE]: { gte: now },
94+
[ALERT_STATUS]: ALERT_STATUS_DELAYED,
95+
});
96+
});
97+
98+
test('should include rule type payload fields so the doc is a complete predecessor', () => {
99+
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
100+
legacyAlert.scheduleActions('default');
101+
102+
const built = buildDelayedAlert<
103+
{ count: number; url: string; 'kibana.alert.nested_field': number },
104+
{},
105+
{},
106+
'default',
107+
'recovered'
108+
>({
109+
legacyAlert,
110+
rule: alertRule,
111+
payload: { count: 1, url: `https://url1`, 'kibana.alert.nested_field': 2 },
112+
timestamp: '2023-03-28T12:27:28.159Z',
113+
kibanaVersion: '8.9.0',
114+
});
115+
116+
expect(built).toMatchObject({
117+
count: 1,
118+
url: `https://url1`,
119+
'kibana.alert.nested_field': 2,
45120
[ALERT_STATUS]: ALERT_STATUS_DELAYED,
46121
});
47122
});
123+
124+
test('should set ALERT_WORKFLOW_STATUS from payload if specified', () => {
125+
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
126+
legacyAlert.scheduleActions('default');
127+
128+
const built = buildDelayedAlert<{}, {}, {}, 'default', 'recovered'>({
129+
legacyAlert,
130+
rule: alertRule,
131+
payload: { [ALERT_WORKFLOW_STATUS]: 'custom_workflow' },
132+
timestamp: '2023-03-28T12:27:28.159Z',
133+
kibanaVersion: '8.9.0',
134+
});
135+
136+
expect(built[ALERT_WORKFLOW_STATUS]).toBe('custom_workflow');
137+
});
138+
139+
test(`should set kibana.space_ids to '*' if dangerouslyCreateAlertsInAllSpaces=true`, () => {
140+
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
141+
legacyAlert.scheduleActions('default');
142+
143+
const built = buildDelayedAlert<{}, {}, {}, 'default', 'recovered'>({
144+
legacyAlert,
145+
rule: alertRule,
146+
timestamp: '2023-03-28T12:27:28.159Z',
147+
kibanaVersion: '8.9.0',
148+
dangerouslyCreateAlertsInAllSpaces: true,
149+
});
150+
151+
expect(built[SPACE_IDS]).toEqual(['*']);
152+
});
153+
154+
test('should merge tags from payload, rule, and de-duplicate', () => {
155+
const legacyAlert = new LegacyAlert<{}, {}, 'default'>('alert-A');
156+
legacyAlert.scheduleActions('default');
157+
158+
const built = buildDelayedAlert<{}, {}, {}, 'default', 'recovered'>({
159+
legacyAlert,
160+
rule: alertRule,
161+
payload: { tags: ['payload-tag', 'rule-'] },
162+
timestamp: '2023-03-28T12:27:28.159Z',
163+
kibanaVersion: '8.9.0',
164+
});
165+
166+
expect(built[TAGS]).toEqual(expect.arrayContaining(['payload-tag', 'rule-', '-tags']));
167+
expect(built[TAGS]?.length).toBe(3);
168+
});
48169
});

0 commit comments

Comments
 (0)