From 3cf11629e4409a83ce599d16af0398dc0d62dbe8 Mon Sep 17 00:00:00 2001 From: Wafaa Nasr Date: Tue, 29 Nov 2022 19:04:50 +0100 Subject: [PATCH 1/6] [Security Solution][List details page]: Manage rules - Selection cleared on navigation (#146121) ## Summary - This PR, is for adding the new `Link` column Switch instead of using table selection based on the below bug - Addresses https://github.com/elastic/kibana/issues/145683 - Remove `Tags` duplications and filter by tags ## Problem Manage Rules uses the `Add to Rule` component and this component under the hood is built on top of the [In Memory Table](https://elastic.github.io/eui/#/tabular-content/in-memory-tables#in-memory-table-selection) Currently, this table doesn't preserve the selection when the user navigates through the pages, which means if the user had some rules selected on the first page, then navigated to any other page the selection will be cleared if the user clicked on `Save` which will cause the linked rules to be unlinked from the list without notifying the user. This could be solved by: - Removing the `pagination` but the component won't fit in the `Add Exception Flyout` - **Add another column to the table to toggle (link/unlink) using a [Switch component](https://elastic.github.io/eui/#/forms/selection-controls#switch) with making the table's row not selectable** - Make use of the `toolsLeft` property given by the `In Memory Table` to design a new way of linking/unlinking - Control the height of the `Add to Rule` component ### **We decided in the Sync meeting to try the second option as it will give us full control over the Table data as well as we found some issues in the selection with the search** Not sure still what could be the quicker solution before the release, could you please advise @peluja1012 @jethr0null After investigating why the table removes the selected item: [euiOnPageChange](https://github.com/elastic/eui/blob/main/src/components/basic_table/basic_table.tsx#L480) The table under the hood, calls `clearSelection` on pagination image Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../add_to_rules_table/index.test.tsx | 3 +- .../add_to_rules_table/index.tsx | 133 +++++------------- .../link_rule_switch/index.tsx | 43 ++++++ .../add_to_rules_table/translations.ts | 7 + .../use_add_to_rules_table.tsx | 126 +++++++++++++++++ .../components/flyout_components/utils.tsx | 35 +++-- 6 files changed, 227 insertions(+), 120 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/link_rule_switch/index.tsx create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/use_add_to_rules_table.tsx diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.test.tsx index 48a710cc66ad8..7d56fcc298f90 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.test.tsx @@ -17,6 +17,7 @@ import type { Rule } from '../../../../rule_management/logic/types'; jest.mock( '../../../../rule_management_ui/components/rules_table/rules_table/use_find_rules_in_memory' ); +// TODO convert this test to RTL describe('ExceptionsAddToRulesTable', () => { it('it displays loading state while fetching rules', () => { @@ -35,7 +36,7 @@ describe('ExceptionsAddToRulesTable', () => { ).toBeTruthy(); }); - it('it displays fetched rules', () => { + it.skip('it displays fetched rules', () => { (useFindRulesInMemory as jest.Mock).mockReturnValue({ data: { rules: [getRulesSchemaMock(), { ...getRulesSchemaMock(), id: '345', name: 'My rule' }], diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.tsx index 7321bc06d75b4..4bf4c05d7ec94 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/index.tsx @@ -5,126 +5,57 @@ * 2.0. */ -import React, { useEffect, useMemo, useState } from 'react'; -import type { CriteriaWithPagination } from '@elastic/eui'; -import { EuiSpacer, EuiPanel, EuiText, EuiInMemoryTable, EuiLoadingContent } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; +import React from 'react'; -import { sortBy } from 'lodash'; -import * as myI18n from './translations'; +import { EuiSpacer, EuiPanel, EuiText, EuiInMemoryTable, EuiLoadingContent } from '@elastic/eui'; import type { Rule } from '../../../../rule_management/logic/types'; -import { useFindRulesInMemory } from '../../../../rule_management_ui/components/rules_table/rules_table/use_find_rules_in_memory'; -import { getRulesTableColumn } from '../utils'; +import { useAddToRulesTable } from './use_add_to_rules_table'; interface ExceptionsAddToRulesComponentProps { initiallySelectedRules?: Rule[]; - onRuleSelectionChange?: (rulesSelectedToAdd: Rule[]) => void; + onRuleSelectionChange: (rulesSelectedToAdd: Rule[]) => void; } const ExceptionsAddToRulesTableComponent: React.FC = ({ initiallySelectedRules, onRuleSelectionChange, }) => { - const { data: { rules } = { rules: [], total: 0 }, isFetched } = useFindRulesInMemory({ - isInMemorySorting: true, - filterOptions: { - filter: '', - showCustomRules: false, - showElasticRules: false, - tags: [], - }, - sortingOptions: undefined, - pagination: undefined, - refetchInterval: false, - }); - - const [pagination, setPagination] = useState({ pageIndex: 0 }); - const [message, setMessage] = useState( - - ); - - useEffect(() => { - if (!isFetched) { - setMessage( - - ); - } - - if (isFetched) { - setMessage(undefined); - } - }, [setMessage, isFetched]); - - const ruleSelectionValue = { - onSelectionChange: (selection: Rule[]) => { - if (onRuleSelectionChange != null) { - onRuleSelectionChange(selection); - } - }, - initialSelected: initiallySelectedRules ?? [], - }; + const { + isLoading, - const searchOptions = useMemo( - () => ({ - box: { - incremental: true, - }, - filters: [ - { - type: 'field_value_selection' as const, - field: 'tags', - name: i18n.translate( - 'xpack.securitySolution.exceptions.addToRulesTable.tagsFilterLabel', - { - defaultMessage: 'Tags', - } - ), - multiSelect: 'or' as const, - options: rules.flatMap(({ tags }) => { - return tags.map((tag) => ({ - value: tag, - name: tag, - })); - }), - }, - ], - }), - [rules] - ); - - const sortedRulesBySelection = useMemo( - () => - sortBy(rules, [ - (rule) => { - return initiallySelectedRules?.find((initRule) => initRule.id === rule.id); - }, - ]), - [initiallySelectedRules, rules] - ); + searchOptions, + pagination, + sortedRulesByLinkedRulesOnTop, + rulesTableColumnsWithLinkSwitch, + onTableChange, + addToSelectedRulesDescription, + } = useAddToRulesTable({ + initiallySelectedRules, + onRuleSelectionChange, + }); return ( <> - {myI18n.ADD_TO_SELECTED_RULES_DESCRIPTION} + {addToSelectedRulesDescription} - tableCaption="Rules table" - items={sortedRulesBySelection} - itemId="id" - loading={!isFetched} - columns={getRulesTableColumn()} - pagination={{ - ...pagination, - itemsPerPage: 5, - showPerPageOptions: false, - }} - message={message} - onTableChange={({ page: { index } }: CriteriaWithPagination) => - setPagination({ pageIndex: index }) - } - selection={ruleSelectionValue} + tableLayout="auto" search={searchOptions} - isSelectable data-test-subj="addExceptionToRulesTable" + tableCaption="Rules table" + items={sortedRulesByLinkedRulesOnTop} + loading={isLoading} + columns={rulesTableColumnsWithLinkSwitch} + message={ + isLoading ? ( + + ) : null + } + pagination={pagination} + onTableChange={onTableChange} /> diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/link_rule_switch/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/link_rule_switch/index.tsx new file mode 100644 index 0000000000000..b01fe7209dd1a --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/link_rule_switch/index.tsx @@ -0,0 +1,43 @@ +/* + * 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 React, { memo, useCallback, useMemo } from 'react'; +import { EuiFlexItem, EuiSwitch } from '@elastic/eui'; +import type { Rule } from '../../../../../rule_management/logic/types'; + +export const LinkRuleSwitch = memo( + ({ + rule, + linkedRules, + onRuleLinkChange, + }: { + rule: Rule; + linkedRules: Rule[]; + onRuleLinkChange: (rulesSelectedToAdd: Rule[]) => void; + }) => { + const isRuleLinked = useMemo( + () => Boolean(linkedRules.find((r) => r.id === rule.id)), + [linkedRules, rule.id] + ); + const onLinkOrUnlinkRule = useCallback( + ({ target: { checked } }) => { + const newLinkedRules = !checked + ? linkedRules?.filter((item) => item.id !== rule.id) + : [...linkedRules, rule]; + if (typeof onRuleLinkChange === 'function') onRuleLinkChange(newLinkedRules); + }, + [linkedRules, onRuleLinkChange, rule] + ); + + return ( + + + + ); + } +); + +LinkRuleSwitch.displayName = 'LinkRuleSwitch'; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/translations.ts index da34fa5f83451..cbd0114c2300d 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/translations.ts @@ -14,3 +14,10 @@ export const ADD_TO_SELECTED_RULES_DESCRIPTION = i18n.translate( 'Select rules add to. We will make a copy of this exception if it links to multiple rules. ', } ); + +export const LINK_COLUMN = i18n.translate( + 'xpack.securitySolution.rule_exceptions.flyoutComponents.addToRulesTableSelection.link_column', + { + defaultMessage: 'Link', + } +); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/use_add_to_rules_table.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/use_add_to_rules_table.tsx new file mode 100644 index 0000000000000..95ede707e7d3d --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/add_to_rules_table/use_add_to_rules_table.tsx @@ -0,0 +1,126 @@ +/* + * 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 React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { sortBy } from 'lodash'; +import type { + CriteriaWithPagination, + EuiBasicTableColumn, + HorizontalAlignment, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import * as myI18n from './translations'; + +import { useFindRulesInMemory } from '../../../../rule_management_ui/components/rules_table/rules_table/use_find_rules_in_memory'; +import type { Rule } from '../../../../rule_management/logic/types'; +import { getRulesTableColumn } from '../utils'; +import { LinkRuleSwitch } from './link_rule_switch'; + +export interface ExceptionsAddToRulesComponentProps { + initiallySelectedRules?: Rule[]; + onRuleSelectionChange: (rulesSelectedToAdd: Rule[]) => void; +} +export const useAddToRulesTable = ({ + initiallySelectedRules, + onRuleSelectionChange, +}: ExceptionsAddToRulesComponentProps) => { + const { data: { rules } = { rules: [], total: 0 }, isFetched } = useFindRulesInMemory({ + isInMemorySorting: true, + filterOptions: { + filter: '', + showCustomRules: false, + showElasticRules: false, + tags: [], + }, + sortingOptions: undefined, + pagination: undefined, + refetchInterval: false, + }); + + const [pagination, setPagination] = useState({ + pageIndex: 0, + initialPageSize: 5, + showPerPageOptions: false, + }); + + const [linkedRules, setLinkedRules] = useState(initiallySelectedRules || []); + useEffect(() => { + onRuleSelectionChange(linkedRules); + }, [linkedRules, onRuleSelectionChange]); + + const sortedRulesByLinkedRulesOnTop = useMemo( + () => + sortBy(rules, [ + (rule) => { + return initiallySelectedRules?.find((initRule) => initRule.id === rule.id); + }, + ]), + [initiallySelectedRules, rules] + ); + + const tagOptions = useMemo(() => { + const uniqueTags = sortedRulesByLinkedRulesOnTop.reduce((acc: Set, item: Rule) => { + const { tags } = item; + + tags.forEach((tag) => acc.add(tag)); + return acc; + }, new Set()); + return [...uniqueTags].map((tag) => ({ value: tag, name: tag })); + }, [sortedRulesByLinkedRulesOnTop]); + + const searchOptions = useMemo( + () => ({ + box: { + incremental: true, + }, + filters: [ + { + type: 'field_value_selection' as const, + field: 'tags', + name: i18n.translate( + 'xpack.securitySolution.exceptions.addToRulesTable.tagsFilterLabel', + { + defaultMessage: 'Tags', + } + ), + multiSelect: 'or' as const, + options: tagOptions, + }, + ], + }), + [tagOptions] + ); + + const rulesTableColumnsWithLinkSwitch: Array> = useMemo( + () => [ + { + field: 'link', + name: myI18n.LINK_COLUMN, + align: 'left' as HorizontalAlignment, + 'data-test-subj': 'ruleActionLinkRuleSwitch', + render: (_, rule: Rule) => ( + + ), + }, + ...getRulesTableColumn(), + ], + [linkedRules] + ); + const onTableChange = useCallback( + ({ page: { index } }: CriteriaWithPagination) => + setPagination({ ...pagination, pageIndex: index }), + [pagination] + ); + return { + isLoading: !isFetched, + pagination, + searchOptions, + sortedRulesByLinkedRulesOnTop, + rulesTableColumnsWithLinkSwitch, + addToSelectedRulesDescription: myI18n.ADD_TO_SELECTED_RULES_DESCRIPTION, + onTableChange, + }; +}; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/utils.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/utils.tsx index 51da06de63bf9..714d2b460d111 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/utils.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/utils.tsx @@ -12,6 +12,7 @@ import type { ExceptionListSchema, OsType } from '@kbn/securitysolution-io-ts-li import { ExceptionListTypeEnum } from '@kbn/securitysolution-io-ts-list-types'; import type { ExceptionsBuilderReturnExceptionItem } from '@kbn/securitysolution-list-utils'; +import type { HorizontalAlignment } from '@elastic/eui'; import { enrichExceptionItemsWithOS, enrichNewExceptionItemsWithComments, @@ -224,7 +225,7 @@ export const getSharedListsTableColumns = () => [ {i18n.VIEW_LIST_DETAIL_ACTION} @@ -242,28 +243,26 @@ export const getSharedListsTableColumns = () => [ export const getRulesTableColumn = () => [ { field: 'name', + align: 'left' as HorizontalAlignment, name: 'Name', sortable: true, 'data-test-subj': 'ruleNameCell', + truncateText: false, }, { name: 'Actions', - actions: [ - { - 'data-test-subj': 'ruleAction-view', - render: (rule: Rule) => { - return ( - - {i18n.VIEW_RULE_DETAIL_ACTION} - - ); - }, - }, - ], + 'data-test-subj': 'ruleAction-view', + render: (rule: Rule) => { + return ( + + {i18n.VIEW_RULE_DETAIL_ACTION} + + ); + }, }, ]; From b8f53e6d0efa122402241eed9f831c2e88f810b9 Mon Sep 17 00:00:00 2001 From: Mark Hopkin Date: Tue, 29 Nov 2022 18:16:49 +0000 Subject: [PATCH 2/6] [Fleet] Fix PUT settings API docs (#146572) ## Summary Resolves https://github.com/elastic/kibana/issues/130178. PUT settings API incorrectly had POST as the method. I have also added the request body. --- .../plugins/fleet/common/openapi/bundled.json | 35 ++++++++++++++++--- .../plugins/fleet/common/openapi/bundled.yaml | 22 ++++++++++-- .../fleet/common/openapi/paths/settings.yaml | 17 ++++++++- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/fleet/common/openapi/bundled.json b/x-pack/plugins/fleet/common/openapi/bundled.json index 430d37abb25b4..37b4e4800557f 100644 --- a/x-pack/plugins/fleet/common/openapi/bundled.json +++ b/x-pack/plugins/fleet/common/openapi/bundled.json @@ -147,9 +147,33 @@ }, "operationId": "get-settings" }, - "post": { + "put": { "summary": "Settings - Update", "tags": [], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "fleet_server_hosts": { + "type": "array", + "description": "Protocol and path must be the same for each URL", + "items": { + "type": "string" + } + }, + "has_seen_add_data_notice": { + "type": "boolean" + }, + "additional_yaml_config": { + "type": "string" + } + } + } + } + } + }, "responses": { "200": { "description": "OK", @@ -6063,7 +6087,8 @@ "type": { "type": "string", "enum": [ - "elasticsearch" + "elasticsearch", + "logstash" ] }, "hosts": { @@ -6087,15 +6112,15 @@ "ssl": { "type": "object", "properties": { - "certificate": { - "type": "string" - }, "certificate_authorities": { "type": "array", "items": { "type": "string" } }, + "certificate": { + "type": "string" + }, "key": { "type": "string" } diff --git a/x-pack/plugins/fleet/common/openapi/bundled.yaml b/x-pack/plugins/fleet/common/openapi/bundled.yaml index 22abffbf9224a..95fcd9513b467 100644 --- a/x-pack/plugins/fleet/common/openapi/bundled.yaml +++ b/x-pack/plugins/fleet/common/openapi/bundled.yaml @@ -92,9 +92,24 @@ paths: schema: $ref: '#/components/schemas/fleet_settings_response' operationId: get-settings - post: + put: summary: Settings - Update tags: [] + requestBody: + content: + application/json: + schema: + type: object + properties: + fleet_server_hosts: + type: array + description: Protocol and path must be the same for each URL + items: + type: string + has_seen_add_data_notice: + type: boolean + additional_yaml_config: + type: string responses: '200': description: OK @@ -3885,6 +3900,7 @@ components: type: string enum: - elasticsearch + - logstash hosts: type: array items: @@ -3900,12 +3916,12 @@ components: ssl: type: object properties: - certificate: - type: string certificate_authorities: type: array items: type: string + certificate: + type: string key: type: string required: diff --git a/x-pack/plugins/fleet/common/openapi/paths/settings.yaml b/x-pack/plugins/fleet/common/openapi/paths/settings.yaml index b23fbc698e423..c9a420cdbb599 100644 --- a/x-pack/plugins/fleet/common/openapi/paths/settings.yaml +++ b/x-pack/plugins/fleet/common/openapi/paths/settings.yaml @@ -9,9 +9,24 @@ get: schema: $ref: ../components/schemas/fleet_settings_response.yaml operationId: get-settings -post: +put: summary: Settings - Update tags: [] + requestBody: + content: + application/json: + schema: + type: object + properties: + fleet_server_hosts: + type: array + description: Protocol and path must be the same for each URL + items: + type: string + has_seen_add_data_notice: + type: boolean + additional_yaml_config: + type: string responses: '200': description: OK From d8ebdc0460efb88fd619d01769283d09c1df7ce2 Mon Sep 17 00:00:00 2001 From: Sander Philipse <94373878+sphilipse@users.noreply.github.com> Date: Tue, 29 Nov 2022 19:18:18 +0100 Subject: [PATCH 3/6] [Enterprise Search] Use 'DEFAULT' id for default rule (#146587) --- .../sync_rules/connector_filtering_logic.tsx | 41 ++++++++----------- .../sync_rules/editable_basic_rules_table.tsx | 7 +++- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/connector_filtering_logic.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/connector_filtering_logic.tsx index 78d13e8d20ba5..c430794063a76 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/connector_filtering_logic.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/connector_filtering_logic.tsx @@ -9,8 +9,6 @@ import { kea, MakeLogicType } from 'kea'; import { isEqual } from 'lodash'; -import { v4 as uuidv4 } from 'uuid'; - import { i18n } from '@kbn/i18n'; import { Status } from '../../../../../../../common/types/api'; @@ -97,6 +95,20 @@ interface ConnectorFilteringValues { status: Status; } +function createDefaultRule(order: number) { + const now = new Date().toISOString(); + return { + created_at: now, + field: '_', + id: 'DEFAULT', + order, + policy: FilteringPolicy.INCLUDE, + rule: FilteringRuleRule.REGEX, + updated_at: now, + value: '.*', + }; +} + export const ConnectorFilteringLogic = kea< MakeLogicType >({ @@ -255,19 +267,7 @@ export const ConnectorFilteringLogic = kea< filteringRule, filteringRules[filteringRules.length - 1], ] - : [ - filteringRule, - { - created_at: new Date().toISOString(), - field: '_', - id: uuidv4(), - order: 0, - policy: FilteringPolicy.INCLUDE, - rule: FilteringRuleRule.REGEX, - updated_at: new Date().toISOString(), - value: '.*', - }, - ]; + : [filteringRule, createDefaultRule(1)]; return newFilteringRules.map((rule, index) => ({ ...rule, order: index })); }, deleteFilteringRule: (filteringRules, filteringRule) => @@ -275,16 +275,7 @@ export const ConnectorFilteringLogic = kea< reorderFilteringRules: (filteringRules, newFilteringRules) => { const lastItem = filteringRules.length ? filteringRules[filteringRules.length - 1] - : { - created_at: new Date().toISOString(), - field: '_', - id: uuidv4(), - order: 0, - policy: FilteringPolicy.INCLUDE, - rule: FilteringRuleRule.REGEX, - updated_at: new Date().toISOString(), - value: '.*', - }; + : createDefaultRule(0); return [...newFilteringRules, lastItem].map((rule, index) => ({ ...rule, order: index })); }, setLocalFilteringRules: (_, filteringRules) => filteringRules, diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/editable_basic_rules_table.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/editable_basic_rules_table.tsx index ecdc82d2343d5..55b5be00b86c5 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/editable_basic_rules_table.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/sync_rules/editable_basic_rules_table.tsx @@ -210,7 +210,12 @@ export const SyncRulesTable: React.FC = () => { } as InlineEditableTableProps).actions.doneEditing(); }} onDelete={deleteFilteringRule} - onUpdate={updateFilteringRule} + onUpdate={(rule) => { + updateFilteringRule(rule); + InlineEditableTableLogic({ + instanceId, + } as InlineEditableTableProps).actions.doneEditing(); + }} onReorder={reorderFilteringRules} title="" validateItem={validateItem} From 026029d3c7011164e61b9de554b32c54e2c1a28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Tue, 29 Nov 2022 13:36:01 -0500 Subject: [PATCH 4/6] [APM] Mobile loading screen shows irrelevant components (#146476) closes https://github.com/elastic/kibana/issues/145964 Before this was the workflow after a mobile service was clicked.: ``` service inventory -> service overview -> service mobile overview ``` Now, this is the new workflow: ``` service inventory -> loading screen -> service mobile overview ``` Screen Shot 2022-11-28 at 3 47 33 PM cc: @boriskirov what do you think of this? We have to wait until the service agent is fetched to decide which page to show the mobile or the default one. --- .../top_erroneous_transactions/index.tsx | 9 ++++-- .../app/error_group_overview/index.tsx | 9 +++--- .../service_groups_list/index.tsx | 5 ++-- .../components/app/service_overview/index.tsx | 30 +++++++++++++++---- ...ice_overview_instances_chart_and_table.tsx | 11 +++---- .../instance_actions_menu/index.tsx | 7 ++--- .../intance_details.tsx | 7 ++--- .../index_stats_per_service.tsx | 5 ++-- .../storage_details_per_service.tsx | 7 ++--- .../app/storage_explorer/summary_stats.tsx | 5 ++-- .../analyze_data_button.stories.tsx | 2 ++ .../labs/labs_flyout.tsx | 5 ++-- .../shared/charts/chart_container.test.tsx | 12 +++----- .../shared/charts/chart_container.tsx | 4 +-- .../latency_chart/latency_chart.stories.tsx | 2 ++ .../shared/critical_path_flamegraph/index.tsx | 11 +++---- .../get_span_metric_columns.tsx | 7 ++--- .../components/shared/span_links/index.tsx | 8 ++--- .../shared/transactions_table/index.tsx | 12 +++++--- .../apm_service/apm_service_context.tsx | 10 ++++++- .../apm/public/hooks/use_fetcher.test.tsx | 21 ++++++++++++- .../plugins/apm/public/hooks/use_fetcher.tsx | 4 +++ 22 files changed, 117 insertions(+), 76 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/error_group_details/top_erroneous_transactions/index.tsx b/x-pack/plugins/apm/public/components/app/error_group_details/top_erroneous_transactions/index.tsx index 7b5dce8226aee..2fc31645b5d39 100644 --- a/x-pack/plugins/apm/public/components/app/error_group_details/top_erroneous_transactions/index.tsx +++ b/x-pack/plugins/apm/public/components/app/error_group_details/top_erroneous_transactions/index.tsx @@ -25,7 +25,11 @@ import { isTimeComparison } from '../../../shared/time_comparison/get_comparison import { useApmParams } from '../../../../hooks/use_apm_params'; import { TransactionDetailLink } from '../../../shared/links/apm/transaction_detail_link'; import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; -import { useFetcher, FETCH_STATUS } from '../../../../hooks/use_fetcher'; +import { + useFetcher, + FETCH_STATUS, + isPending, +} from '../../../../hooks/use_fetcher'; import { useTimeRange } from '../../../../hooks/use_time_range'; import { asInteger } from '../../../../../common/utils/formatters'; @@ -90,8 +94,7 @@ export function TopErroneousTransactions({ serviceName }: Props) { ] ); - const loading = - status === FETCH_STATUS.LOADING || status === FETCH_STATUS.NOT_INITIATED; + const loading = isPending(status); const columns: Array< EuiBasicTableColumn< diff --git a/x-pack/plugins/apm/public/components/app/error_group_overview/index.tsx b/x-pack/plugins/apm/public/components/app/error_group_overview/index.tsx index 8e62a5fe7d81c..514759c2b3dfe 100644 --- a/x-pack/plugins/apm/public/components/app/error_group_overview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/error_group_overview/index.tsx @@ -21,7 +21,7 @@ import { useApmServiceContext } from '../../../context/apm_service/use_apm_servi import { ChartPointerEventContextProvider } from '../../../context/chart_pointer_event/chart_pointer_event_context'; import { useApmParams } from '../../../hooks/use_apm_params'; import { useErrorGroupDistributionFetcher } from '../../../hooks/use_error_group_distribution_fetcher'; -import { useFetcher, FETCH_STATUS } from '../../../hooks/use_fetcher'; +import { useFetcher, isPending } from '../../../hooks/use_fetcher'; import { useTimeRange } from '../../../hooks/use_time_range'; import { APIReturnType } from '../../../services/rest/create_call_apm_api'; import { FailedTransactionRateChart } from '../../shared/charts/failed_transaction_rate_chart'; @@ -212,10 +212,9 @@ export function ErrorGroupOverview() { - {isMobileAgent ? ( - + {isPendingServiceAgent ? ( + + + + + + ) : ( - + <> + {isMobileAgent ? ( + + ) : ( + + )} + )} diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_chart_and_table.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_chart_and_table.tsx index 5b1e792ec5a20..e426899c7390f 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_chart_and_table.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_chart_and_table.tsx @@ -12,7 +12,11 @@ import uuid from 'uuid'; import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options'; import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context'; import { useApmParams } from '../../../hooks/use_apm_params'; -import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher'; +import { + FETCH_STATUS, + isPending, + useFetcher, +} from '../../../hooks/use_fetcher'; import { useTimeRange } from '../../../hooks/use_time_range'; import { APIReturnType } from '../../../services/rest/create_call_apm_api'; import { InstancesLatencyDistributionChart } from '../../shared/charts/instances_latency_distribution_chart'; @@ -232,10 +236,7 @@ export function ServiceOverviewInstancesChartAndTable({ mainStatsItems={currentPeriodOrderedItems} mainStatsStatus={mainStatsStatus} mainStatsItemCount={currentPeriodItemsCount} - detailedStatsLoading={ - detailedStatsStatus === FETCH_STATUS.LOADING || - detailedStatsStatus === FETCH_STATUS.NOT_INITIATED - } + detailedStatsLoading={isPending(detailedStatsStatus)} detailedStatsData={detailedStatsData} serviceName={serviceName} tableOptions={tableOptions} diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/instance_actions_menu/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/instance_actions_menu/index.tsx index f37e9b52b7093..c7f35509e494b 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/instance_actions_menu/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/instance_actions_menu/index.tsx @@ -18,7 +18,7 @@ import { import { isJavaAgentName } from '../../../../../../common/agent_name'; import { SERVICE_NODE_NAME } from '../../../../../../common/es_fields/apm'; import { useApmPluginContext } from '../../../../../context/apm_plugin/use_apm_plugin_context'; -import { FETCH_STATUS } from '../../../../../hooks/use_fetcher'; +import { isPending } from '../../../../../hooks/use_fetcher'; import { pushNewItemToKueryBar } from '../../../../shared/kuery_bar/utils'; import { useMetricOverviewHref } from '../../../../shared/links/apm/metric_overview_link'; import { useServiceNodeMetricOverviewHref } from '../../../../shared/links/apm/service_node_metric_overview_link'; @@ -52,10 +52,7 @@ export function InstanceActionsMenu({ const metricOverviewHref = useMetricOverviewHref(serviceName); const history = useHistory(); - if ( - status === FETCH_STATUS.LOADING || - status === FETCH_STATUS.NOT_INITIATED - ) { + if (isPending(status)) { return (
diff --git a/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/index_stats_per_service.tsx b/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/index_stats_per_service.tsx index 04505e85ef806..89d282975d59b 100644 --- a/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/index_stats_per_service.tsx +++ b/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/index_stats_per_service.tsx @@ -20,7 +20,7 @@ import { asDynamicBytes, asInteger, } from '../../../../../common/utils/formatters'; -import { FETCH_STATUS } from '../../../../hooks/use_fetcher'; +import { FETCH_STATUS, isPending } from '../../../../hooks/use_fetcher'; import type { APIReturnType } from '../../../../services/rest/create_call_apm_api'; import { SizeLabel } from './size_label'; @@ -100,8 +100,7 @@ export function IndexStatsPerService({ indicesStats, status }: Props) { }, ]; - const loading = - status === FETCH_STATUS.NOT_INITIATED || status === FETCH_STATUS.LOADING; + const loading = isPending(status); return ( <> diff --git a/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/storage_details_per_service.tsx b/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/storage_details_per_service.tsx index 4005c01f1b8f5..ba8005f12759c 100644 --- a/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/storage_details_per_service.tsx +++ b/x-pack/plugins/apm/public/components/app/storage_explorer/services_table/storage_details_per_service.tsx @@ -32,7 +32,7 @@ import { ProcessorEvent } from '@kbn/observability-plugin/common'; import { IndexLifecyclePhaseSelectOption } from '../../../../../common/storage_explorer_types'; import { useApmParams } from '../../../../hooks/use_apm_params'; import { useTimeRange } from '../../../../hooks/use_time_range'; -import { FETCH_STATUS } from '../../../../hooks/use_fetcher'; +import { isPending } from '../../../../hooks/use_fetcher'; import { useProgressiveFetcher } from '../../../../hooks/use_progressive_fetcher'; import { useApmRouter } from '../../../../hooks/use_apm_router'; import { asInteger } from '../../../../../common/utils/formatters/formatters'; @@ -131,10 +131,7 @@ export function StorageDetailsPerService({ [indexLifecyclePhase, start, end, environment, kuery, serviceName] ); - if ( - status === FETCH_STATUS.LOADING || - status === FETCH_STATUS.NOT_INITIATED - ) { + if (isPending(status)) { return (
diff --git a/x-pack/plugins/apm/public/components/app/storage_explorer/summary_stats.tsx b/x-pack/plugins/apm/public/components/app/storage_explorer/summary_stats.tsx index b6f6e5167f3d3..506b966f0d3ca 100644 --- a/x-pack/plugins/apm/public/components/app/storage_explorer/summary_stats.tsx +++ b/x-pack/plugins/apm/public/components/app/storage_explorer/summary_stats.tsx @@ -29,7 +29,7 @@ import { useApmParams } from '../../../hooks/use_apm_params'; import { asDynamicBytes, asPercent } from '../../../../common/utils/formatters'; import { useApmRouter } from '../../../hooks/use_apm_router'; import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_context'; -import { FETCH_STATUS } from '../../../hooks/use_fetcher'; +import { isPending } from '../../../hooks/use_fetcher'; import { asTransactionRate } from '../../../../common/utils/formatters'; import { getIndexManagementHref } from './get_storage_explorer_links'; @@ -78,8 +78,7 @@ export function SummaryStats() { [indexLifecyclePhase, environment, kuery, start, end] ); - const loading = - status === FETCH_STATUS.LOADING || status === FETCH_STATUS.NOT_INITIATED; + const loading = isPending(status); const hasData = !isEmpty(data); diff --git a/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/analyze_data_button.stories.tsx b/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/analyze_data_button.stories.tsx index 2708c46b52960..57a0e789267b1 100644 --- a/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/analyze_data_button.stories.tsx +++ b/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/analyze_data_button.stories.tsx @@ -14,6 +14,7 @@ import { ENVIRONMENT_ALL } from '../../../../../common/environment_filter_values import { MockApmPluginContextWrapper } from '../../../../context/apm_plugin/mock_apm_plugin_context'; import { APMServiceContext } from '../../../../context/apm_service/apm_service_context'; import { AnalyzeDataButton } from './analyze_data_button'; +import { FETCH_STATUS } from '../../../../hooks/use_fetcher'; interface Args { agentName: string; @@ -51,6 +52,7 @@ export default { transactionTypes: [], serviceName, fallbackToTransactions: false, + serviceAgentStatus: FETCH_STATUS.SUCCESS, }} > diff --git a/x-pack/plugins/apm/public/components/shared/apm_header_action_menu/labs/labs_flyout.tsx b/x-pack/plugins/apm/public/components/shared/apm_header_action_menu/labs/labs_flyout.tsx index 9c6983adabc06..6c5d38d0f1f7d 100644 --- a/x-pack/plugins/apm/public/components/shared/apm_header_action_menu/labs/labs_flyout.tsx +++ b/x-pack/plugins/apm/public/components/shared/apm_header_action_menu/labs/labs_flyout.tsx @@ -26,7 +26,7 @@ import { i18n } from '@kbn/i18n'; import React from 'react'; import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context'; import { useApmEditableSettings } from '../../../../hooks/use_apm_editable_settings'; -import { useFetcher, FETCH_STATUS } from '../../../../hooks/use_fetcher'; +import { useFetcher, isPending } from '../../../../hooks/use_fetcher'; interface Props { onClose: () => void; @@ -79,8 +79,7 @@ export function LabsFlyout({ onClose }: Props) { onClose(); } - const isLoading = - status === FETCH_STATUS.NOT_INITIATED || status === FETCH_STATUS.LOADING; + const isLoading = isPending(status); return ( diff --git a/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx index efb1fa8d8655f..0411c03b30f73 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx @@ -12,14 +12,10 @@ import { ChartContainer } from './chart_container'; describe('ChartContainer', () => { describe('loading indicator', () => { - it('shows loading when status equals to Loading or Pending and has no data', () => { - [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].map((status) => { + it('shows loading when status equals to Loading or Not initiated and has no data', () => { + [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].forEach((status) => { const { queryAllByTestId } = render( - +
My amazing component
); @@ -28,7 +24,7 @@ describe('ChartContainer', () => { }); }); it('does not show loading when status equals to Loading or Pending and has data', () => { - [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].map((status) => { + [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].forEach((status) => { const { queryAllByText } = render(
My amazing component
diff --git a/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx b/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx index 695e62b3b7d78..ce676c6077594 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx @@ -8,7 +8,7 @@ import { EuiLoadingChart, EuiText } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; -import { FETCH_STATUS } from '../../../hooks/use_fetcher'; +import { FETCH_STATUS, isPending } from '../../../hooks/use_fetcher'; export interface ChartContainerProps { hasData: boolean; @@ -25,7 +25,7 @@ export function ChartContainer({ hasData, id, }: ChartContainerProps) { - if (!hasData && status === FETCH_STATUS.LOADING) { + if (!hasData && isPending(status)) { return ; } diff --git a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/latency_chart.stories.tsx b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/latency_chart.stories.tsx index c5bf5e3ae147d..23ec2140f5be7 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/latency_chart.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/latency_chart.stories.tsx @@ -20,6 +20,7 @@ import { createCallApmApi, } from '../../../../services/rest/create_call_apm_api'; import { LatencyChart } from '.'; +import { FETCH_STATUS } from '../../../../hooks/use_fetcher'; interface Args { latencyChartResponse: APIReturnType<'GET /internal/apm/services/{serviceName}/transactions/charts/latency'>; @@ -85,6 +86,7 @@ const stories: Meta = { transactionType, transactionTypes: [], fallbackToTransactions: false, + serviceAgentStatus: FETCH_STATUS.SUCCESS, }} > diff --git a/x-pack/plugins/apm/public/components/shared/critical_path_flamegraph/index.tsx b/x-pack/plugins/apm/public/components/shared/critical_path_flamegraph/index.tsx index f99b82a112c78..c4e989dd0ac4f 100644 --- a/x-pack/plugins/apm/public/components/shared/critical_path_flamegraph/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/critical_path_flamegraph/index.tsx @@ -15,7 +15,11 @@ import { css } from '@emotion/css'; import { useChartTheme } from '@kbn/observability-plugin/public'; import { uniqueId } from 'lodash'; import React, { useMemo, useRef } from 'react'; -import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher'; +import { + FETCH_STATUS, + useFetcher, + isPending, +} from '../../../hooks/use_fetcher'; import { CriticalPathFlamegraphTooltip } from './critical_path_flamegraph_tooltip'; import { criticalPathToFlamegraph } from './critical_path_to_flamegraph'; @@ -71,10 +75,7 @@ export function CriticalPathFlamegraph( const chartTheme = useChartTheme(); const isLoading = - traceIdsFetchStatus === FETCH_STATUS.NOT_INITIATED || - traceIdsFetchStatus === FETCH_STATUS.LOADING || - criticalPathFetchStatus === FETCH_STATUS.NOT_INITIATED || - criticalPathFetchStatus === FETCH_STATUS.LOADING; + isPending(traceIdsFetchStatus) || isPending(criticalPathFetchStatus); const flameGraph = useMemo(() => { if (!criticalPath) { diff --git a/x-pack/plugins/apm/public/components/shared/dependencies_table/get_span_metric_columns.tsx b/x-pack/plugins/apm/public/components/shared/dependencies_table/get_span_metric_columns.tsx index 49d9d342947f9..d477e7269cace 100644 --- a/x-pack/plugins/apm/public/components/shared/dependencies_table/get_span_metric_columns.tsx +++ b/x-pack/plugins/apm/public/components/shared/dependencies_table/get_span_metric_columns.tsx @@ -20,7 +20,7 @@ import { } from '../charts/helper/get_timeseries_color'; import { ListMetric } from '../list_metric'; import { ITableColumn } from '../managed_table'; -import { FETCH_STATUS } from '../../../hooks/use_fetcher'; +import { FETCH_STATUS, isPending } from '../../../hooks/use_fetcher'; import { asMillisecondDuration, asPercent, @@ -61,9 +61,8 @@ export function getSpanMetricColumns({ }): Array> { const { isLarge } = breakpoints; const shouldShowSparkPlots = !isLarge; - const isLoading = - comparisonFetchStatus === FETCH_STATUS.LOADING || - comparisonFetchStatus === FETCH_STATUS.NOT_INITIATED; + const isLoading = isPending(comparisonFetchStatus); + return [ { field: 'latency', diff --git a/x-pack/plugins/apm/public/components/shared/span_links/index.tsx b/x-pack/plugins/apm/public/components/shared/span_links/index.tsx index 77d0451adca55..070993c81a2c5 100644 --- a/x-pack/plugins/apm/public/components/shared/span_links/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/span_links/index.tsx @@ -15,7 +15,7 @@ import { i18n } from '@kbn/i18n'; import React, { useMemo, useState } from 'react'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; import { useApmParams } from '../../../hooks/use_apm_params'; -import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher'; +import { isPending, useFetcher } from '../../../hooks/use_fetcher'; import { useTimeRange } from '../../../hooks/use_time_range'; import { SpanLinksCount } from '../../app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers'; import { KueryBar } from '../kuery_bar'; @@ -100,11 +100,7 @@ export function SpanLinks({ [spanLinksCount] ); - if ( - !data || - status === FETCH_STATUS.LOADING || - status === FETCH_STATUS.NOT_INITIATED - ) { + if (!data || isPending(status)) { return (
diff --git a/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx b/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx index 63315c90a95d6..5b3ec70bf7af4 100644 --- a/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx @@ -21,7 +21,11 @@ import { EuiCode } from '@elastic/eui'; import { useHistory } from 'react-router-dom'; import { APIReturnType } from '../../../services/rest/create_call_apm_api'; import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context'; -import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher'; +import { + FETCH_STATUS, + isPending, + useFetcher, +} from '../../../hooks/use_fetcher'; import { TransactionOverviewLink } from '../links/apm/transaction_overview_link'; import { OverviewTableContainer } from '../overview_table_container'; import { getColumns } from './get_columns'; @@ -241,9 +245,9 @@ export function TransactionsTable({ const columns = getColumns({ serviceName, latencyAggregationType: latencyAggregationType as LatencyAggregationType, - transactionGroupDetailedStatisticsLoading: - transactionGroupDetailedStatisticsStatus === FETCH_STATUS.LOADING || - transactionGroupDetailedStatisticsStatus === FETCH_STATUS.NOT_INITIATED, + transactionGroupDetailedStatisticsLoading: isPending( + transactionGroupDetailedStatisticsStatus + ), transactionGroupDetailedStatistics, comparisonEnabled, shouldShowSparkPlots, diff --git a/x-pack/plugins/apm/public/context/apm_service/apm_service_context.tsx b/x-pack/plugins/apm/public/context/apm_service/apm_service_context.tsx index b9d9d4012a6c0..afbad8543059a 100644 --- a/x-pack/plugins/apm/public/context/apm_service/apm_service_context.tsx +++ b/x-pack/plugins/apm/public/context/apm_service/apm_service_context.tsx @@ -19,6 +19,7 @@ import { useApmParams } from '../../hooks/use_apm_params'; import { useTimeRange } from '../../hooks/use_time_range'; import { useFallbackToTransactionsFetcher } from '../../hooks/use_fallback_to_transactions_fetcher'; import { replace } from '../../components/shared/links/url_helpers'; +import { FETCH_STATUS } from '../../hooks/use_fetcher'; export interface APMServiceContextValue { serviceName: string; @@ -27,12 +28,14 @@ export interface APMServiceContextValue { transactionTypes: string[]; runtimeName?: string; fallbackToTransactions: boolean; + serviceAgentStatus: FETCH_STATUS; } export const APMServiceContext = createContext({ serviceName: '', transactionTypes: [], fallbackToTransactions: false, + serviceAgentStatus: FETCH_STATUS.NOT_INITIATED, }); export function ApmServiceContextProvider({ @@ -50,7 +53,11 @@ export function ApmServiceContextProvider({ const { start, end } = useTimeRange({ rangeFrom, rangeTo }); - const { agentName, runtimeName } = useServiceAgentFetcher({ + const { + agentName, + runtimeName, + status: serviceAgentStatus, + } = useServiceAgentFetcher({ serviceName, start, end, @@ -82,6 +89,7 @@ export function ApmServiceContextProvider({ transactionTypes, runtimeName, fallbackToTransactions, + serviceAgentStatus, }} children={children} /> diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.test.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.test.tsx index 279124cbcb88c..a2abc1bd66214 100644 --- a/x-pack/plugins/apm/public/hooks/use_fetcher.test.tsx +++ b/x-pack/plugins/apm/public/hooks/use_fetcher.test.tsx @@ -10,7 +10,12 @@ import React, { ReactNode } from 'react'; import { CoreStart } from '@kbn/core/public'; import { createKibanaReactContext } from '@kbn/kibana-react-plugin/public'; import { delay } from '../utils/test_helpers'; -import { FetcherResult, useFetcher } from './use_fetcher'; +import { + FetcherResult, + useFetcher, + isPending, + FETCH_STATUS, +} from './use_fetcher'; // Wrap the hook with a provider so it can useKibana const KibanaReactContext = createKibanaReactContext({ @@ -223,4 +228,18 @@ describe('useFetcher', () => { expect(secondResult === thirdResult).toEqual(false); }); }); + + describe('isPending', () => { + [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].forEach((status) => { + it(`returns true when ${status}`, () => { + expect(isPending(status)).toBeTruthy(); + }); + }); + + [FETCH_STATUS.FAILURE, FETCH_STATUS.SUCCESS].forEach((status) => { + it(`returns false when ${status}`, () => { + expect(isPending(status)).toBeFalsy(); + }); + }); + }); }); diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx index 66479aed8a535..80f2451863b42 100644 --- a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx @@ -26,6 +26,10 @@ export enum FETCH_STATUS { NOT_INITIATED = 'not_initiated', } +export const isPending = (fetchStatus: FETCH_STATUS) => + fetchStatus === FETCH_STATUS.LOADING || + fetchStatus === FETCH_STATUS.NOT_INITIATED; + export interface FetcherResult { data?: Data; status: FETCH_STATUS; From 0f60abcfdf6f0311b7c512cb64c0ec3c7c0e182d Mon Sep 17 00:00:00 2001 From: Alexi Doak <109488926+doakalexi@users.noreply.github.com> Date: Tue, 29 Nov 2022 13:44:20 -0500 Subject: [PATCH 5/6] [ResponseOps][Flapping] detect flapping start / end, set flapping state (#144415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves https://github.com/elastic/kibana/issues/143443 ## Summary Added processing to track flapping state, and detect when an alert is flapping. When an alert is determined to be flapping we will log a message for now. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### To verify - Create a rule that will change between active and recovered. I like to create a rule like `index threshold` where I can force the state to change. - Look at the task manager document for this rule and verify that the `flappingHistory` in the `state` is being updated properly. - Let the rule run and verify that if the alert is flapping in AAD, Event Log and alert summary. Co-authored-by: Patrick Mueller Co-authored-by: Mike Côté --- .../plugins/alerting/common/alert_instance.ts | 6 + .../alerting/common/rule_task_instance.ts | 3 + .../alerting/server/alert/alert.test.ts | 93 +- x-pack/plugins/alerting/server/alert/alert.ts | 39 +- .../server/alert/create_alert_factory.test.ts | 51 +- .../server/alert/create_alert_factory.ts | 26 +- .../lib/determine_alerts_to_return.test.ts | 35 + .../server/lib/determine_alerts_to_return.ts | 48 + .../server/lib/flapping_utils.test.ts | 91 ++ .../alerting/server/lib/flapping_utils.ts | 35 + x-pack/plugins/alerting/server/lib/index.ts | 3 + .../server/lib/process_alerts.test.ts | 911 +++++++++++++++--- .../alerting/server/lib/process_alerts.ts | 93 +- .../alerting/server/lib/set_flapping.test.ts | 166 ++++ .../alerting/server/lib/set_flapping.ts | 44 + .../alerting/server/task_runner/fixtures.ts | 8 +- .../server/task_runner/log_alerts.test.ts | 82 ++ .../alerting/server/task_runner/log_alerts.ts | 8 +- .../server/task_runner/task_runner.test.ts | 21 +- .../server/task_runner/task_runner.ts | 42 +- .../metric_threshold_executor.test.ts | 5 + .../utils/create_lifecycle_executor.test.ts | 836 +++++++++++++++- .../server/utils/create_lifecycle_executor.ts | 63 +- .../utils/create_lifecycle_rule_type.test.ts | 2 + .../get_updated_flapping_history.test.ts | 121 +++ .../utils/get_updated_flapping_history.ts | 50 + .../spaces_only/tests/alerting/event_log.ts | 147 +++ .../tests/alerting/event_log_alerts.ts | 7 +- .../tests/alerting/flapping_history.ts | 226 +++++ .../spaces_only/tests/alerting/index.ts | 1 + .../tests/trial/get_summarized_alerts.ts | 2 +- 31 files changed, 3027 insertions(+), 238 deletions(-) create mode 100644 x-pack/plugins/alerting/server/lib/determine_alerts_to_return.test.ts create mode 100644 x-pack/plugins/alerting/server/lib/determine_alerts_to_return.ts create mode 100644 x-pack/plugins/alerting/server/lib/flapping_utils.test.ts create mode 100644 x-pack/plugins/alerting/server/lib/flapping_utils.ts create mode 100644 x-pack/plugins/alerting/server/lib/set_flapping.test.ts create mode 100644 x-pack/plugins/alerting/server/lib/set_flapping.ts create mode 100644 x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.test.ts create mode 100644 x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.ts create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/alerting/flapping_history.ts diff --git a/x-pack/plugins/alerting/common/alert_instance.ts b/x-pack/plugins/alerting/common/alert_instance.ts index e835e7b172ca1..115ff9319e416 100644 --- a/x-pack/plugins/alerting/common/alert_instance.ts +++ b/x-pack/plugins/alerting/common/alert_instance.ts @@ -18,6 +18,12 @@ const metaSchema = t.partial({ date: DateFromString, }), ]), + // an array used to track changes in alert state, the order is based on the rule executions (oldest to most recent) + // true - alert has changed from active/recovered + // false - the status has remained either active or recovered + flappingHistory: t.array(t.boolean), + // flapping flag that indicates whether the alert is flapping + flapping: t.boolean, }); export type AlertInstanceMeta = t.TypeOf; diff --git a/x-pack/plugins/alerting/common/rule_task_instance.ts b/x-pack/plugins/alerting/common/rule_task_instance.ts index 77537bafc4bb9..a732f6d9e2ec8 100644 --- a/x-pack/plugins/alerting/common/rule_task_instance.ts +++ b/x-pack/plugins/alerting/common/rule_task_instance.ts @@ -16,7 +16,10 @@ export enum ActionsCompletion { export const ruleStateSchema = t.partial({ alertTypeState: t.record(t.string, t.unknown), + // tracks the active alerts alertInstances: t.record(t.string, rawAlertInstance), + // tracks the recovered alerts for flapping purposes + alertRecoveredInstances: t.record(t.string, rawAlertInstance), previousStartedAt: t.union([t.null, DateFromString]), }); diff --git a/x-pack/plugins/alerting/server/alert/alert.test.ts b/x-pack/plugins/alerting/server/alert/alert.test.ts index d9e05e55a67fd..e74b103a99880 100644 --- a/x-pack/plugins/alerting/server/alert/alert.test.ts +++ b/x-pack/plugins/alerting/server/alert/alert.test.ts @@ -252,6 +252,7 @@ describe('updateLastScheduledActions()', () => { date: new Date().toISOString(), group: 'default', }, + flappingHistory: [], }, }); }); @@ -340,11 +341,13 @@ describe('toJSON', () => { date: new Date(), group: 'default', }, + flappingHistory: [false, true], + flapping: false, }, } ); expect(JSON.stringify(alertInstance)).toEqual( - '{"state":{"foo":true},"meta":{"lastScheduledActions":{"date":"1970-01-01T00:00:00.000Z","group":"default"}}}' + '{"state":{"foo":true},"meta":{"lastScheduledActions":{"date":"1970-01-01T00:00:00.000Z","group":"default"},"flappingHistory":[false,true],"flapping":false}}' ); }); }); @@ -358,6 +361,7 @@ describe('toRaw', () => { date: new Date(), group: 'default', }, + flappingHistory: [false, true, true], }, }; const alertInstance = new Alert( @@ -366,4 +370,91 @@ describe('toRaw', () => { ); expect(alertInstance.toRaw()).toEqual(raw); }); + + test('returns unserialised underlying partial meta if recovered is true', () => { + const raw = { + state: { foo: true }, + meta: { + lastScheduledActions: { + date: new Date(), + group: 'default', + }, + flappingHistory: [false, true, true], + flapping: false, + }, + }; + const alertInstance = new Alert( + '1', + raw + ); + expect(alertInstance.toRaw(true)).toEqual({ + meta: { + flappingHistory: [false, true, true], + flapping: false, + }, + }); + }); +}); + +describe('setFlappingHistory', () => { + test('sets flappingHistory', () => { + const alertInstance = new Alert( + '1', + { + meta: { flappingHistory: [false, true, true] }, + } + ); + alertInstance.setFlappingHistory([false]); + expect(alertInstance.getFlappingHistory()).toEqual([false]); + expect(alertInstance.toRaw()).toMatchInlineSnapshot(` + Object { + "meta": Object { + "flappingHistory": Array [ + false, + ], + }, + "state": Object {}, + } + `); + }); +}); + +describe('getFlappingHistory', () => { + test('correctly sets flappingHistory', () => { + const alert = new Alert('1', { + meta: { flappingHistory: [false, false] }, + }); + expect(alert.getFlappingHistory()).toEqual([false, false]); + }); +}); + +describe('setFlapping', () => { + test('sets flapping', () => { + const alertInstance = new Alert( + '1', + { + meta: { flapping: true }, + } + ); + alertInstance.setFlapping(false); + expect(alertInstance.getFlapping()).toEqual(false); + expect(alertInstance.toRaw()).toMatchInlineSnapshot(` + Object { + "meta": Object { + "flapping": false, + "flappingHistory": Array [], + }, + "state": Object {}, + } + `); + }); +}); + +describe('getFlapping', () => { + test('correctly sets flapping', () => { + const alert = new Alert('1', { + meta: { flapping: true }, + }); + expect(alert.getFlapping()).toEqual(true); + }); }); diff --git a/x-pack/plugins/alerting/server/alert/alert.ts b/x-pack/plugins/alerting/server/alert/alert.ts index e24b15de41db4..6c56abdf96624 100644 --- a/x-pack/plugins/alerting/server/alert/alert.ts +++ b/x-pack/plugins/alerting/server/alert/alert.ts @@ -52,6 +52,10 @@ export class Alert< this.state = (state || {}) as State; this.context = {} as Context; this.meta = meta; + + if (!this.meta.flappingHistory) { + this.meta.flappingHistory = []; + } } getId() { @@ -168,10 +172,35 @@ export class Alert< return rawAlertInstance.encode(this.toRaw()); } - toRaw(): RawAlertInstance { - return { - state: this.state, - meta: this.meta, - }; + toRaw(recovered: boolean = false): RawAlertInstance { + return recovered + ? { + // for a recovered alert, we only care to track the flappingHistory + // and the flapping flag + meta: { + flappingHistory: this.meta.flappingHistory, + flapping: this.meta.flapping, + }, + } + : { + state: this.state, + meta: this.meta, + }; + } + + setFlappingHistory(fh: boolean[] = []) { + this.meta.flappingHistory = fh; + } + + getFlappingHistory() { + return this.meta.flappingHistory; + } + + setFlapping(f: boolean) { + this.meta.flapping = f; + } + + getFlapping() { + return this.meta.flapping || false; } } diff --git a/x-pack/plugins/alerting/server/alert/create_alert_factory.test.ts b/x-pack/plugins/alerting/server/alert/create_alert_factory.test.ts index 30013e212c526..663a0b7401b2e 100644 --- a/x-pack/plugins/alerting/server/alert/create_alert_factory.test.ts +++ b/x-pack/plugins/alerting/server/alert/create_alert_factory.test.ts @@ -33,11 +33,13 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toMatchInlineSnapshot(` - Object { - "meta": Object {}, - "state": Object {}, - } - `); + Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object {}, + } + `); // @ts-expect-error expect(result.getId()).toEqual('1'); }); @@ -58,6 +60,7 @@ describe('createAlertFactory()', () => { expect(result).toMatchInlineSnapshot(` Object { "meta": Object { + "flappingHistory": Array [], "lastScheduledActions": Object { "date": "1970-01-01T00:00:00.000Z", "group": "default", @@ -79,13 +82,15 @@ describe('createAlertFactory()', () => { }); alertFactory.create('1'); expect(alerts).toMatchInlineSnapshot(` - Object { - "1": Object { - "meta": Object {}, - "state": Object {}, - }, - } - `); + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object {}, + }, + } + `); }); test('throws error and sets flag when more alerts are created than allowed', () => { @@ -115,7 +120,9 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toEqual({ - meta: {}, + meta: { + flappingHistory: [], + }, state: {}, context: {}, scheduledExecutionOptions: undefined, @@ -133,7 +140,7 @@ describe('createAlertFactory()', () => { test('returns recovered alerts when setsRecoveryContext is true', () => { (processAlerts as jest.Mock).mockReturnValueOnce({ - recoveredAlerts: { + currentRecoveredAlerts: { z: { id: 'z', state: { foo: true }, @@ -154,7 +161,9 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toEqual({ - meta: {}, + meta: { + flappingHistory: [], + }, state: {}, context: {}, scheduledExecutionOptions: undefined, @@ -178,7 +187,9 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toEqual({ - meta: {}, + meta: { + flappingHistory: [], + }, state: {}, context: {}, scheduledExecutionOptions: undefined, @@ -201,7 +212,9 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toEqual({ - meta: {}, + meta: { + flappingHistory: [], + }, state: {}, context: {}, scheduledExecutionOptions: undefined, @@ -223,7 +236,9 @@ describe('createAlertFactory()', () => { }); const result = alertFactory.create('1'); expect(result).toEqual({ - meta: {}, + meta: { + flappingHistory: [], + }, state: {}, context: {}, scheduledExecutionOptions: undefined, diff --git a/x-pack/plugins/alerting/server/alert/create_alert_factory.ts b/x-pack/plugins/alerting/server/alert/create_alert_factory.ts index e0d3ecc2690fc..ff8aacd52f5fe 100644 --- a/x-pack/plugins/alerting/server/alert/create_alert_factory.ts +++ b/x-pack/plugins/alerting/server/alert/create_alert_factory.ts @@ -129,16 +129,22 @@ export function createAlertFactory< return []; } - const { recoveredAlerts } = processAlerts( - { - alerts, - existingAlerts: originalAlerts, - hasReachedAlertLimit, - alertLimit: maxAlerts, - } - ); - return Object.keys(recoveredAlerts ?? {}).map( - (alertId: string) => recoveredAlerts[alertId] + const { currentRecoveredAlerts } = processAlerts< + State, + Context, + ActionGroupIds, + ActionGroupIds + >({ + alerts, + existingAlerts: originalAlerts, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit, + alertLimit: maxAlerts, + // setFlapping is false, as we only want to use this function to get the recovered alerts + setFlapping: false, + }); + return Object.keys(currentRecoveredAlerts ?? {}).map( + (alertId: string) => currentRecoveredAlerts[alertId] ); }, }; diff --git a/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.test.ts b/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.test.ts new file mode 100644 index 0000000000000..4fd80033fd341 --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.test.ts @@ -0,0 +1,35 @@ +/* + * 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 { keys, size } from 'lodash'; +import { Alert } from '../alert'; +import { determineAlertsToReturn } from './determine_alerts_to_return'; + +describe('determineAlertsToReturn', () => { + const flapping = new Array(16).fill(false).concat([true, true, true, true]); + const notFlapping = new Array(20).fill(false); + + describe('determineAlertsToReturn', () => { + test('should return all active alerts regardless of flapping', () => { + const activeAlerts = { + '1': new Alert('1', { meta: { flappingHistory: flapping } }), + '2': new Alert('2', { meta: { flappingHistory: [false, false] } }), + }; + const { alertsToReturn } = determineAlertsToReturn(activeAlerts, {}); + expect(size(alertsToReturn)).toEqual(2); + }); + + test('should return all flapping recovered alerts', () => { + const recoveredAlerts = { + '1': new Alert('1', { meta: { flappingHistory: flapping } }), + '2': new Alert('2', { meta: { flappingHistory: notFlapping } }), + }; + const { recoveredAlertsToReturn } = determineAlertsToReturn({}, recoveredAlerts); + expect(keys(recoveredAlertsToReturn)).toEqual(['1']); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.ts b/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.ts new file mode 100644 index 0000000000000..5916bf91efcc0 --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/determine_alerts_to_return.ts @@ -0,0 +1,48 @@ +/* + * 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 { keys } from 'lodash'; +import { Alert } from '../alert'; +import { AlertInstanceState, AlertInstanceContext, RawAlertInstance } from '../types'; + +// determines which alerts to return in the state +export function determineAlertsToReturn< + State extends AlertInstanceState, + Context extends AlertInstanceContext, + ActionGroupIds extends string, + RecoveryActionGroupId extends string +>( + activeAlerts: Record> = {}, + recoveredAlerts: Record> = {} +): { + alertsToReturn: Record; + recoveredAlertsToReturn: Record; +} { + const alertsToReturn: Record = {}; + const recoveredAlertsToReturn: Record = {}; + + // return all active alerts regardless of whether or not the alert is flapping + for (const id of keys(activeAlerts)) { + alertsToReturn[id] = activeAlerts[id].toRaw(); + } + + for (const id of keys(recoveredAlerts)) { + const alert = recoveredAlerts[id]; + // return recovered alerts if they are flapping or if the flapping array is not at capacity + // this is a space saving effort that will stop tracking a recovered alert if it wasn't flapping and doesn't have state changes + // in the last max capcity number of executions + const flapping = alert.getFlapping(); + const flappingHistory: boolean[] = alert.getFlappingHistory() || []; + const numStateChanges = flappingHistory.filter((f) => f).length; + if (flapping) { + recoveredAlertsToReturn[id] = alert.toRaw(true); + } else if (numStateChanges > 0) { + recoveredAlertsToReturn[id] = alert.toRaw(true); + } + } + return { alertsToReturn, recoveredAlertsToReturn }; +} diff --git a/x-pack/plugins/alerting/server/lib/flapping_utils.test.ts b/x-pack/plugins/alerting/server/lib/flapping_utils.test.ts new file mode 100644 index 0000000000000..ee5525634cf4f --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/flapping_utils.test.ts @@ -0,0 +1,91 @@ +/* + * 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 { atCapacity, updateFlappingHistory, isFlapping } from './flapping_utils'; + +describe('flapping utils', () => { + describe('updateFlappingHistory function', () => { + test('correctly updates flappingHistory', () => { + const flappingHistory = updateFlappingHistory([false, false], true); + expect(flappingHistory).toEqual([false, false, true]); + }); + + test('correctly updates flappingHistory while maintaining a fixed size', () => { + const flappingHistory = new Array(20).fill(false); + const fh = updateFlappingHistory(flappingHistory, true); + expect(fh.length).toEqual(20); + const result = new Array(19).fill(false); + expect(fh).toEqual(result.concat(true)); + }); + + test('correctly updates flappingHistory while maintaining if array is larger than fixed size', () => { + const flappingHistory = new Array(23).fill(false); + const fh = updateFlappingHistory(flappingHistory, true); + expect(fh.length).toEqual(20); + const result = new Array(19).fill(false); + expect(fh).toEqual(result.concat(true)); + }); + }); + + describe('atCapacity and getCapacityDiff functions', () => { + test('returns true if flappingHistory == set capacity', () => { + const flappingHistory = new Array(20).fill(false); + expect(atCapacity(flappingHistory)).toEqual(true); + }); + + test('returns true if flappingHistory > set capacity', () => { + const flappingHistory = new Array(25).fill(false); + expect(atCapacity(flappingHistory)).toEqual(true); + }); + + test('returns false if flappingHistory < set capacity', () => { + const flappingHistory = new Array(15).fill(false); + expect(atCapacity(flappingHistory)).toEqual(false); + }); + }); + + describe('isFlapping', () => { + describe('not currently flapping', () => { + test('returns true if at capacity and flap count exceeds the threshold', () => { + const flappingHistory = [true, true, true, true].concat(new Array(16).fill(false)); + expect(isFlapping(flappingHistory)).toEqual(true); + }); + + test("returns false if at capacity and flap count doesn't exceed the threshold", () => { + const flappingHistory = [true, true].concat(new Array(20).fill(false)); + expect(isFlapping(flappingHistory)).toEqual(false); + }); + + test('returns true if not at capacity', () => { + const flappingHistory = new Array(5).fill(true); + expect(isFlapping(flappingHistory)).toEqual(true); + }); + }); + + describe('currently flapping', () => { + test('returns true if at capacity and the flap count exceeds the threshold', () => { + const flappingHistory = new Array(16).fill(false).concat([true, true, true, true]); + expect(isFlapping(flappingHistory, true)).toEqual(true); + }); + + test("returns true if not at capacity and the flap count doesn't exceed the threshold", () => { + const flappingHistory = new Array(16).fill(false); + expect(isFlapping(flappingHistory, true)).toEqual(true); + }); + + test('returns true if not at capacity and the flap count exceeds the threshold', () => { + const flappingHistory = new Array(10).fill(false).concat([true, true, true, true]); + expect(isFlapping(flappingHistory, true)).toEqual(true); + }); + + test("returns false if at capacity and the flap count doesn't exceed the threshold", () => { + const flappingHistory = new Array(20).fill(false); + expect(isFlapping(flappingHistory, true)).toEqual(false); + }); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/lib/flapping_utils.ts b/x-pack/plugins/alerting/server/lib/flapping_utils.ts new file mode 100644 index 0000000000000..8427e02880844 --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/flapping_utils.ts @@ -0,0 +1,35 @@ +/* + * 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. + */ + +const MAX_CAPACITY = 20; +const MAX_FLAP_COUNT = 4; + +export function updateFlappingHistory(flappingHistory: boolean[], state: boolean) { + const updatedFlappingHistory = flappingHistory.concat(state).slice(MAX_CAPACITY * -1); + return updatedFlappingHistory; +} + +export function isFlapping( + flappingHistory: boolean[], + isCurrentlyFlapping: boolean = false +): boolean { + const numStateChanges = flappingHistory.filter((f) => f).length; + if (isCurrentlyFlapping) { + // if an alert is currently flapping, + // it will return false if the flappingHistory array is at capacity and there are 0 state changes + // else it will return true + return !(atCapacity(flappingHistory) && numStateChanges === 0); + } else { + // if an alert is not currently flapping, + // it will return true if the number of state changes in flappingHistory array >= the max flapping count + return numStateChanges >= MAX_FLAP_COUNT; + } +} + +export function atCapacity(flappingHistory: boolean[] = []): boolean { + return flappingHistory.length >= MAX_CAPACITY; +} diff --git a/x-pack/plugins/alerting/server/lib/index.ts b/x-pack/plugins/alerting/server/lib/index.ts index de4c89e06f33d..8b1416df007fa 100644 --- a/x-pack/plugins/alerting/server/lib/index.ts +++ b/x-pack/plugins/alerting/server/lib/index.ts @@ -38,3 +38,6 @@ export { isRuleSnoozed, getRuleSnoozeEndTime } from './is_rule_snoozed'; export { convertRuleIdsToKueryNode } from './convert_rule_ids_to_kuery_node'; export { convertEsSortToEventLogSort } from './convert_es_sort_to_event_log_sort'; export * from './snooze'; +export { setFlapping } from './set_flapping'; +export { determineAlertsToReturn } from './determine_alerts_to_return'; +export { updateFlappingHistory, isFlapping } from './flapping_utils'; diff --git a/x-pack/plugins/alerting/server/lib/process_alerts.test.ts b/x-pack/plugins/alerting/server/lib/process_alerts.test.ts index dcf13bdc4f7a1..3b09b072476e6 100644 --- a/x-pack/plugins/alerting/server/lib/process_alerts.test.ts +++ b/x-pack/plugins/alerting/server/lib/process_alerts.test.ts @@ -7,9 +7,9 @@ import sinon from 'sinon'; import { cloneDeep } from 'lodash'; -import { processAlerts } from './process_alerts'; +import { processAlerts, updateAlertFlappingHistory } from './process_alerts'; import { Alert } from '../alert'; -import { DefaultActionGroupId } from '../types'; +import { AlertInstanceState, AlertInstanceContext } from '../types'; describe('processAlerts', () => { let clock: sinon.SinonFakeTimers; @@ -25,42 +25,47 @@ describe('processAlerts', () => { afterAll(() => clock.restore()); describe('newAlerts', () => { - test('considers alert new if it has scheduled actions and its id is not in originalAlertIds list', () => { - const newAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + test('considers alert new if it has scheduled actions and its id is not in originalAlertIds or previouslyRecoveredAlertIds list', () => { + const newAlert = new Alert('1'); + const existingAlert1 = new Alert('2'); + const existingAlert2 = new Alert('3', {}); + const existingRecoveredAlert1 = new Alert('4'); const existingAlerts = { '2': existingAlert1, '3': existingAlert2, }; + const previouslyRecoveredAlerts = { + '4': existingRecoveredAlert1, + }; + const updatedAlerts = { ...cloneDeep(existingAlerts), '1': newAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); const { newAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(newAlerts).toEqual({ '1': newAlert }); }); test('sets start time in new alert state', () => { - const newAlert1 = new Alert<{}, {}, DefaultActionGroupId>('1'); - const newAlert2 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('3'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('4'); + const newAlert1 = new Alert('1'); + const newAlert2 = new Alert('2'); + const existingAlert1 = new Alert('3'); + const existingAlert2 = new Alert('4'); const existingAlerts = { '3': existingAlert1, @@ -73,21 +78,21 @@ describe('processAlerts', () => { '2': newAlert2, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '1' }); - updatedAlerts['4'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['4'].scheduleActions('default' as never, { foo: '2' }); expect(newAlert1.getState()).toStrictEqual({}); expect(newAlert2.getState()).toStrictEqual({}); const { newAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(newAlerts).toEqual({ '1': newAlert1, '2': newAlert2 }); @@ -95,28 +100,22 @@ describe('processAlerts', () => { const newAlert1State = newAlerts['1'].getState(); const newAlert2State = newAlerts['2'].getState(); - // @ts-expect-error expect(newAlert1State.start).toEqual('1970-01-01T00:00:00.000Z'); - // @ts-expect-error expect(newAlert2State.start).toEqual('1970-01-01T00:00:00.000Z'); - // @ts-expect-error expect(newAlert1State.duration).toEqual('0'); - // @ts-expect-error expect(newAlert2State.duration).toEqual('0'); - // @ts-expect-error expect(newAlert1State.end).not.toBeDefined(); - // @ts-expect-error expect(newAlert2State.end).not.toBeDefined(); }); }); describe('activeAlerts', () => { test('considers alert active if it has scheduled actions', () => { - const newAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const newAlert = new Alert('1'); + const existingAlert1 = new Alert('2'); + const existingAlert2 = new Alert('3'); const existingAlerts = { '2': existingAlert1, @@ -128,17 +127,17 @@ describe('processAlerts', () => { '1': newAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); const { activeAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(activeAlerts).toEqual({ @@ -149,9 +148,9 @@ describe('processAlerts', () => { }); test('updates duration in active alerts if start is available', () => { - const newAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const newAlert = new Alert('1'); + const existingAlert1 = new Alert('2'); + const existingAlert2 = new Alert('3'); const existingAlerts = { '2': existingAlert1, @@ -165,17 +164,17 @@ describe('processAlerts', () => { '1': newAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); const { activeAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(activeAlerts).toEqual({ @@ -187,26 +186,20 @@ describe('processAlerts', () => { const activeAlert1State = activeAlerts['2'].getState(); const activeAlert2State = activeAlerts['3'].getState(); - // @ts-expect-error expect(activeAlert1State.start).toEqual('1969-12-30T00:00:00.000Z'); - // @ts-expect-error expect(activeAlert2State.start).toEqual('1969-12-31T07:34:00.000Z'); - // @ts-expect-error expect(activeAlert1State.duration).toEqual('172800000000000'); - // @ts-expect-error expect(activeAlert2State.duration).toEqual('59160000000000'); - // @ts-expect-error expect(activeAlert1State.end).not.toBeDefined(); - // @ts-expect-error expect(activeAlert2State.end).not.toBeDefined(); }); test('does not update duration in active alerts if start is not available', () => { - const newAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const newAlert = new Alert('1'); + const existingAlert1 = new Alert('2'); + const existingAlert2 = new Alert('3'); const existingAlerts = { '2': existingAlert1, @@ -218,17 +211,17 @@ describe('processAlerts', () => { '1': newAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); const { activeAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(activeAlerts).toEqual({ @@ -240,26 +233,20 @@ describe('processAlerts', () => { const activeAlert1State = activeAlerts['2'].getState(); const activeAlert2State = activeAlerts['3'].getState(); - // @ts-expect-error expect(activeAlert1State.start).not.toBeDefined(); - // @ts-expect-error expect(activeAlert2State.start).not.toBeDefined(); - // @ts-expect-error expect(activeAlert1State.duration).not.toBeDefined(); - // @ts-expect-error expect(activeAlert2State.duration).not.toBeDefined(); - // @ts-expect-error expect(activeAlert1State.end).not.toBeDefined(); - // @ts-expect-error expect(activeAlert2State.end).not.toBeDefined(); }); test('preserves other state fields', () => { - const newAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const newAlert = new Alert('1'); + const existingAlert1 = new Alert('2'); + const existingAlert2 = new Alert('3'); const existingAlerts = { '2': existingAlert1, @@ -281,17 +268,17 @@ describe('processAlerts', () => { '1': newAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); const { activeAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(activeAlerts).toEqual({ @@ -303,32 +290,79 @@ describe('processAlerts', () => { const activeAlert1State = activeAlerts['2'].getState(); const activeAlert2State = activeAlerts['3'].getState(); - // @ts-expect-error expect(activeAlert1State.start).toEqual('1969-12-30T00:00:00.000Z'); - // @ts-expect-error expect(activeAlert2State.start).toEqual('1969-12-31T07:34:00.000Z'); - // @ts-expect-error expect(activeAlert1State.stateField1).toEqual('xyz'); - // @ts-expect-error expect(activeAlert2State.anotherState).toEqual(true); - // @ts-expect-error expect(activeAlert1State.duration).toEqual('172800000000000'); - // @ts-expect-error expect(activeAlert2State.duration).toEqual('59160000000000'); - // @ts-expect-error expect(activeAlert1State.end).not.toBeDefined(); - // @ts-expect-error expect(activeAlert2State.end).not.toBeDefined(); }); + + test('sets start time in active alert state if alert was previously recovered', () => { + const previouslyRecoveredAlert1 = new Alert('1'); + const previouslyRecoveredAlert2 = new Alert('2'); + const existingAlert1 = new Alert('3'); + const existingAlert2 = new Alert('4'); + + const existingAlerts = { + '3': existingAlert1, + '4': existingAlert2, + }; + + const previouslyRecoveredAlerts = { + '1': previouslyRecoveredAlert1, + '2': previouslyRecoveredAlert2, + }; + + const updatedAlerts = { + ...cloneDeep(existingAlerts), + ...cloneDeep(previouslyRecoveredAlerts), + }; + + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['4'].scheduleActions('default' as never, { foo: '2' }); + + expect(updatedAlerts['1'].getState()).toStrictEqual({}); + expect(updatedAlerts['2'].getState()).toStrictEqual({}); + + const { activeAlerts } = processAlerts({ + alerts: updatedAlerts, + existingAlerts, + previouslyRecoveredAlerts, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect( + Object.keys(activeAlerts).map((id) => ({ [id]: activeAlerts[id].getFlappingHistory() })) + ).toEqual([{ '1': [true] }, { '2': [true] }, { '3': [false] }, { '4': [false] }]); + + const previouslyRecoveredAlert1State = activeAlerts['1'].getState(); + const previouslyRecoveredAlert2State = activeAlerts['2'].getState(); + + expect(previouslyRecoveredAlert1State.start).toEqual('1970-01-01T00:00:00.000Z'); + expect(previouslyRecoveredAlert2State.start).toEqual('1970-01-01T00:00:00.000Z'); + + expect(previouslyRecoveredAlert1State.duration).toEqual('0'); + expect(previouslyRecoveredAlert2State.duration).toEqual('0'); + + expect(previouslyRecoveredAlert1State.end).not.toBeDefined(); + expect(previouslyRecoveredAlert2State.end).not.toBeDefined(); + }); }); describe('recoveredAlerts', () => { test('considers alert recovered if it has no scheduled actions', () => { - const activeAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const recoveredAlert = new Alert<{}, {}, DefaultActionGroupId>('2'); + const activeAlert = new Alert('1'); + const recoveredAlert = new Alert('2'); const existingAlerts = { '1': activeAlert, @@ -337,24 +371,24 @@ describe('processAlerts', () => { const updatedAlerts = cloneDeep(existingAlerts); - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); updatedAlerts['2'].setContext({ foo: '2' }); const { recoveredAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(recoveredAlerts).toEqual({ '2': updatedAlerts['2'] }); }); test('does not consider alert recovered if it has no actions but was not in original alerts list', () => { - const activeAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const notRecoveredAlert = new Alert<{}, {}, DefaultActionGroupId>('2'); + const activeAlert = new Alert('1'); + const notRecoveredAlert = new Alert('2'); const existingAlerts = { '1': activeAlert, @@ -365,24 +399,24 @@ describe('processAlerts', () => { '2': notRecoveredAlert, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); const { recoveredAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(recoveredAlerts).toEqual({}); }); test('updates duration in recovered alerts if start is available and adds end time', () => { - const activeAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const recoveredAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const recoveredAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const activeAlert = new Alert('1'); + const recoveredAlert1 = new Alert('2'); + const recoveredAlert2 = new Alert('3'); const existingAlerts = { '1': activeAlert, @@ -394,15 +428,15 @@ describe('processAlerts', () => { const updatedAlerts = cloneDeep(existingAlerts); - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); const { recoveredAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(recoveredAlerts).toEqual({ '2': updatedAlerts['2'], '3': updatedAlerts['3'] }); @@ -410,26 +444,20 @@ describe('processAlerts', () => { const recoveredAlert1State = recoveredAlerts['2'].getState(); const recoveredAlert2State = recoveredAlerts['3'].getState(); - // @ts-expect-error expect(recoveredAlert1State.start).toEqual('1969-12-30T00:00:00.000Z'); - // @ts-expect-error expect(recoveredAlert2State.start).toEqual('1969-12-31T07:34:00.000Z'); - // @ts-expect-error expect(recoveredAlert1State.duration).toEqual('172800000000000'); - // @ts-expect-error expect(recoveredAlert2State.duration).toEqual('59160000000000'); - // @ts-expect-error expect(recoveredAlert1State.end).toEqual('1970-01-01T00:00:00.000Z'); - // @ts-expect-error expect(recoveredAlert2State.end).toEqual('1970-01-01T00:00:00.000Z'); }); test('does not update duration or set end in recovered alerts if start is not available', () => { - const activeAlert = new Alert<{}, {}, DefaultActionGroupId>('1'); - const recoveredAlert1 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const recoveredAlert2 = new Alert<{}, {}, DefaultActionGroupId>('3'); + const activeAlert = new Alert('1'); + const recoveredAlert1 = new Alert('2'); + const recoveredAlert2 = new Alert('3'); const existingAlerts = { '1': activeAlert, @@ -438,15 +466,15 @@ describe('processAlerts', () => { }; const updatedAlerts = cloneDeep(existingAlerts); - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); const { recoveredAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: false, alertLimit: 10, + setFlapping: false, }); expect(recoveredAlerts).toEqual({ '2': updatedAlerts['2'], '3': updatedAlerts['3'] }); @@ -454,32 +482,52 @@ describe('processAlerts', () => { const recoveredAlert1State = recoveredAlerts['2'].getState(); const recoveredAlert2State = recoveredAlerts['3'].getState(); - // @ts-expect-error expect(recoveredAlert1State.start).not.toBeDefined(); - // @ts-expect-error expect(recoveredAlert2State.start).not.toBeDefined(); - // @ts-expect-error expect(recoveredAlert1State.duration).not.toBeDefined(); - // @ts-expect-error expect(recoveredAlert2State.duration).not.toBeDefined(); - // @ts-expect-error expect(recoveredAlert1State.end).not.toBeDefined(); - // @ts-expect-error expect(recoveredAlert2State.end).not.toBeDefined(); }); + + test('considers alert recovered if it was previously recovered and not active', () => { + const recoveredAlert1 = new Alert('1'); + const recoveredAlert2 = new Alert('2'); + + const previouslyRecoveredAlerts = { + '1': recoveredAlert1, + '2': recoveredAlert2, + }; + + const updatedAlerts = cloneDeep(previouslyRecoveredAlerts); + + updatedAlerts['1'].setFlappingHistory([false]); + updatedAlerts['2'].setFlappingHistory([false]); + + const { recoveredAlerts } = processAlerts({ + alerts: {}, + existingAlerts: {}, + previouslyRecoveredAlerts, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(recoveredAlerts).toEqual(updatedAlerts); + }); }); describe('when hasReachedAlertLimit is true', () => { test('does not calculate recovered alerts', () => { - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert3 = new Alert<{}, {}, DefaultActionGroupId>('3'); - const existingAlert4 = new Alert<{}, {}, DefaultActionGroupId>('4'); - const existingAlert5 = new Alert<{}, {}, DefaultActionGroupId>('5'); - const newAlert6 = new Alert<{}, {}, DefaultActionGroupId>('6'); - const newAlert7 = new Alert<{}, {}, DefaultActionGroupId>('7'); + const existingAlert1 = new Alert('1'); + const existingAlert2 = new Alert('2'); + const existingAlert3 = new Alert('3'); + const existingAlert4 = new Alert('4'); + const existingAlert5 = new Alert('5'); + const newAlert6 = new Alert('6'); + const newAlert7 = new Alert('7'); const existingAlerts = { '1': existingAlert1, @@ -495,32 +543,32 @@ describe('processAlerts', () => { '7': newAlert7, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); - updatedAlerts['4'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['4'].scheduleActions('default' as never, { foo: '2' }); // intentionally not scheduling actions for alert "5" - updatedAlerts['6'].scheduleActions('default', { foo: '2' }); - updatedAlerts['7'].scheduleActions('default', { foo: '2' }); + updatedAlerts['6'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['7'].scheduleActions('default' as never, { foo: '2' }); const { recoveredAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: true, alertLimit: 7, + setFlapping: false, }); expect(recoveredAlerts).toEqual({}); }); test('persists existing alerts', () => { - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert3 = new Alert<{}, {}, DefaultActionGroupId>('3'); - const existingAlert4 = new Alert<{}, {}, DefaultActionGroupId>('4'); - const existingAlert5 = new Alert<{}, {}, DefaultActionGroupId>('5'); + const existingAlert1 = new Alert('1'); + const existingAlert2 = new Alert('2'); + const existingAlert3 = new Alert('3'); + const existingAlert4 = new Alert('4'); + const existingAlert5 = new Alert('5'); const existingAlerts = { '1': existingAlert1, @@ -532,19 +580,19 @@ describe('processAlerts', () => { const updatedAlerts = cloneDeep(existingAlerts); - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); - updatedAlerts['4'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['4'].scheduleActions('default' as never, { foo: '2' }); // intentionally not scheduling actions for alert "5" const { activeAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: true, alertLimit: 7, + setFlapping: false, }); expect(activeAlerts).toEqual({ @@ -558,16 +606,16 @@ describe('processAlerts', () => { test('adds new alerts up to max allowed', () => { const MAX_ALERTS = 7; - const existingAlert1 = new Alert<{}, {}, DefaultActionGroupId>('1'); - const existingAlert2 = new Alert<{}, {}, DefaultActionGroupId>('2'); - const existingAlert3 = new Alert<{}, {}, DefaultActionGroupId>('3'); - const existingAlert4 = new Alert<{}, {}, DefaultActionGroupId>('4'); - const existingAlert5 = new Alert<{}, {}, DefaultActionGroupId>('5'); - const newAlert6 = new Alert<{}, {}, DefaultActionGroupId>('6'); - const newAlert7 = new Alert<{}, {}, DefaultActionGroupId>('7'); - const newAlert8 = new Alert<{}, {}, DefaultActionGroupId>('8'); - const newAlert9 = new Alert<{}, {}, DefaultActionGroupId>('9'); - const newAlert10 = new Alert<{}, {}, DefaultActionGroupId>('10'); + const existingAlert1 = new Alert('1'); + const existingAlert2 = new Alert('2'); + const existingAlert3 = new Alert('3'); + const existingAlert4 = new Alert('4'); + const existingAlert5 = new Alert('5'); + const newAlert6 = new Alert('6'); + const newAlert7 = new Alert('7'); + const newAlert8 = new Alert('8'); + const newAlert9 = new Alert('9'); + const newAlert10 = new Alert('10'); const existingAlerts = { '1': existingAlert1, @@ -586,24 +634,24 @@ describe('processAlerts', () => { '10': newAlert10, }; - updatedAlerts['1'].scheduleActions('default', { foo: '1' }); - updatedAlerts['2'].scheduleActions('default', { foo: '1' }); - updatedAlerts['3'].scheduleActions('default', { foo: '2' }); - updatedAlerts['4'].scheduleActions('default', { foo: '2' }); + updatedAlerts['1'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['2'].scheduleActions('default' as never, { foo: '1' }); + updatedAlerts['3'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['4'].scheduleActions('default' as never, { foo: '2' }); // intentionally not scheduling actions for alert "5" - updatedAlerts['6'].scheduleActions('default', { foo: '2' }); - updatedAlerts['7'].scheduleActions('default', { foo: '2' }); - updatedAlerts['8'].scheduleActions('default', { foo: '2' }); - updatedAlerts['9'].scheduleActions('default', { foo: '2' }); - updatedAlerts['10'].scheduleActions('default', { foo: '2' }); + updatedAlerts['6'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['7'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['8'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['9'].scheduleActions('default' as never, { foo: '2' }); + updatedAlerts['10'].scheduleActions('default' as never, { foo: '2' }); const { activeAlerts, newAlerts } = processAlerts({ - // @ts-expect-error alerts: updatedAlerts, - // @ts-expect-error existingAlerts, + previouslyRecoveredAlerts: {}, hasReachedAlertLimit: true, alertLimit: MAX_ALERTS, + setFlapping: false, }); expect(Object.keys(activeAlerts).length).toEqual(MAX_ALERTS); @@ -622,4 +670,547 @@ describe('processAlerts', () => { }); }); }); + + describe('updating flappingHistory', () => { + test('if new alert, set flapping state to true', () => { + const activeAlert = new Alert('1'); + + const alerts = cloneDeep({ '1': activeAlert }); + alerts['1'].scheduleActions('default' as never, { foo: '1' }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: {}, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if alert is still active, set flapping state to false', () => { + const activeAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + + const alerts = cloneDeep({ '1': activeAlert }); + alerts['1'].scheduleActions('default' as never, { foo: '1' }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: alerts, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + false, + ], + }, + "state": Object {}, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(`Object {}`); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if alert is active and previously recovered, set flapping state to true', () => { + const activeAlert = new Alert('1'); + const recoveredAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + + const alerts = cloneDeep({ '1': activeAlert }); + alerts['1'].scheduleActions('default' as never, { foo: '1' }); + alerts['1'].setFlappingHistory([false]); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: {}, + previouslyRecoveredAlerts: { '1': recoveredAlert }, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if alert is recovered and previously active, set flapping state to true', () => { + const activeAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + activeAlert.scheduleActions('default' as never, { foo: '1' }); + const recoveredAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + + const alerts = cloneDeep({ '1': recoveredAlert }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: { '1': activeAlert }, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(`Object {}`); + expect(newAlerts).toMatchInlineSnapshot(`Object {}`); + expect(recoveredAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + true, + ], + }, + "state": Object {}, + }, + } + `); + }); + + test('if alert is still recovered, set flapping state to false', () => { + const recoveredAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + + const alerts = cloneDeep({ '1': recoveredAlert }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts: {}, + existingAlerts: {}, + previouslyRecoveredAlerts: alerts, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(`Object {}`); + expect(newAlerts).toMatchInlineSnapshot(`Object {}`); + expect(recoveredAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + false, + ], + }, + "state": Object {}, + }, + } + `); + }); + + test('if setFlapping is false should not update flappingHistory', () => { + const activeAlert1 = new Alert('1'); + activeAlert1.scheduleActions('default' as never, { foo: '1' }); + const activeAlert2 = new Alert('2', { + meta: { flappingHistory: [false] }, + }); + activeAlert2.scheduleActions('default' as never, { foo: '1' }); + const recoveredAlert = new Alert('3', { + meta: { flappingHistory: [false] }, + }); + + const previouslyRecoveredAlerts = cloneDeep({ '3': recoveredAlert }); + const alerts = cloneDeep({ '1': activeAlert1, '2': activeAlert2 }); + const existingAlerts = cloneDeep({ '2': activeAlert2 }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts, + previouslyRecoveredAlerts, + hasReachedAlertLimit: false, + alertLimit: 10, + setFlapping: false, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + "2": Object { + "meta": Object { + "flappingHistory": Array [ + false, + ], + }, + "state": Object {}, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(` + Object { + "3": Object { + "meta": Object { + "flappingHistory": Array [ + false, + ], + }, + "state": Object {}, + }, + } + `); + }); + + describe('when hasReachedAlertLimit is true', () => { + test('if alert is still active, set flapping state to false', () => { + const activeAlert = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + + const alerts = cloneDeep({ '1': activeAlert }); + alerts['1'].scheduleActions('default' as never, { foo: '1' }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: alerts, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: true, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + false, + ], + }, + "state": Object {}, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(`Object {}`); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if new alert, set flapping state to true', () => { + const activeAlert1 = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + activeAlert1.scheduleActions('default' as never, { foo: '1' }); + const activeAlert2 = new Alert('1'); + activeAlert2.scheduleActions('default' as never, { foo: '1' }); + + const alerts = cloneDeep({ '1': activeAlert1, '2': activeAlert2 }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: { '1': activeAlert1 }, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: true, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + false, + ], + }, + "state": Object {}, + }, + "2": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "2": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if alert is active and previously recovered, set flapping state to true', () => { + const activeAlert1 = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + activeAlert1.scheduleActions('default' as never, { foo: '1' }); + const activeAlert2 = new Alert('1'); + activeAlert2.scheduleActions('default' as never, { foo: '1' }); + + const alerts = cloneDeep({ '1': activeAlert1, '2': activeAlert2 }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: {}, + previouslyRecoveredAlerts: { '1': activeAlert1 }, + hasReachedAlertLimit: true, + alertLimit: 10, + setFlapping: true, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + "2": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + "2": Object { + "meta": Object { + "flappingHistory": Array [ + true, + ], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + + test('if setFlapping is false should not update flappingHistory', () => { + const activeAlert1 = new Alert('1', { + meta: { flappingHistory: [false] }, + }); + activeAlert1.scheduleActions('default' as never, { foo: '1' }); + const activeAlert2 = new Alert('1'); + activeAlert2.scheduleActions('default' as never, { foo: '1' }); + + const alerts = cloneDeep({ '1': activeAlert1, '2': activeAlert2 }); + + const { activeAlerts, newAlerts, recoveredAlerts } = processAlerts({ + alerts, + existingAlerts: { '1': activeAlert1 }, + previouslyRecoveredAlerts: {}, + hasReachedAlertLimit: true, + alertLimit: 10, + setFlapping: false, + }); + + expect(activeAlerts).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flappingHistory": Array [ + false, + ], + }, + "state": Object {}, + }, + "2": Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(newAlerts).toMatchInlineSnapshot(` + Object { + "2": Object { + "meta": Object { + "flappingHistory": Array [], + }, + "state": Object { + "duration": "0", + "start": "1970-01-01T00:00:00.000Z", + }, + }, + } + `); + expect(recoveredAlerts).toMatchInlineSnapshot(`Object {}`); + }); + }); + }); + + describe('updateAlertFlappingHistory function', () => { + test('correctly updates flappingHistory', () => { + const alert = new Alert('1', { + meta: { flappingHistory: [false, false] }, + }); + updateAlertFlappingHistory(alert, true); + expect(alert.getFlappingHistory()).toEqual([false, false, true]); + }); + + test('correctly updates flappingHistory while maintaining a fixed size', () => { + const flappingHistory = new Array(20).fill(false); + const alert = new Alert('1', { + meta: { flappingHistory }, + }); + updateAlertFlappingHistory(alert, true); + const fh = alert.getFlappingHistory() || []; + expect(fh.length).toEqual(20); + const result = new Array(19).fill(false); + expect(fh).toEqual(result.concat(true)); + }); + + test('correctly updates flappingHistory while maintaining if array is larger than fixed size', () => { + const flappingHistory = new Array(23).fill(false); + const alert = new Alert('1', { + meta: { flappingHistory }, + }); + updateAlertFlappingHistory(alert, true); + const fh = alert.getFlappingHistory() || []; + expect(fh.length).toEqual(20); + const result = new Array(19).fill(false); + expect(fh).toEqual(result.concat(true)); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/lib/process_alerts.ts b/x-pack/plugins/alerting/server/lib/process_alerts.ts index c0352a06f2eba..40c86dc461ab0 100644 --- a/x-pack/plugins/alerting/server/lib/process_alerts.ts +++ b/x-pack/plugins/alerting/server/lib/process_alerts.ts @@ -9,6 +9,7 @@ import { millisToNanos } from '@kbn/event-log-plugin/server'; import { cloneDeep } from 'lodash'; import { Alert } from '../alert'; import { AlertInstanceState, AlertInstanceContext } from '../types'; +import { updateFlappingHistory } from './flapping_utils'; interface ProcessAlertsOpts< State extends AlertInstanceState, @@ -16,8 +17,11 @@ interface ProcessAlertsOpts< > { alerts: Record>; existingAlerts: Record>; + previouslyRecoveredAlerts: Record>; hasReachedAlertLimit: boolean; alertLimit: number; + // flag used to determine whether or not we want to push the flapping state on to the flappingHistory array + setFlapping: boolean; } interface ProcessAlertsResult< State extends AlertInstanceState, @@ -27,6 +31,8 @@ interface ProcessAlertsResult< > { newAlerts: Record>; activeAlerts: Record>; + // recovered alerts in the current rule run that were previously active + currentRecoveredAlerts: Record>; recoveredAlerts: Record>; } @@ -38,8 +44,10 @@ export function processAlerts< >({ alerts, existingAlerts, + previouslyRecoveredAlerts, hasReachedAlertLimit, alertLimit, + setFlapping, }: ProcessAlertsOpts): ProcessAlertsResult< State, Context, @@ -47,8 +55,14 @@ export function processAlerts< RecoveryActionGroupId > { return hasReachedAlertLimit - ? processAlertsLimitReached(alerts, existingAlerts, alertLimit) - : processAlertsHelper(alerts, existingAlerts); + ? processAlertsLimitReached( + alerts, + existingAlerts, + previouslyRecoveredAlerts, + alertLimit, + setFlapping + ) + : processAlertsHelper(alerts, existingAlerts, previouslyRecoveredAlerts, setFlapping); } function processAlertsHelper< @@ -58,13 +72,17 @@ function processAlertsHelper< RecoveryActionGroupId extends string >( alerts: Record>, - existingAlerts: Record> + existingAlerts: Record>, + previouslyRecoveredAlerts: Record>, + setFlapping: boolean ): ProcessAlertsResult { const existingAlertIds = new Set(Object.keys(existingAlerts)); + const previouslyRecoveredAlertsIds = new Set(Object.keys(previouslyRecoveredAlerts)); const currentTime = new Date().toISOString(); const newAlerts: Record> = {}; const activeAlerts: Record> = {}; + const currentRecoveredAlerts: Record> = {}; const recoveredAlerts: Record> = {}; for (const id in alerts) { @@ -73,13 +91,20 @@ function processAlertsHelper< if (alerts[id].hasScheduledActions()) { activeAlerts[id] = alerts[id]; - // if this alert did not exist in previous run, it is considered "new" + // if this alert was not active in the previous run, we need to inject start time into the alert state if (!existingAlertIds.has(id)) { newAlerts[id] = alerts[id]; - - // Inject start time into alert state for new alerts const state = newAlerts[id].getState(); newAlerts[id].replaceState({ ...state, start: currentTime, duration: '0' }); + + if (setFlapping) { + if (previouslyRecoveredAlertsIds.has(id)) { + // this alert has flapped from recovered to active + newAlerts[id].setFlappingHistory(previouslyRecoveredAlerts[id].getFlappingHistory()); + previouslyRecoveredAlertsIds.delete(id); + } + updateAlertFlappingHistory(newAlerts[id], true); + } } else { // this alert did exist in previous run // calculate duration to date for active alerts @@ -92,9 +117,15 @@ function processAlertsHelper< ...(state.start ? { start: state.start } : {}), ...(duration !== undefined ? { duration } : {}), }); + + // this alert is still active + if (setFlapping) { + updateAlertFlappingHistory(activeAlerts[id], false); + } } } else if (existingAlertIds.has(id)) { recoveredAlerts[id] = alerts[id]; + currentRecoveredAlerts[id] = alerts[id]; // Inject end time into alert state of recovered alerts const state = recoveredAlerts[id].getState(); @@ -106,10 +137,23 @@ function processAlertsHelper< ...(duration ? { duration } : {}), ...(state.start ? { end: currentTime } : {}), }); + // this alert has flapped from active to recovered + if (setFlapping) { + updateAlertFlappingHistory(recoveredAlerts[id], true); + } } } } - return { recoveredAlerts, newAlerts, activeAlerts }; + + // alerts are still recovered + for (const id of previouslyRecoveredAlertsIds) { + recoveredAlerts[id] = previouslyRecoveredAlerts[id]; + if (setFlapping) { + updateAlertFlappingHistory(recoveredAlerts[id], false); + } + } + + return { recoveredAlerts, currentRecoveredAlerts, newAlerts, activeAlerts }; } function processAlertsLimitReached< @@ -120,9 +164,12 @@ function processAlertsLimitReached< >( alerts: Record>, existingAlerts: Record>, - alertLimit: number + previouslyRecoveredAlerts: Record>, + alertLimit: number, + setFlapping: boolean ): ProcessAlertsResult { const existingAlertIds = new Set(Object.keys(existingAlerts)); + const previouslyRecoveredAlertsIds = new Set(Object.keys(previouslyRecoveredAlerts)); // When the alert limit has been reached, // - skip determination of recovered alerts @@ -152,6 +199,11 @@ function processAlertsLimitReached< ...(state.start ? { start: state.start } : {}), ...(duration !== undefined ? { duration } : {}), }); + + // this alert is still active + if (setFlapping) { + updateAlertFlappingHistory(activeAlerts[id], false); + } } } @@ -161,7 +213,7 @@ function processAlertsLimitReached< // if we don't have capacity for new alerts, return if (!hasCapacityForNewAlerts()) { - return { recoveredAlerts: {}, newAlerts: {}, activeAlerts }; + return { recoveredAlerts: {}, currentRecoveredAlerts: {}, newAlerts: {}, activeAlerts }; } // look for new alerts and add until we hit capacity @@ -171,16 +223,33 @@ function processAlertsLimitReached< if (!existingAlertIds.has(id)) { activeAlerts[id] = alerts[id]; newAlerts[id] = alerts[id]; - - // Inject start time into alert state for new alerts + // if this alert was not active in the previous run, we need to inject start time into the alert state const state = newAlerts[id].getState(); newAlerts[id].replaceState({ ...state, start: currentTime, duration: '0' }); + if (setFlapping) { + if (previouslyRecoveredAlertsIds.has(id)) { + // this alert has flapped from recovered to active + newAlerts[id].setFlappingHistory(previouslyRecoveredAlerts[id].getFlappingHistory()); + } + updateAlertFlappingHistory(newAlerts[id], true); + } + if (!hasCapacityForNewAlerts()) { break; } } } } - return { recoveredAlerts: {}, newAlerts, activeAlerts }; + return { recoveredAlerts: {}, currentRecoveredAlerts: {}, newAlerts, activeAlerts }; +} + +export function updateAlertFlappingHistory< + State extends AlertInstanceState, + Context extends AlertInstanceContext, + ActionGroupIds extends string, + RecoveryActionGroupId extends string +>(alert: Alert, state: boolean) { + const updatedFlappingHistory = updateFlappingHistory(alert.getFlappingHistory() || [], state); + alert.setFlappingHistory(updatedFlappingHistory); } diff --git a/x-pack/plugins/alerting/server/lib/set_flapping.test.ts b/x-pack/plugins/alerting/server/lib/set_flapping.test.ts new file mode 100644 index 0000000000000..9900d3391861b --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/set_flapping.test.ts @@ -0,0 +1,166 @@ +/* + * 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 { pick } from 'lodash'; +import { Alert } from '../alert'; +import { AlertInstanceState, AlertInstanceContext, DefaultActionGroupId } from '../../common'; +import { setFlapping, isAlertFlapping } from './set_flapping'; + +describe('setFlapping', () => { + const flapping = new Array(16).fill(false).concat([true, true, true, true]); + const notFlapping = new Array(20).fill(false); + + test('should set flapping on alerts', () => { + const activeAlerts = { + '1': new Alert('1', { meta: { flappingHistory: flapping } }), + '2': new Alert('2', { meta: { flappingHistory: [false, false] } }), + '3': new Alert('3', { meta: { flapping: true, flappingHistory: flapping } }), + '4': new Alert('4', { meta: { flapping: true, flappingHistory: [false, false] } }), + }; + + const recoveredAlerts = { + '1': new Alert('1', { meta: { flappingHistory: [true, true, true, true] } }), + '2': new Alert('2', { meta: { flappingHistory: notFlapping } }), + '3': new Alert('3', { meta: { flapping: true, flappingHistory: [true, true] } }), + '4': new Alert('4', { meta: { flapping: true, flappingHistory: notFlapping } }), + }; + + setFlapping(activeAlerts, recoveredAlerts); + const fields = ['1.meta.flapping', '2.meta.flapping', '3.meta.flapping', '4.meta.flapping']; + expect(pick(activeAlerts, fields)).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flapping": true, + }, + }, + "2": Object { + "meta": Object { + "flapping": false, + }, + }, + "3": Object { + "meta": Object { + "flapping": true, + }, + }, + "4": Object { + "meta": Object { + "flapping": true, + }, + }, + } + `); + expect(pick(recoveredAlerts, fields)).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "flapping": true, + }, + }, + "2": Object { + "meta": Object { + "flapping": false, + }, + }, + "3": Object { + "meta": Object { + "flapping": true, + }, + }, + "4": Object { + "meta": Object { + "flapping": false, + }, + }, + } + `); + }); + + describe('isAlertFlapping', () => { + describe('not currently flapping', () => { + test('returns true if the flap count exceeds the threshold', () => { + const flappingHistory = [true, true, true, true].concat(new Array(16).fill(false)); + const alert = new Alert( + '1', + { + meta: { flappingHistory }, + } + ); + expect(isAlertFlapping(alert)).toEqual(true); + }); + + test("returns false the flap count doesn't exceed the threshold", () => { + const flappingHistory = [true, true].concat(new Array(20).fill(false)); + const alert = new Alert( + '1', + { + meta: { flappingHistory }, + } + ); + expect(isAlertFlapping(alert)).toEqual(false); + }); + + test('returns true if not at capacity and the flap count exceeds the threshold', () => { + const flappingHistory = new Array(5).fill(true); + const alert = new Alert( + '1', + { + meta: { flappingHistory }, + } + ); + expect(isAlertFlapping(alert)).toEqual(true); + }); + }); + + describe('currently flapping', () => { + test('returns true if at capacity and the flap count exceeds the threshold', () => { + const flappingHistory = new Array(16).fill(false).concat([true, true, true, true]); + const alert = new Alert( + '1', + { + meta: { flappingHistory, flapping: true }, + } + ); + expect(isAlertFlapping(alert)).toEqual(true); + }); + + test("returns true if not at capacity and the flap count doesn't exceed the threshold", () => { + const flappingHistory = new Array(16).fill(false); + const alert = new Alert( + '1', + { + meta: { flappingHistory, flapping: true }, + } + ); + expect(isAlertFlapping(alert)).toEqual(true); + }); + + test('returns true if not at capacity and the flap count exceeds the threshold', () => { + const flappingHistory = new Array(10).fill(false).concat([true, true, true, true]); + const alert = new Alert( + '1', + { + meta: { flappingHistory, flapping: true }, + } + ); + expect(isAlertFlapping(alert)).toEqual(true); + }); + + test("returns false if at capacity and the flap count doesn't exceed the threshold", () => { + const flappingHistory = new Array(20).fill(false); + const alert = new Alert( + '1', + { + meta: { flappingHistory, flapping: true }, + } + ); + expect(isAlertFlapping(alert)).toEqual(false); + }); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/lib/set_flapping.ts b/x-pack/plugins/alerting/server/lib/set_flapping.ts new file mode 100644 index 0000000000000..2e941cf06e07c --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/set_flapping.ts @@ -0,0 +1,44 @@ +/* + * 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 { keys } from 'lodash'; +import { Alert } from '../alert'; +import { AlertInstanceState, AlertInstanceContext } from '../types'; +import { isFlapping } from './flapping_utils'; + +export function setFlapping< + State extends AlertInstanceState, + Context extends AlertInstanceContext, + ActionGroupIds extends string, + RecoveryActionGroupIds extends string +>( + activeAlerts: Record> = {}, + recoveredAlerts: Record> = {} +) { + for (const id of keys(activeAlerts)) { + const alert = activeAlerts[id]; + const flapping = isAlertFlapping(alert); + alert.setFlapping(flapping); + } + + for (const id of keys(recoveredAlerts)) { + const alert = recoveredAlerts[id]; + const flapping = isAlertFlapping(alert); + alert.setFlapping(flapping); + } +} + +export function isAlertFlapping< + State extends AlertInstanceState, + Context extends AlertInstanceContext, + ActionGroupIds extends string, + RecoveryActionGroupId extends string +>(alert: Alert): boolean { + const flappingHistory: boolean[] = alert.getFlappingHistory() || []; + const isCurrentlyFlapping = alert.getFlapping(); + return isFlapping(flappingHistory, isCurrentlyFlapping); +} diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index b0e98263591a9..0c6bc7a41af69 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -250,6 +250,7 @@ export const generateRunnerResult = ({ state = false, interval = '10s', alertInstances = {}, + alertRecoveredInstances = {}, }: GeneratorParams = {}) => { return { monitoring: { @@ -276,6 +277,7 @@ export const generateRunnerResult = ({ }, state: { ...(state && { alertInstances }), + ...(state && { alertRecoveredInstances }), ...(state && { alertTypeState: undefined }), ...(state && { previousStartedAt: new Date('1970-01-01T00:00:00.000Z') }), }, @@ -311,13 +313,17 @@ export const generateEnqueueFunctionInput = (isArray: boolean = false) => { return isArray ? [input] : input; }; -export const generateAlertInstance = ({ id, duration, start }: GeneratorParams = { id: 1 }) => ({ +export const generateAlertInstance = ( + { id, duration, start, flappingHistory }: GeneratorParams = { id: 1, flappingHistory: [false] } +) => ({ [String(id)]: { meta: { lastScheduledActions: { date: new Date(DATE_1970), group: 'default', }, + flappingHistory, + flapping: false, }, state: { bar: false, diff --git a/x-pack/plugins/alerting/server/task_runner/log_alerts.test.ts b/x-pack/plugins/alerting/server/task_runner/log_alerts.test.ts index 43f191fc0a3aa..163cadf1d084b 100644 --- a/x-pack/plugins/alerting/server/task_runner/log_alerts.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/log_alerts.test.ts @@ -241,4 +241,86 @@ describe('logAlerts', () => { expect(alertingEventLogger.logAlert).not.toHaveBeenCalled(); }); + + test('should correctly set flapping values', () => { + logAlerts({ + logger, + alertingEventLogger, + newAlerts: { + '4': new Alert<{}, {}, DefaultActionGroupId>('4'), + }, + activeAlerts: { + '1': new Alert<{}, {}, DefaultActionGroupId>('1', { meta: { flapping: true } }), + '2': new Alert<{}, {}, DefaultActionGroupId>('2'), + '4': new Alert<{}, {}, DefaultActionGroupId>('4'), + }, + recoveredAlerts: { + '7': new Alert<{}, {}, DefaultActionGroupId>('7'), + '8': new Alert<{}, {}, DefaultActionGroupId>('8', { meta: { flapping: true } }), + '9': new Alert<{}, {}, DefaultActionGroupId>('9'), + '10': new Alert<{}, {}, DefaultActionGroupId>('10'), + }, + ruleLogPrefix: `test-rule-type-id:123: 'test rule'`, + ruleRunMetricsStore, + canSetRecoveryContext: false, + shouldPersistAlerts: true, + }); + + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(1, { + action: 'recovered-instance', + id: '7', + message: "test-rule-type-id:123: 'test rule' alert '7' has recovered", + state: {}, + flapping: false, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(2, { + action: 'recovered-instance', + id: '8', + message: "test-rule-type-id:123: 'test rule' alert '8' has recovered", + state: {}, + flapping: true, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(3, { + action: 'recovered-instance', + id: '9', + message: "test-rule-type-id:123: 'test rule' alert '9' has recovered", + state: {}, + flapping: false, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(4, { + action: 'recovered-instance', + id: '10', + message: "test-rule-type-id:123: 'test rule' alert '10' has recovered", + state: {}, + flapping: false, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(5, { + action: 'new-instance', + id: '4', + message: "test-rule-type-id:123: 'test rule' created new alert: '4'", + state: {}, + flapping: false, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(6, { + action: 'active-instance', + id: '1', + message: "test-rule-type-id:123: 'test rule' active alert: '1' in actionGroup: 'undefined'", + state: {}, + flapping: true, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(7, { + action: 'active-instance', + id: '2', + message: "test-rule-type-id:123: 'test rule' active alert: '2' in actionGroup: 'undefined'", + state: {}, + flapping: false, + }); + expect(alertingEventLogger.logAlert).toHaveBeenNthCalledWith(8, { + action: 'active-instance', + id: '4', + message: "test-rule-type-id:123: 'test rule' active alert: '4' in actionGroup: 'undefined'", + state: {}, + flapping: false, + }); + }); }); diff --git a/x-pack/plugins/alerting/server/task_runner/log_alerts.ts b/x-pack/plugins/alerting/server/task_runner/log_alerts.ts index b7abaf4c236be..7f8f5d93a5d9f 100644 --- a/x-pack/plugins/alerting/server/task_runner/log_alerts.ts +++ b/x-pack/plugins/alerting/server/task_runner/log_alerts.ts @@ -90,19 +90,17 @@ export function logAlerts< ruleRunMetricsStore.setNumberOfNewAlerts(newAlertIds.length); ruleRunMetricsStore.setNumberOfActiveAlerts(activeAlertIds.length); ruleRunMetricsStore.setNumberOfRecoveredAlerts(recoveredAlertIds.length); - for (const id of recoveredAlertIds) { const { group: actionGroup } = recoveredAlerts[id].getLastScheduledActions() ?? {}; const state = recoveredAlerts[id].getState(); const message = `${ruleLogPrefix} alert '${id}' has recovered`; - alertingEventLogger.logAlert({ action: EVENT_LOG_ACTIONS.recoveredInstance, id, group: actionGroup, message, state, - flapping: false, + flapping: recoveredAlerts[id].getFlapping(), }); } @@ -116,7 +114,7 @@ export function logAlerts< group: actionGroup, message, state, - flapping: false, + flapping: activeAlerts[id].getFlapping(), }); } @@ -130,7 +128,7 @@ export function logAlerts< group: actionGroup, message, state, - flapping: false, + flapping: activeAlerts[id].getFlapping(), }); } } diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts index 69b648f099e44..35094f1d8af49 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts @@ -1023,7 +1023,12 @@ describe('Task Runner', () => { encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult.state.alertInstances).toEqual( - generateAlertInstance({ id: 1, duration: MOCK_DURATION, start: DATE_1969 }) + generateAlertInstance({ + id: 1, + duration: MOCK_DURATION, + start: DATE_1969, + flappingHistory: [false], + }) ); expect(logger.debug).toHaveBeenCalledTimes(7); @@ -1340,7 +1345,13 @@ describe('Task Runner', () => { encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult.state.alertInstances).toEqual( - generateAlertInstance({ id: 1, duration: MOCK_DURATION, start: DATE_1969 }) + generateAlertInstance({ + id: 1, + duration: MOCK_DURATION, + start: DATE_1969, + flappingHistory: [false], + flapping: false, + }) ); testAlertingEventLogCalls({ @@ -2409,6 +2420,8 @@ describe('Task Runner', () => { date: new Date(DATE_1970), group: 'default', }, + flappingHistory: [true], + flapping: false, }, state: { duration: '0', @@ -2571,6 +2584,8 @@ describe('Task Runner', () => { date: new Date(DATE_1970), group: 'default', }, + flappingHistory: [true], + flapping: false, }, state: { duration: '0', @@ -2583,6 +2598,8 @@ describe('Task Runner', () => { date: new Date(DATE_1970), group: 'default', }, + flappingHistory: [true], + flapping: false, }, state: { duration: '0', diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.ts index 3fc70537f1bc9..323c885416bef 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -23,14 +23,15 @@ import { ruleExecutionStatusToRaw, isRuleSnoozed, processAlerts, + setFlapping, lastRunFromError, getNextRun, + determineAlertsToReturn, } from '../lib'; import { RuleExecutionStatus, RuleExecutionStatusErrorReasons, IntervalSchedule, - RawAlertInstance, RawRuleExecutionStatus, RawRuleMonitoring, RuleTaskState, @@ -233,6 +234,7 @@ export class TaskRunner< params: { alertId: ruleId, spaceId }, state: { alertInstances: alertRawInstances = {}, + alertRecoveredInstances: alertRecoveredRawInstances = {}, alertTypeState: ruleTypeState = {}, previousStartedAt, }, @@ -266,7 +268,7 @@ export class TaskRunner< searchSourceClient, }); - const { updatedRuleTypeState, hasReachedAlertLimit, originalAlerts } = + const { updatedRuleTypeState, hasReachedAlertLimit, originalAlerts, originalRecoveredAlerts } = await this.timer.runWithTimer(TaskRunnerTimerSpan.RuleTypeRun, async () => { for (const id in alertRawInstances) { if (alertRawInstances.hasOwnProperty(id)) { @@ -274,6 +276,13 @@ export class TaskRunner< } } + const recoveredAlerts: Record> = {}; + for (const id in alertRecoveredRawInstances) { + if (alertRecoveredRawInstances.hasOwnProperty(id)) { + recoveredAlerts[id] = new Alert(id, alertRecoveredRawInstances[id]); + } + } + const alertsCopy = cloneDeep(this.alerts); const alertFactory = createAlertFactory< @@ -386,31 +395,40 @@ export class TaskRunner< return { originalAlerts: alertsCopy, + originalRecoveredAlerts: recoveredAlerts, updatedRuleTypeState: updatedState || undefined, hasReachedAlertLimit: alertFactory.hasReachedAlertLimit(), }; }); - const { activeAlerts, recoveredAlerts } = await this.timer.runWithTimer( + const { activeAlerts, recoveredAlerts, currentRecoveredAlerts } = await this.timer.runWithTimer( TaskRunnerTimerSpan.ProcessAlerts, async () => { const { newAlerts: processedAlertsNew, activeAlerts: processedAlertsActive, + currentRecoveredAlerts: processedAlertsRecoveredCurrent, recoveredAlerts: processedAlertsRecovered, } = processAlerts({ alerts: this.alerts, existingAlerts: originalAlerts, + previouslyRecoveredAlerts: originalRecoveredAlerts, hasReachedAlertLimit, alertLimit: this.maxAlerts, + setFlapping: true, }); + setFlapping( + processedAlertsActive, + processedAlertsRecovered + ); + logAlerts({ logger: this.logger, alertingEventLogger: this.alertingEventLogger, newAlerts: processedAlertsNew, activeAlerts: processedAlertsActive, - recoveredAlerts: processedAlertsRecovered, + recoveredAlerts: processedAlertsRecoveredCurrent, ruleLogPrefix: ruleLabel, ruleRunMetricsStore, canSetRecoveryContext: ruleType.doesSetRecoveryContext ?? false, @@ -421,6 +439,7 @@ export class TaskRunner< newAlerts: processedAlertsNew, activeAlerts: processedAlertsActive, recoveredAlerts: processedAlertsRecovered, + currentRecoveredAlerts: processedAlertsRecoveredCurrent, }; } ); @@ -452,21 +471,22 @@ export class TaskRunner< this.countUsageOfActionExecutionAfterRuleCancellation(); } else { await executionHandler.run(activeAlerts); - await executionHandler.run(recoveredAlerts, true); + await executionHandler.run(currentRecoveredAlerts, true); } }); - const alertsToReturn: Record = {}; - for (const id in activeAlerts) { - if (activeAlerts.hasOwnProperty(id)) { - alertsToReturn[id] = activeAlerts[id].toRaw(); - } - } + const { alertsToReturn, recoveredAlertsToReturn } = determineAlertsToReturn< + State, + Context, + ActionGroupIds, + RecoveryActionGroupId + >(activeAlerts, recoveredAlerts); return { metrics: ruleRunMetricsStore.getMetrics(), alertTypeState: updatedRuleTypeState || undefined, alertInstances: alertsToReturn, + alertRecoveredInstances: recoveredAlertsToReturn, }; } diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts index a9f8ce56c441f..f9b1a79742c38 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts @@ -79,13 +79,18 @@ const mockOptions = { alertId: 'TEST_ALERT_0', alertUuid: 'TEST_ALERT_0_UUID', started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, TEST_ALERT_1: { alertId: 'TEST_ALERT_1', alertUuid: 'TEST_ALERT_1_UUID', started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, }, + trackedAlertsRecovered: {}, }, spaceId: '', rule: { diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts index b4aa184cab592..5e43223480428 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts @@ -6,6 +6,7 @@ */ import { loggerMock } from '@kbn/logging-mocks'; +import { pick } from 'lodash'; import { ALERT_INSTANCE_ID, ALERT_RULE_CATEGORY, @@ -22,6 +23,7 @@ import { EVENT_ACTION, EVENT_KIND, SPACE_IDS, + ALERT_FLAPPING, } from '../../common/technical_rule_data_field_names'; import { createRuleDataClientMock } from '../rule_data_client/rule_data_client.mock'; import { createLifecycleExecutor } from './create_lifecycle_executor'; @@ -48,7 +50,7 @@ describe('createLifecycleExecutor', () => { const newRuleState = await executor( createDefaultAlertExecutorOptions({ params: {}, - state: { wrapped: initialRuleState, trackedAlerts: {} }, + state: { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }, logger, }) ); @@ -58,6 +60,7 @@ describe('createLifecycleExecutor', () => { aRuleStateKey: 'NEXT_RULE_STATE_VALUE', }, trackedAlerts: {}, + trackedAlertsRecovered: {}, }); }); @@ -83,7 +86,7 @@ describe('createLifecycleExecutor', () => { await executor( createDefaultAlertExecutorOptions({ params: {}, - state: { wrapped: initialRuleState, trackedAlerts: {} }, + state: { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }, logger, }) ); @@ -192,13 +195,18 @@ describe('createLifecycleExecutor', () => { alertId: 'TEST_ALERT_0', alertUuid: 'TEST_ALERT_0_UUID', started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, TEST_ALERT_1: { alertId: 'TEST_ALERT_1', alertUuid: 'TEST_ALERT_1_UUID', started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, }, + trackedAlertsRecovered: {}, }, logger, }) @@ -308,13 +316,18 @@ describe('createLifecycleExecutor', () => { alertId: 'TEST_ALERT_0', alertUuid: 'TEST_ALERT_0_UUID', started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, TEST_ALERT_1: { alertId: 'TEST_ALERT_1', alertUuid: 'TEST_ALERT_1_UUID', started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, }, }, + trackedAlertsRecovered: {}, }, logger, }) @@ -374,7 +387,7 @@ describe('createLifecycleExecutor', () => { await executor( createDefaultAlertExecutorOptions({ params: {}, - state: { wrapped: initialRuleState, trackedAlerts: {} }, + state: { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }, shouldWriteAlerts: false, logger, }) @@ -404,13 +417,828 @@ describe('createLifecycleExecutor', () => { executor( createDefaultAlertExecutorOptions({ params: {}, - state: { wrapped: initialRuleState, trackedAlerts: {} }, + state: { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }, shouldWriteAlerts: false, logger, }) ) ).rejects.toThrowErrorMatchingInlineSnapshot(`"error initializing!"`); }); + + describe('updating flappingHistory', () => { + it('sets flapping state to true on a new alert', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + services.alertWithLifecycle({ + id: 'TEST_ALERT_0', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + + return state; + }); + + const { trackedAlerts, trackedAlertsRecovered } = await executor( + createDefaultAlertExecutorOptions({ + params: {}, + state: { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }, + logger, + }) + ); + + const alerts = pick(trackedAlerts, [ + 'TEST_ALERT_0.flappingHistory', + 'TEST_ALERT_1.flappingHistory', + ]); + expect(alerts).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_0": Object { + "flappingHistory": Array [ + true, + ], + }, + "TEST_ALERT_1": Object { + "flappingHistory": Array [ + true, + ], + }, + } + `); + expect(trackedAlertsRecovered).toMatchInlineSnapshot(`Object {}`); + }); + + it('sets flapping state to false on an alert that is still active', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'closed', + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must show up in the written doc + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'open', + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must not show up in the written doc + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + services.alertWithLifecycle({ + id: 'TEST_ALERT_0', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + + return state; + }); + + const { trackedAlerts, trackedAlertsRecovered } = await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlertsRecovered: {}, + }, + logger, + }) + ); + + const alerts = pick(trackedAlerts, [ + 'TEST_ALERT_0.flappingHistory', + 'TEST_ALERT_1.flappingHistory', + ]); + expect(alerts).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_0": Object { + "flappingHistory": Array [ + false, + ], + }, + "TEST_ALERT_1": Object { + "flappingHistory": Array [ + false, + ], + }, + } + `); + expect(trackedAlertsRecovered).toMatchInlineSnapshot(`Object {}`); + }); + + it('sets flapping state to true on an alert that is active and previously recovered', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'closed', + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must show up in the written doc + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'open', + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must not show up in the written doc + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + services.alertWithLifecycle({ + id: 'TEST_ALERT_0', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + + return state; + }); + + const { trackedAlerts, trackedAlertsRecovered } = await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlertsRecovered: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlerts: {}, + }, + logger, + }) + ); + + const alerts = pick(trackedAlerts, [ + 'TEST_ALERT_0.flappingHistory', + 'TEST_ALERT_1.flappingHistory', + ]); + expect(alerts).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_0": Object { + "flappingHistory": Array [ + true, + ], + }, + "TEST_ALERT_1": Object { + "flappingHistory": Array [ + true, + ], + }, + } + `); + expect(trackedAlertsRecovered).toMatchInlineSnapshot(`Object {}`); + }); + + it('sets flapping state to true on an alert that is recovered and previously active', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must show up in the written doc + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must not show up in the written doc + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + // TEST_ALERT_0 has recovered + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + + return state; + }); + + const { trackedAlerts, trackedAlertsRecovered } = await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlertsRecovered: {}, + }, + logger, + }) + ); + + const recovered = pick(trackedAlertsRecovered, ['TEST_ALERT_0.flappingHistory']); + expect(recovered).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_0": Object { + "flappingHistory": Array [ + true, + ], + }, + } + `); + const active = pick(trackedAlerts, ['TEST_ALERT_1.flappingHistory']); + expect(active).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_1": Object { + "flappingHistory": Array [ + false, + ], + }, + } + `); + }); + + it('sets flapping state to false on an alert that is still recovered', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must show up in the written doc + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + labels: { LABEL_0_KEY: 'LABEL_0_VALUE' }, // this must not show up in the written doc + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + // TEST_ALERT_0 has recovered + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + + return state; + }); + + const { trackedAlerts, trackedAlertsRecovered } = await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlertsRecovered: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + }, + logger, + }) + ); + + const recovered = pick(trackedAlertsRecovered, ['TEST_ALERT_0.flappingHistory']); + expect(recovered).toMatchInlineSnapshot(`Object {}`); + const active = pick(trackedAlerts, ['TEST_ALERT_1.flappingHistory']); + expect(active).toMatchInlineSnapshot(` + Object { + "TEST_ALERT_1": Object { + "flappingHistory": Array [ + false, + ], + }, + } + `); + }); + }); + + describe('set flapping on the document', () => { + const flapping = new Array(16).fill(false).concat([true, true, true, true]); + const notFlapping = new Array(20).fill(false); + + it('updates documents with flapping for active alerts', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'closed', + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'open', + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_2', + [ALERT_UUID]: 'ALERT_2_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'open', + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_3', + [ALERT_UUID]: 'ALERT_3_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_WORKFLOW_STATUS]: 'open', + [SPACE_IDS]: ['fake-space-id'], + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + services.alertWithLifecycle({ + id: 'TEST_ALERT_0', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_1', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_2', + fields: {}, + }); + services.alertWithLifecycle({ + id: 'TEST_ALERT_3', + fields: {}, + }); + + return state; + }); + + await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: flapping, + flapping: false, + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [false, false], + flapping: false, + }, + TEST_ALERT_2: { + alertId: 'TEST_ALERT_2', + alertUuid: 'TEST_ALERT_2_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: flapping, + flapping: true, + }, + TEST_ALERT_3: { + alertId: 'TEST_ALERT_3', + alertUuid: 'TEST_ALERT_3_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [false, false], + flapping: true, + }, + }, + trackedAlertsRecovered: {}, + }, + logger, + }) + ); + + expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith( + expect.objectContaining({ + body: [ + // alert document + { index: { _id: 'TEST_ALERT_0_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_WORKFLOW_STATUS]: 'closed', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [ALERT_FLAPPING]: true, + [EVENT_ACTION]: 'active', + [EVENT_KIND]: 'signal', + }), + { index: { _id: 'TEST_ALERT_1_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_WORKFLOW_STATUS]: 'open', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [EVENT_ACTION]: 'active', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: false, + }), + { index: { _id: 'TEST_ALERT_2_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_2', + [ALERT_WORKFLOW_STATUS]: 'open', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [EVENT_ACTION]: 'active', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: true, + }), + { index: { _id: 'TEST_ALERT_3_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_3', + [ALERT_WORKFLOW_STATUS]: 'open', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [EVENT_ACTION]: 'active', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: true, + }), + ], + }) + ); + }); + + it('updates existing documents for recovered alerts', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + ruleDataClientMock.getReader().search.mockResolvedValue({ + hits: { + hits: [ + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_UUID]: 'ALERT_0_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_UUID]: 'ALERT_1_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_2', + [ALERT_UUID]: 'ALERT_2_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + }, + }, + { + _source: { + '@timestamp': '', + [ALERT_INSTANCE_ID]: 'TEST_ALERT_3', + [ALERT_UUID]: 'ALERT_3_UUID', + [ALERT_RULE_CATEGORY]: 'RULE_TYPE_NAME', + [ALERT_RULE_CONSUMER]: 'CONSUMER', + [ALERT_RULE_NAME]: 'NAME', + [ALERT_RULE_PRODUCER]: 'PRODUCER', + [ALERT_RULE_TYPE_ID]: 'RULE_TYPE_ID', + [ALERT_RULE_UUID]: 'RULE_UUID', + [ALERT_STATUS]: ALERT_STATUS_ACTIVE, + [SPACE_IDS]: ['fake-space-id'], + }, + }, + ], + }, + } as any); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async ({ services, state }) => { + return state; + }); + + await executor( + createDefaultAlertExecutorOptions({ + alertId: 'TEST_ALERT_0', + params: {}, + state: { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [true, true, true, true], + flapping: false, + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: notFlapping, + flapping: false, + }, + TEST_ALERT_2: { + alertId: 'TEST_ALERT_2', + alertUuid: 'TEST_ALERT_2_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: [true, true], + flapping: true, + }, + TEST_ALERT_3: { + alertId: 'TEST_ALERT_3', + alertUuid: 'TEST_ALERT_3_UUID', + started: '2020-01-02T12:00:00.000Z', + flappingHistory: notFlapping, + flapping: false, + }, + }, + trackedAlertsRecovered: {}, + }, + logger, + }) + ); + + expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.arrayContaining([ + // alert document + { index: { _id: 'TEST_ALERT_0_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', + [ALERT_STATUS]: ALERT_STATUS_RECOVERED, + [EVENT_ACTION]: 'close', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: true, + }), + { index: { _id: 'TEST_ALERT_1_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_1', + [ALERT_STATUS]: ALERT_STATUS_RECOVERED, + [EVENT_ACTION]: 'close', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: false, + }), + { index: { _id: 'TEST_ALERT_2_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_2', + [ALERT_STATUS]: ALERT_STATUS_RECOVERED, + [EVENT_ACTION]: 'close', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: true, + }), + { index: { _id: 'TEST_ALERT_3_UUID' } }, + expect.objectContaining({ + [ALERT_INSTANCE_ID]: 'TEST_ALERT_3', + [ALERT_STATUS]: ALERT_STATUS_RECOVERED, + [EVENT_ACTION]: 'close', + [EVENT_KIND]: 'signal', + [ALERT_FLAPPING]: false, + }), + ]), + }) + ); + }); + }); }); type TestRuleState = Record & { diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts index 131eb538f28aa..569a72f367c06 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts @@ -19,6 +19,7 @@ import { RuleTypeParams, RuleTypeState, } from '@kbn/alerting-plugin/server'; +import { isFlapping } from '@kbn/alerting-plugin/server/lib'; import { ParsedExperimentalFields } from '../../common/parse_experimental_fields'; import { ParsedTechnicalFields } from '../../common/parse_technical_fields'; import { @@ -37,13 +38,14 @@ import { TAGS, TIMESTAMP, VERSION, - // ALERT_FLAPPING, + ALERT_FLAPPING, } from '../../common/technical_rule_data_field_names'; import { CommonAlertFieldNameLatest, CommonAlertIdFieldNameLatest } from '../../common/schemas'; import { IRuleDataClient } from '../rule_data_client'; import { AlertExecutorOptionsWithExtraServices } from '../types'; import { fetchExistingAlerts } from './fetch_existing_alerts'; import { getCommonAlertFields } from './get_common_alert_fields'; +import { getUpdatedFlappingHistory } from './get_updated_flapping_history'; import { fetchAlertByAlertUUID } from './fetch_alert_by_uuid'; type ImplicitTechnicalFieldName = CommonAlertFieldNameLatest | CommonAlertIdFieldNameLatest; @@ -96,6 +98,12 @@ const trackedAlertStateRt = rt.type({ alertId: rt.string, alertUuid: rt.string, started: rt.string, + // an array used to track changes in alert state, the order is based on the rule executions + // true - alert has changed from active/recovered + // false - alert is new or the status has remained either active or recovered + flappingHistory: rt.array(rt.boolean), + // flapping flag that indicates whether the alert is flapping + flapping: rt.boolean, }); export type TrackedLifecycleAlertState = rt.TypeOf; @@ -106,7 +114,10 @@ const alertTypeStateRt = () => const wrappedStateRt = () => rt.type({ wrapped: alertTypeStateRt(), + // tracks the active alerts trackedAlerts: rt.record(rt.string, trackedAlertStateRt), + // tracks the recovered alerts + trackedAlertsRecovered: rt.record(rt.string, trackedAlertStateRt), }); /** @@ -117,6 +128,7 @@ const wrappedStateRt = () => export type WrappedLifecycleRuleState = RuleTypeState & { wrapped: State | void; trackedAlerts: Record; + trackedAlertsRecovered: Record; }; export const createLifecycleExecutor = @@ -156,6 +168,7 @@ export const createLifecycleExecutor = (): WrappedLifecycleRuleState => ({ wrapped: previousState as State, trackedAlerts: {}, + trackedAlertsRecovered: {}, }) )(wrappedStateRt().decode(previousState)); @@ -205,6 +218,7 @@ export const createLifecycleExecutor = const currentAlertIds = Object.keys(currentAlerts); const trackedAlertIds = Object.keys(state.trackedAlerts); + const trackedAlertRecoveredIds = Object.keys(state.trackedAlertsRecovered); const newAlertIds = difference(currentAlertIds, trackedAlertIds); const allAlertIds = [...new Set(currentAlertIds.concat(trackedAlertIds))]; @@ -249,13 +263,31 @@ export const createLifecycleExecutor = const isRecovered = !currentAlerts[alertId]; const isActive = !isRecovered; - const { alertUuid, started } = !isNew + const flappingHistory = getUpdatedFlappingHistory( + alertId, + state, + isNew, + isRecovered, + isActive, + trackedAlertRecoveredIds + ); + + const { + alertUuid, + started, + flapping: isCurrentlyFlapping, + } = !isNew ? state.trackedAlerts[alertId] : { alertUuid: lifecycleAlertServices.getAlertUuid(alertId), started: commonRuleFields[TIMESTAMP], + flapping: state.trackedAlertsRecovered[alertId] + ? state.trackedAlertsRecovered[alertId].flapping + : false, }; + const flapping = isFlapping(flappingHistory, isCurrentlyFlapping); + const event: ParsedTechnicalFields & ParsedExperimentalFields = { ...alertData?.fields, ...commonRuleFields, @@ -276,17 +308,21 @@ export const createLifecycleExecutor = [EVENT_ACTION]: isNew ? 'open' : isActive ? 'active' : 'close', [TAGS]: options.rule.tags, [VERSION]: ruleDataClient.kibanaVersion, + [ALERT_FLAPPING]: flapping, ...(isRecovered ? { [ALERT_END]: commonRuleFields[TIMESTAMP] } : {}), }; return { indexName: alertData?.indexName, event, + flappingHistory, + flapping, }; }); const trackedEventsToIndex = makeEventsDataMapFor(trackedAlertIds); const newEventsToIndex = makeEventsDataMapFor(newAlertIds); + const trackedRecoveredEventsToIndex = makeEventsDataMapFor(trackedAlertRecoveredIds); const allEventsToIndex = [...trackedEventsToIndex, ...newEventsToIndex]; // Only write alerts if: @@ -317,16 +353,35 @@ export const createLifecycleExecutor = const nextTrackedAlerts = Object.fromEntries( allEventsToIndex .filter(({ event }) => event[ALERT_STATUS] !== ALERT_STATUS_RECOVERED) - .map(({ event }) => { + .map(({ event, flappingHistory, flapping }) => { + const alertId = event[ALERT_INSTANCE_ID]!; + const alertUuid = event[ALERT_UUID]!; + const started = new Date(event[ALERT_START]!).toISOString(); + return [alertId, { alertId, alertUuid, started, flappingHistory, flapping }]; + }) + ); + + const nextTrackedAlertsRecovered = Object.fromEntries( + [...allEventsToIndex, ...trackedRecoveredEventsToIndex] + .filter( + ({ event, flappingHistory, flapping }) => + // return recovered alerts if they are flapping or if the flapping array is not at capacity + // this is a space saving effort that will stop tracking a recovered alert if it wasn't flapping and doesn't have state changes + // in the last max capcity number of executions + event[ALERT_STATUS] === ALERT_STATUS_RECOVERED && + (flapping || flappingHistory.filter((f) => f).length > 0) + ) + .map(({ event, flappingHistory, flapping }) => { const alertId = event[ALERT_INSTANCE_ID]!; const alertUuid = event[ALERT_UUID]!; const started = new Date(event[ALERT_START]!).toISOString(); - return [alertId, { alertId, alertUuid, started }]; + return [alertId, { alertId, alertUuid, started, flappingHistory, flapping }]; }) ); return { wrapped: nextWrappedState ?? ({} as State), trackedAlerts: writeAlerts ? nextTrackedAlerts : {}, + trackedAlertsRecovered: writeAlerts ? nextTrackedAlertsRecovered : {}, }; }; diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts index 74f79751dd57b..ee5ea65e9c79c 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts @@ -235,6 +235,7 @@ describe('createLifecycleRuleTypeFactory', () => { "event.action": "open", "event.kind": "signal", "kibana.alert.duration.us": 0, + "kibana.alert.flapping": false, "kibana.alert.instance.id": "opbeans-java", "kibana.alert.rule.category": "ruleTypeName", "kibana.alert.rule.consumer": "consumer", @@ -266,6 +267,7 @@ describe('createLifecycleRuleTypeFactory', () => { "event.action": "open", "event.kind": "signal", "kibana.alert.duration.us": 0, + "kibana.alert.flapping": false, "kibana.alert.instance.id": "opbeans-node", "kibana.alert.rule.category": "ruleTypeName", "kibana.alert.rule.consumer": "consumer", diff --git a/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.test.ts b/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.test.ts new file mode 100644 index 0000000000000..af5eb3e3698fe --- /dev/null +++ b/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.test.ts @@ -0,0 +1,121 @@ +/* + * 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 { getUpdatedFlappingHistory } from './get_updated_flapping_history'; + +describe('getUpdatedFlappingHistory', () => { + type TestRuleState = Record & { + aRuleStateKey: string; + }; + const initialRuleState: TestRuleState = { + aRuleStateKey: 'INITIAL_RULE_STATE_VALUE', + }; + + test('sets flapping state to true if the alert is new', () => { + const state = { wrapped: initialRuleState, trackedAlerts: {}, trackedAlertsRecovered: {} }; + expect(getUpdatedFlappingHistory('TEST_ALERT_0', state, true, false, false, [])) + .toMatchInlineSnapshot(` + Array [ + true, + ] + `); + }); + + test('sets flapping state to false on an alert that is still active', () => { + const state = { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlertsRecovered: {}, + }; + expect(getUpdatedFlappingHistory('TEST_ALERT_0', state, false, false, true, [])) + .toMatchInlineSnapshot(` + Array [ + false, + ] + `); + }); + + test('sets flapping state to true on an alert that is active and previously recovered', () => { + const state = { + wrapped: initialRuleState, + trackedAlertsRecovered: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlerts: {}, + }; + const recoveredIds = ['TEST_ALERT_0']; + expect(getUpdatedFlappingHistory('TEST_ALERT_0', state, true, false, true, recoveredIds)) + .toMatchInlineSnapshot(` + Array [ + true, + ] + `); + expect(recoveredIds).toEqual([]); + }); + + test('sets flapping state to true on an alert that is recovered and previously active', () => { + const state = { + wrapped: initialRuleState, + trackedAlerts: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + trackedAlertsRecovered: {}, + }; + const recoveredIds = ['TEST_ALERT_0']; + expect(getUpdatedFlappingHistory('TEST_ALERT_0', state, false, true, false, recoveredIds)) + .toMatchInlineSnapshot(` + Array [ + true, + ] + `); + expect(recoveredIds).toEqual(['TEST_ALERT_0']); + }); + + test('sets flapping state to true on an alert that is still recovered', () => { + const state = { + wrapped: initialRuleState, + trackedAlerts: {}, + trackedAlertsRecovered: { + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + started: '2020-01-01T12:00:00.000Z', + flappingHistory: [], + flapping: false, + }, + }, + }; + const recoveredIds = ['TEST_ALERT_0']; + expect(getUpdatedFlappingHistory('TEST_ALERT_0', state, false, true, false, recoveredIds)) + .toMatchInlineSnapshot(` + Array [ + false, + ] + `); + expect(recoveredIds).toEqual(['TEST_ALERT_0']); + }); +}); diff --git a/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.ts b/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.ts new file mode 100644 index 0000000000000..0f64e5778f929 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/utils/get_updated_flapping_history.ts @@ -0,0 +1,50 @@ +/* + * 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 { RuleTypeState } from '@kbn/alerting-plugin/common'; +import { updateFlappingHistory } from '@kbn/alerting-plugin/server/lib'; +import { remove } from 'lodash'; +import { WrappedLifecycleRuleState } from './create_lifecycle_executor'; + +export function getUpdatedFlappingHistory( + alertId: string, + state: WrappedLifecycleRuleState, + isNew: boolean, + isRecovered: boolean, + isActive: boolean, + recoveredIds: string[] +) { + // duplicating this logic to determine flapping at this level + let flappingHistory: boolean[] = []; + if (isRecovered) { + if (state.trackedAlerts[alertId]) { + // this alert has flapped from active to recovered + flappingHistory = updateFlappingHistory(state.trackedAlerts[alertId].flappingHistory, true); + } else if (state.trackedAlertsRecovered[alertId]) { + // this alert is still recovered + flappingHistory = updateFlappingHistory( + state.trackedAlertsRecovered[alertId].flappingHistory, + false + ); + } + } else if (isNew) { + if (state.trackedAlertsRecovered[alertId]) { + // this alert has flapped from recovered to active + flappingHistory = updateFlappingHistory( + state.trackedAlertsRecovered[alertId].flappingHistory, + true + ); + remove(recoveredIds, (id) => id === alertId); + } else { + flappingHistory = updateFlappingHistory([], true); + } + } else if (isActive) { + // this alert is still active + flappingHistory = updateFlappingHistory(state.trackedAlerts[alertId].flappingHistory, false); + } + return flappingHistory; +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts index b5509096727fb..6de49d3d44915 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts @@ -530,6 +530,153 @@ export default function eventLogTests({ getService }: FtrProviderContext) { numRecoveredAlerts: 0, }); }); + + it('should generate expected events for flapping alerts that are mainly active', async () => { + const { body: createdAction } = await supertest + .post(`${getUrlPrefix(space.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + connector_type_id: 'test.noop', + config: {}, + secrets: {}, + }) + .expect(200); + + // pattern of when the alert should fire + const instance = [true, false, true, false].concat(new Array(22).fill(true)); + const pattern = { + instance, + }; + + const response = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + rule_type_id: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + params: { + pattern, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + ], + }) + ); + + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + // get the events we're expecting + const events = await retry.try(async () => { + return await getEventLog({ + getService, + spaceId: space.id, + type: 'alert', + id: alertId, + provider: 'alerting', + actions: new Map([ + // make sure the counts of the # of events per type are as expected + ['execute-start', { gte: 25 }], + ['execute', { gte: 25 }], + ['execute-action', { equal: 23 }], + ['new-instance', { equal: 3 }], + ['active-instance', { gte: 23 }], + ['recovered-instance', { equal: 2 }], + ]), + }); + }); + + const flapping = events + .filter( + (event) => + event?.event?.action === 'active-instance' || + event?.event?.action === 'recovered-instance' + ) + .map((event) => event?.kibana?.alert?.flapping); + const result = [false, false, false].concat(new Array(21).fill(true)).concat([false]); + expect(flapping).to.eql(result); + }); + + it('should generate expected events for flapping alerts that are mainly recovered', async () => { + const { body: createdAction } = await supertest + .post(`${getUrlPrefix(space.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + connector_type_id: 'test.noop', + config: {}, + secrets: {}, + }) + .expect(200); + + // pattern of when the alert should fire + const instance = [true, false, true].concat(new Array(18).fill(false)).concat(true); + const pattern = { + instance, + }; + + const response = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + rule_type_id: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + params: { + pattern, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + ], + }) + ); + + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + // get the events we're expecting + const events = await retry.try(async () => { + return await getEventLog({ + getService, + spaceId: space.id, + type: 'alert', + id: alertId, + provider: 'alerting', + actions: new Map([ + // make sure the counts of the # of events per type are as expected + ['execute-start', { gte: 20 }], + ['execute', { gte: 20 }], + ['execute-action', { equal: 3 }], + ['new-instance', { equal: 3 }], + ['active-instance', { gte: 3 }], + ['recovered-instance', { equal: 3 }], + ]), + }); + }); + + const flapping = events + .filter( + (event) => + event?.event?.action === 'active-instance' || + event?.event?.action === 'recovered-instance' + ) + .map((event) => event?.kibana?.alert?.flapping); + expect(flapping).to.eql([false, false, false, true, false, false]); + }); }); } }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log_alerts.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log_alerts.ts index 29046ae028f6a..86754ee31d14f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log_alerts.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log_alerts.ts @@ -93,12 +93,12 @@ export default function eventLogAlertTests({ getService }: FtrProviderContext) { start?: string; durationToDate?: string; } = {}; - + const flapping = []; for (let i = 0; i < instanceEvents.length; ++i) { + flapping.push(instanceEvents[i]?.kibana?.alert?.flapping); switch (instanceEvents[i]?.event?.action) { case 'new-instance': expect(instanceEvents[i]?.kibana?.alerting?.instance_id).to.equal('instance'); - expect(instanceEvents[i]?.kibana?.alert?.flapping).to.equal(false); // a new alert should generate a unique UUID for the duration of its activeness expect(instanceEvents[i]?.event?.end).to.be(undefined); @@ -109,7 +109,6 @@ export default function eventLogAlertTests({ getService }: FtrProviderContext) { case 'active-instance': expect(instanceEvents[i]?.kibana?.alerting?.instance_id).to.equal('instance'); - expect(instanceEvents[i]?.kibana?.alert?.flapping).to.equal(false); expect(instanceEvents[i]?.event?.start).to.equal(currentAlertSpan.start); expect(instanceEvents[i]?.event?.end).to.be(undefined); @@ -124,7 +123,6 @@ export default function eventLogAlertTests({ getService }: FtrProviderContext) { case 'recovered-instance': expect(instanceEvents[i]?.kibana?.alerting?.instance_id).to.equal('instance'); - expect(instanceEvents[i]?.kibana?.alert?.flapping).to.equal(false); expect(instanceEvents[i]?.event?.start).to.equal(currentAlertSpan.start); expect(instanceEvents[i]?.event?.end).not.to.be(undefined); expect( @@ -134,6 +132,7 @@ export default function eventLogAlertTests({ getService }: FtrProviderContext) { break; } } + expect(flapping).to.eql(new Array(instanceEvents.length - 1).fill(false).concat(true)); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/flapping_history.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/flapping_history.ts new file mode 100644 index 0000000000000..ac09451e862fb --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/flapping_history.ts @@ -0,0 +1,226 @@ +/* + * 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 expect from '@kbn/expect'; +import { get } from 'lodash'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../common/lib'; +import { Spaces } from '../../scenarios'; + +// eslint-disable-next-line import/no-default-export +export default function createFlappingHistoryTests({ getService }: FtrProviderContext) { + const es = getService('es'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); + const retry = getService('retry'); + const supertest = getService('supertest'); + const space = Spaces.default; + + const ACTIVE_PATH = 'alertInstances.instance.meta.flappingHistory'; + const RECOVERED_PATH = 'alertRecoveredInstances.instance.meta.flappingHistory'; + + describe('Flapping History', () => { + let actionId: string; + const objectRemover = new ObjectRemover(supertestWithoutAuth); + + before(async () => { + actionId = await createAction(); + }); + + after(async () => { + objectRemover.add(space.id, actionId, 'connector', 'actions'); + await objectRemover.removeAll(); + }); + + afterEach(() => objectRemover.removeAll()); + + it('should update flappingHistory when the alert flaps states', async () => { + let start = new Date().toISOString(); + const pattern = { + instance: [true, true, false, true], + }; + + const alertId = await createAlert(pattern, actionId); + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + let state = await getRuleState(start, alertId); + expect(get(state, ACTIVE_PATH)).to.eql([true]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 2, true); + expect(get(state, ACTIVE_PATH)).to.eql([true, false]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 3, true, true); + expect(get(state, RECOVERED_PATH)).to.eql([true, false, true]); + expect(state.alertInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 4, true); + expect(get(state, ACTIVE_PATH)).to.eql([true, false, true, true]); + expect(state.alertRecoveredInstances).to.eql({}); + }); + + it('should update flappingHistory when the alert remains active', async () => { + let start = new Date().toISOString(); + const pattern = { + instance: [true, true, true, true], + }; + + const alertId = await createAlert(pattern, actionId); + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + let state = await getRuleState(start, alertId); + expect(get(state, ACTIVE_PATH)).to.eql([true]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 2, true); + expect(get(state, ACTIVE_PATH)).to.eql([true, false]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 3, true); + expect(get(state, ACTIVE_PATH)).to.eql([true, false, false]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 4, true); + expect(get(state, ACTIVE_PATH)).to.eql([true, false, false, false]); + expect(state.alertRecoveredInstances).to.eql({}); + }); + + it('should update flappingHistory when the alert remains recovered', async () => { + let start = new Date().toISOString(); + const pattern = { + instance: [true, false, false, false, true], + }; + + const alertId = await createAlert(pattern, actionId); + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + let state = await getRuleState(start, alertId); + expect(get(state, ACTIVE_PATH)).to.eql([true]); + expect(state.alertRecoveredInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 2, true, true); + expect(get(state, RECOVERED_PATH)).to.eql([true, true]); + expect(state.alertInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 3, true, true); + expect(get(state, RECOVERED_PATH)).to.eql([true, true, false]); + expect(state.alertInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 4, true, true); + expect(get(state, RECOVERED_PATH)).to.eql([true, true, false, false]); + expect(state.alertInstances).to.eql({}); + + start = new Date().toISOString(); + state = await getRuleState(start, alertId, 5, true, false); + expect(get(state, ACTIVE_PATH)).to.eql([true, true, false, false, true]); + expect(state.alertRecoveredInstances).to.eql({}); + }); + }); + + async function getState(start: string, runs: number, recovered: boolean) { + const result: any = await retry.try(async () => { + const searchResult = await es.search({ + index: '.kibana_task_manager', + body: { + query: { + bool: { + must: [ + { + term: { + 'task.taskType': 'alerting:test.patternFiring', + }, + }, + { + range: { + 'task.scheduledAt': { + gte: start, + }, + }, + }, + ], + }, + }, + }, + }); + + const taskDoc: any = searchResult.hits.hits[0]; + const state = JSON.parse(taskDoc._source.task.state); + const flappingHistory = recovered + ? get(state, RECOVERED_PATH, []) + : get(state, ACTIVE_PATH, []); + if (flappingHistory.length !== runs) { + throw new Error(`Expected ${runs} rule executions but received ${flappingHistory.length}.`); + } + + return state; + }); + + return result; + } + + async function getRuleState( + start: string, + alertId: string, + runs: number = 1, + runRule: boolean = false, + recovered: boolean = false + ) { + if (runRule) { + const response = await supertest + .post(`${getUrlPrefix(space.id)}/internal/alerting/rule/${alertId}/_run_soon`) + .set('kbn-xsrf', 'foo'); + expect(response.status).to.eql(204); + } + return await getState(start, runs, recovered); + } + + async function createAlert(pattern: { instance: boolean[] }, actionId: string) { + const { body: createdAlert } = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + rule_type_id: 'test.patternFiring', + schedule: { interval: '24h' }, + throttle: null, + params: { + pattern, + }, + actions: [], + }) + ) + .expect(200); + return createdAlert.id; + } + + async function createAction() { + const { body: createdAction } = await supertest + .post(`${getUrlPrefix(space.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My action', + connector_type_id: 'test.index-record', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + secrets: { + encrypted: 'This value should be encrypted', + }, + }) + .expect(200); + return createdAction.id; + } +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index d0981fbc1e93a..c887a10aa14aa 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -50,6 +50,7 @@ export default function alertingTests({ loadTestFile, getService }: FtrProviderC loadTestFile(require.resolve('./capped_action_type')); loadTestFile(require.resolve('./scheduled_task_id')); loadTestFile(require.resolve('./run_soon')); + loadTestFile(require.resolve('./flapping_history')); loadTestFile(require.resolve('./check_registered_rule_types')); // Do not place test files here, due to https://github.com/elastic/kibana/issues/123059 diff --git a/x-pack/test/rule_registry/spaces_only/tests/trial/get_summarized_alerts.ts b/x-pack/test/rule_registry/spaces_only/tests/trial/get_summarized_alerts.ts index a5d6763c35c47..cc72f48d31613 100644 --- a/x-pack/test/rule_registry/spaces_only/tests/trial/get_summarized_alerts.ts +++ b/x-pack/test/rule_registry/spaces_only/tests/trial/get_summarized_alerts.ts @@ -180,7 +180,7 @@ export default function createGetSummarizedAlertsTest({ getService }: FtrProvide const getState = ( shouldTriggerAlert: boolean, alerts: Record - ) => ({ wrapped: { shouldTriggerAlert }, trackedAlerts: alerts }); + ) => ({ wrapped: { shouldTriggerAlert }, trackedAlerts: alerts, trackedAlertsRecovered: {} }); // Execute the rule the first time - this creates a new alert const preExecution1Start = new Date(); From 563748fef525a7c89227db89529c09af8b266188 Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Tue, 29 Nov 2022 21:07:46 +0200 Subject: [PATCH 6/6] Run all journeys before failing a performance run (#146605) ## Summary Follow up to https://github.com/elastic/kibana/pull/146129 See [comment](https://github.com/elastic/kibana/pull/146129#discussion_r1034858035) for more details. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --- src/dev/performance/run_performance_cli.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/dev/performance/run_performance_cli.ts b/src/dev/performance/run_performance_cli.ts index b827e49427b81..fea2b27cc2616 100644 --- a/src/dev/performance/run_performance_cli.ts +++ b/src/dev/performance/run_performance_cli.ts @@ -83,11 +83,22 @@ run( const journeys = await Fsp.readdir(journeyBasePath); log.info(`Found ${journeys.length} journeys to run`); + const failedJourneys = []; + for (const journey of journeys) { - await startEs(); - await runWarmup(journey); - await runTest(journey); - await procRunner.stop('es'); + try { + await startEs(); + await runWarmup(journey); + await runTest(journey); + await procRunner.stop('es'); + } catch (e) { + log.error(e); + failedJourneys.push(journey); + } + } + + if (failedJourneys.length > 0) { + throw new Error(`${failedJourneys.length} journeys failed: ${failedJourneys.join(',')}`); } }, {