Skip to content

Commit c4a66c9

Browse files
kibanamachinedoakalexi
kibanamachine
and
doakalexi
authored
[9.1] [Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1 (#242024) (#242457)
# Backport This will backport the following commits from `main` to `9.1`: - [[Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1 (#242024)](#242024) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Alexi Doak","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-11-10T17:21:27Z","message":"[Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1 (#242024)\n\nResolves https://github.com/elastic/kibana/issues/239852\n\n## Summary\n\nThis PR fixes a bug where active alerts aren't created when flapping is\ndisabled and the alert delay feature is used. This bug was happening bc\nthe flapping code created clones of active and recovered alert objects\nfor tracking in the state. When flapping is disabled this code is\nskipped, so when the active and recovered alerts are deleted from their\nobjects they are also deleted in the objects tracking active and\nrecovered alerts. This PR clones the active and recovered objects before\nany flapping or alert delay processing to fix this bug.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n### To verify\n\n1. Set flapping to be disabled in the rule settings\n2. Create an alert and set the alert delay to be 2\n3. Verify that the alert delay feature delays the alert as expected and\nthe alert actually gets created.","sha":"5d2171a9e9706a00908d1c7e594b575a6179159f","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","backport:version","v9.1.0","v8.19.0","v9.2.0","v9.3.0"],"title":"[Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1","number":242024,"url":"https://github.com/elastic/kibana/pull/242024","mergeCommit":{"message":"[Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1 (#242024)\n\nResolves https://github.com/elastic/kibana/issues/239852\n\n## Summary\n\nThis PR fixes a bug where active alerts aren't created when flapping is\ndisabled and the alert delay feature is used. This bug was happening bc\nthe flapping code created clones of active and recovered alert objects\nfor tracking in the state. When flapping is disabled this code is\nskipped, so when the active and recovered alerts are deleted from their\nobjects they are also deleted in the objects tracking active and\nrecovered alerts. This PR clones the active and recovered objects before\nany flapping or alert delay processing to fix this bug.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n### To verify\n\n1. Set flapping to be disabled in the rule settings\n2. Create an alert and set the alert delay to be 2\n3. Verify that the alert delay feature delays the alert as expected and\nthe alert actually gets created.","sha":"5d2171a9e9706a00908d1c7e594b575a6179159f"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19","9.2"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.2","label":"v9.2.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/242024","number":242024,"mergeCommit":{"message":"[Alerting] [Bug] Turning flapping off prevents alerts to be generated when alert delay is set to more than 1 (#242024)\n\nResolves https://github.com/elastic/kibana/issues/239852\n\n## Summary\n\nThis PR fixes a bug where active alerts aren't created when flapping is\ndisabled and the alert delay feature is used. This bug was happening bc\nthe flapping code created clones of active and recovered alert objects\nfor tracking in the state. When flapping is disabled this code is\nskipped, so when the active and recovered alerts are deleted from their\nobjects they are also deleted in the objects tracking active and\nrecovered alerts. This PR clones the active and recovered objects before\nany flapping or alert delay processing to fix this bug.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n### To verify\n\n1. Set flapping to be disabled in the rule settings\n2. Create an alert and set the alert delay to be 2\n3. Verify that the alert delay feature delays the alert as expected and\nthe alert actually gets created.","sha":"5d2171a9e9706a00908d1c7e594b575a6179159f"}}]}] BACKPORT--> Co-authored-by: Alexi Doak <[email protected]>
1 parent c2fb827 commit c4a66c9

File tree

2 files changed

+312
-3
lines changed

2 files changed

+312
-3
lines changed

‎x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* 2.0.
66
*/
77
import type { KibanaRequest, Logger } from '@kbn/core/server';
8-
import { cloneDeep, keys } from 'lodash';
8+
import { cloneDeep, clone, keys } from 'lodash';
99
import { Alert } from '../alert/alert';
1010
import type { AlertFactory } from '../alert/create_alert_factory';
1111
import { createAlertFactory, getPublicAlertFactory } from '../alert/create_alert_factory';
@@ -185,9 +185,9 @@ export class LegacyAlertsClient<
185185

186186
this.processedAlerts.new = processedAlertsNew;
187187
this.processedAlerts.active = processedAlertsActive;
188-
this.processedAlerts.trackedActiveAlerts = processedAlertsActive;
188+
this.processedAlerts.trackedActiveAlerts = clone(processedAlertsActive);
189189
this.processedAlerts.recovered = processedAlertsRecovered;
190-
this.processedAlerts.trackedRecoveredAlerts = processedAlertsRecovered;
190+
this.processedAlerts.trackedRecoveredAlerts = clone(processedAlertsRecovered);
191191
}
192192

193193
public logAlerts({ ruleRunMetricsStore, shouldLogAlerts }: LogAlertsOpts) {

‎x-pack/platform/test/alerting_api_integration/spaces_only/tests/alerting/group4/alerts_as_data/alerts_as_data_alert_delay.ts‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
getTestRuleData,
4444
getUrlPrefix,
4545
ObjectRemover,
46+
resetRulesSettings,
4647
} from '../../../../../common/lib';
4748

4849
export default function createAlertsAsDataAlertDelayInstallResourcesTest({
@@ -89,6 +90,7 @@ export default function createAlertsAsDataAlertDelayInstallResourcesTest({
8990
conflicts: 'proceed',
9091
ignore_unavailable: true,
9192
});
93+
await resetRulesSettings(supertestWithoutAuth, Spaces.space1.id);
9294
});
9395
after(async () => {
9496
await esTestIndexTool.destroy();
@@ -527,6 +529,313 @@ export default function createAlertsAsDataAlertDelayInstallResourcesTest({
527529
// After the third run, we should have 0 alert docs for the 0 active alerts
528530
expect(alertDocsRun3.length).to.equal(0);
529531
});
532+
533+
it('should generate expected events with a alertDelay with AAD when flapping is disabled', async () => {
534+
await supertestWithoutAuth
535+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/settings/_flapping`)
536+
.set('kbn-xsrf', 'foo')
537+
.send({
538+
enabled: false,
539+
look_back_window: 20,
540+
status_change_threshold: 4,
541+
})
542+
.expect(200);
543+
const { body: createdAction } = await supertestWithoutAuth
544+
.post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`)
545+
.set('kbn-xsrf', 'foo')
546+
.send({
547+
name: 'MY action',
548+
connector_type_id: 'test.noop',
549+
config: {},
550+
secrets: {},
551+
})
552+
.expect(200);
553+
554+
// pattern of when the alert should fire
555+
const pattern = {
556+
instance: [true, true, true, true, false, true],
557+
};
558+
559+
const response = await supertestWithoutAuth
560+
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
561+
.set('kbn-xsrf', 'foo')
562+
.send(
563+
getTestRuleData({
564+
rule_type_id: 'test.patternFiringAad',
565+
schedule: { interval: '1d' },
566+
throttle: null,
567+
notify_when: null,
568+
params: {
569+
pattern,
570+
},
571+
actions: [
572+
{
573+
id: createdAction.id,
574+
group: 'default',
575+
params: {},
576+
frequency: {
577+
summary: false,
578+
throttle: null,
579+
notify_when: RuleNotifyWhen.CHANGE,
580+
},
581+
},
582+
],
583+
alert_delay: {
584+
active: 3,
585+
},
586+
})
587+
);
588+
589+
expect(response.status).to.eql(200);
590+
const ruleId = response.body.id;
591+
objectRemover.add(Spaces.space1.id, ruleId, 'rule', 'alerting');
592+
593+
// --------------------------
594+
// RUN 1 - 0 new alerts
595+
// --------------------------
596+
let events: IValidatedEvent[] = await waitForEventLogDocs(
597+
ruleId,
598+
new Map([['execute', { equal: 1 }]])
599+
);
600+
let executeEvent = events[0];
601+
expect(get(executeEvent, ACTIVE_PATH)).to.be(0);
602+
expect(get(executeEvent, NEW_PATH)).to.be(0);
603+
expect(get(executeEvent, RECOVERED_PATH)).to.be(0);
604+
expect(get(executeEvent, ACTION_PATH)).to.be(0);
605+
expect(get(executeEvent, DELAYED_PATH)).to.be(1);
606+
607+
// Query for alerts
608+
const alertDocsRun1 = await queryForAlertDocs<PatternFiringAlert>();
609+
610+
// Get alert state from task document
611+
let state: any = await getTaskState(ruleId);
612+
expect(state.alertInstances.instance.meta.activeCount).to.equal(1);
613+
expect(state.alertInstances.instance.state.patternIndex).to.equal(0);
614+
615+
// After the first run, we should have 0 alert docs for the 0 active alerts
616+
expect(alertDocsRun1.length).to.equal(0);
617+
618+
// --------------------------
619+
// RUN 2 - 0 new alerts
620+
// --------------------------
621+
let runSoon = await supertestWithoutAuth
622+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${ruleId}/_run_soon`)
623+
.set('kbn-xsrf', 'foo');
624+
expect(runSoon.status).to.eql(204);
625+
626+
events = await waitForEventLogDocs(ruleId, new Map([['execute', { equal: 2 }]]));
627+
executeEvent = events[1];
628+
expect(get(executeEvent, ACTIVE_PATH)).to.be(0);
629+
expect(get(executeEvent, NEW_PATH)).to.be(0);
630+
expect(get(executeEvent, RECOVERED_PATH)).to.be(0);
631+
expect(get(executeEvent, ACTION_PATH)).to.be(0);
632+
expect(get(executeEvent, DELAYED_PATH)).to.be(1);
633+
634+
// Query for alerts
635+
const alertDocsRun2 = await queryForAlertDocs<PatternFiringAlert>();
636+
637+
// Get alert state from task document
638+
state = await getTaskState(ruleId);
639+
expect(state.alertInstances.instance.meta.activeCount).to.equal(2);
640+
expect(state.alertInstances.instance.state.patternIndex).to.equal(1);
641+
642+
// After the second run, we should have 0 alert docs for the 0 active alerts
643+
expect(alertDocsRun2.length).to.equal(0);
644+
645+
// --------------------------
646+
// RUN 3 - 1 new alert
647+
// --------------------------
648+
runSoon = await supertestWithoutAuth
649+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${ruleId}/_run_soon`)
650+
.set('kbn-xsrf', 'foo');
651+
expect(runSoon.status).to.eql(204);
652+
653+
events = await waitForEventLogDocs(ruleId, new Map([['execute', { equal: 3 }]]));
654+
executeEvent = events[2];
655+
let executionUuid = get(executeEvent, UUID_PATH);
656+
expect(get(executeEvent, ACTIVE_PATH)).to.be(1);
657+
expect(get(executeEvent, NEW_PATH)).to.be(1);
658+
expect(get(executeEvent, RECOVERED_PATH)).to.be(0);
659+
expect(get(executeEvent, ACTION_PATH)).to.be(1);
660+
expect(get(executeEvent, DELAYED_PATH)).to.be(0);
661+
662+
// Query for alerts
663+
const alertDocsRun3 = await queryForAlertDocs<PatternFiringAlert>();
664+
665+
// Get alert state from task document
666+
state = await getTaskState(ruleId);
667+
expect(state.alertInstances.instance.meta.activeCount).to.equal(3);
668+
expect(state.alertInstances.instance.state.patternIndex).to.equal(2);
669+
670+
// After the third run, we should have 1 alert docs for the 1 active alert
671+
expect(alertDocsRun3.length).to.equal(1);
672+
673+
testExpectRuleData(alertDocsRun3, ruleId, { pattern }, executionUuid!);
674+
let source: PatternFiringAlert = alertDocsRun3[0]._source!;
675+
676+
// Each doc should have active status and default action group id
677+
expect(source[ALERT_ACTION_GROUP]).to.equal('default');
678+
// patternIndex should be 2 for the third run
679+
expect(source.patternIndex).to.equal(2);
680+
// alert UUID should equal doc id
681+
expect(source[ALERT_UUID]).to.equal(alertDocsRun3[0]._id);
682+
// duration should be 0 since this is a new alert
683+
expect(source[ALERT_DURATION]).to.equal(0);
684+
// start should be defined
685+
expect(source[ALERT_START]).to.match(timestampPattern);
686+
// time_range.gte should be same as start
687+
expect(source[ALERT_TIME_RANGE]?.gte).to.equal(source[ALERT_START]);
688+
// timestamp should be defined
689+
expect(source['@timestamp']).to.match(timestampPattern);
690+
// status should be active
691+
expect(source[ALERT_STATUS]).to.equal('active');
692+
// workflow status should be 'open'
693+
expect(source[ALERT_WORKFLOW_STATUS]).to.equal('open');
694+
// event.action should be 'open'
695+
expect(source[EVENT_ACTION]).to.equal('open');
696+
// event.kind should be 'signal'
697+
expect(source[EVENT_KIND]).to.equal('signal');
698+
// tags should equal rule tags because rule type doesn't set any tags
699+
expect(source.tags).to.eql(['foo']);
700+
// alert consecutive matches should match the active count
701+
expect(source[ALERT_CONSECUTIVE_MATCHES]).to.equal(3);
702+
703+
// --------------------------
704+
// RUN 4 - 1 active alert
705+
// --------------------------
706+
runSoon = await supertestWithoutAuth
707+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${ruleId}/_run_soon`)
708+
.set('kbn-xsrf', 'foo');
709+
expect(runSoon.status).to.eql(204);
710+
711+
events = await waitForEventLogDocs(ruleId, new Map([['execute', { equal: 4 }]]));
712+
executeEvent = events[3];
713+
executionUuid = get(executeEvent, UUID_PATH);
714+
expect(get(executeEvent, ACTIVE_PATH)).to.be(1);
715+
expect(get(executeEvent, NEW_PATH)).to.be(0);
716+
expect(get(executeEvent, RECOVERED_PATH)).to.be(0);
717+
expect(get(executeEvent, ACTION_PATH)).to.be(0);
718+
expect(get(executeEvent, DELAYED_PATH)).to.be(0);
719+
720+
// Query for alerts
721+
const alertDocsRun4 = await queryForAlertDocs<PatternFiringAlert>();
722+
723+
// Get alert state from task document
724+
state = await getTaskState(ruleId);
725+
expect(state.alertInstances.instance.meta.activeCount).to.equal(4);
726+
expect(state.alertInstances.instance.state.patternIndex).to.equal(3);
727+
728+
// After the fourth run, we should have 1 alert docs for the 1 active alert
729+
expect(alertDocsRun4.length).to.equal(1);
730+
731+
testExpectRuleData(alertDocsRun4, ruleId, { pattern }, executionUuid!);
732+
source = alertDocsRun4[0]._source!;
733+
const run3Source = alertDocsRun3[0]._source!;
734+
735+
expect(source[ALERT_UUID]).to.equal(run3Source[ALERT_UUID]);
736+
// patternIndex should be 3 for the fourth run
737+
expect(source.patternIndex).to.equal(3);
738+
expect(source[ALERT_ACTION_GROUP]).to.equal('default');
739+
// start time should be defined and the same as prior run
740+
expect(source[ALERT_START]).to.match(timestampPattern);
741+
expect(source[ALERT_START]).to.equal(run3Source[ALERT_START]);
742+
// timestamp should be defined and not the same as prior run
743+
expect(source['@timestamp']).to.match(timestampPattern);
744+
expect(source['@timestamp']).not.to.equal(run3Source['@timestamp']);
745+
// status should still be active
746+
expect(source[ALERT_STATUS]).to.equal('active');
747+
// event.action set to active
748+
expect(source[EVENT_ACTION]).to.eql('active');
749+
expect(source.tags).to.eql(['foo']);
750+
// these values should be the same as previous run
751+
expect(source[EVENT_KIND]).to.eql(run3Source[EVENT_KIND]);
752+
expect(source[ALERT_WORKFLOW_STATUS]).to.eql(run3Source[ALERT_WORKFLOW_STATUS]);
753+
expect(source[ALERT_TIME_RANGE]?.gte).to.equal(run3Source[ALERT_TIME_RANGE]?.gte);
754+
// alert consecutive matches should match the active count
755+
expect(source[ALERT_CONSECUTIVE_MATCHES]).to.equal(4);
756+
757+
// --------------------------
758+
// RUN 5 - 1 recovered alert
759+
// --------------------------
760+
runSoon = await supertestWithoutAuth
761+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${ruleId}/_run_soon`)
762+
.set('kbn-xsrf', 'foo');
763+
expect(runSoon.status).to.eql(204);
764+
765+
events = await waitForEventLogDocs(ruleId, new Map([['execute', { equal: 5 }]]));
766+
executeEvent = events[4];
767+
executionUuid = get(executeEvent, UUID_PATH);
768+
expect(get(executeEvent, ACTIVE_PATH)).to.be(0);
769+
expect(get(executeEvent, NEW_PATH)).to.be(0);
770+
expect(get(executeEvent, RECOVERED_PATH)).to.be(1);
771+
expect(get(executeEvent, ACTION_PATH)).to.be(0);
772+
expect(get(executeEvent, DELAYED_PATH)).to.be(0);
773+
774+
// Query for alerts
775+
const alertDocsRun5 = await queryForAlertDocs<PatternFiringAlert>();
776+
777+
// After the fourth run, we should have 1 alert docs for the 1 recovered alert
778+
expect(alertDocsRun5.length).to.equal(1);
779+
780+
testExpectRuleData(alertDocsRun5, ruleId, { pattern }, executionUuid!);
781+
source = alertDocsRun5[0]._source!;
782+
783+
// action group should be set to recovered
784+
expect(source[ALERT_ACTION_GROUP]).to.be('recovered');
785+
// rule type AAD payload should be set to recovery values
786+
expect(source.instancePattern).to.eql([]);
787+
expect(source.patternIndex).to.eql(-1);
788+
// uuid is the same
789+
expect(source[ALERT_UUID]).to.equal(run3Source[ALERT_UUID]);
790+
// start time should be defined and the same as before
791+
expect(source[ALERT_START]).to.match(timestampPattern);
792+
expect(source[ALERT_START]).to.equal(run3Source[ALERT_START]);
793+
// timestamp should be defined and not the same as prior run
794+
expect(source['@timestamp']).to.match(timestampPattern);
795+
expect(source['@timestamp']).not.to.equal(run3Source['@timestamp']);
796+
// end time should be defined
797+
expect(source[ALERT_END]).to.match(timestampPattern);
798+
// status should be set to recovered
799+
expect(source[ALERT_STATUS]).to.equal('recovered');
800+
// event.action set to close
801+
expect(source[EVENT_ACTION]).to.eql('close');
802+
expect(source.tags).to.eql(['foo']);
803+
// these values should be the same as previous run
804+
expect(source[EVENT_KIND]).to.eql(run3Source[EVENT_KIND]);
805+
expect(source[ALERT_WORKFLOW_STATUS]).to.eql(run3Source[ALERT_WORKFLOW_STATUS]);
806+
expect(source[ALERT_TIME_RANGE]?.gte).to.equal(run3Source[ALERT_TIME_RANGE]?.gte);
807+
// time_range.lte should be set to end time
808+
expect(source[ALERT_TIME_RANGE]?.lte).to.equal(source[ALERT_END]);
809+
// alert consecutive matches should match the active count
810+
expect(source[ALERT_CONSECUTIVE_MATCHES]).to.equal(0);
811+
812+
// --------------------------
813+
// RUN 6 - 0 new alerts
814+
// --------------------------
815+
runSoon = await supertestWithoutAuth
816+
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${ruleId}/_run_soon`)
817+
.set('kbn-xsrf', 'foo');
818+
expect(runSoon.status).to.eql(204);
819+
820+
events = await waitForEventLogDocs(ruleId, new Map([['execute', { equal: 6 }]]));
821+
executeEvent = events[5];
822+
expect(get(executeEvent, ACTIVE_PATH)).to.be(0);
823+
expect(get(executeEvent, NEW_PATH)).to.be(0);
824+
expect(get(executeEvent, RECOVERED_PATH)).to.be(0);
825+
expect(get(executeEvent, ACTION_PATH)).to.be(0);
826+
expect(get(executeEvent, DELAYED_PATH)).to.be(1);
827+
828+
// Query for alerts
829+
const alertDocsRun6 = await queryForAlertDocs<PatternFiringAlert>();
830+
831+
// Get alert state from task document
832+
state = await getTaskState(ruleId);
833+
expect(state.alertInstances.instance.meta.activeCount).to.equal(1);
834+
expect(state.alertInstances.instance.state.patternIndex).to.equal(5);
835+
836+
// After the sixth run, we should have 1 alert docs for the previously recovered alert
837+
expect(alertDocsRun6.length).to.equal(1);
838+
});
530839
});
531840

532841
function testExpectRuleData(

0 commit comments

Comments
 (0)