From 81f71086c6bc09453edad62e70f9c6ce61c375a1 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 7 Jan 2021 13:22:50 +0000 Subject: [PATCH] [Logs UI] Fix alerts recovery (#87369) * Ensure that if alert instances are instantiated they are used in a way recognised by the framework --- .../log_threshold_executor.test.ts | 98 +------------------ .../log_threshold/log_threshold_executor.ts | 22 +---- 2 files changed, 7 insertions(+), 113 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.test.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.test.ts index e04fe338f3436..dea808a29d1cb 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.test.ts @@ -413,31 +413,6 @@ describe('Log threshold executor', () => { describe('Results processors', () => { describe('Can process ungrouped results', () => { - test('It handles the OK state correctly', () => { - const alertInstanceUpdaterMock = jest.fn(); - const alertParams = { - ...baseAlertParams, - criteria: [positiveCriteria[0]], - }; - const results = { - hits: { - total: { - value: 2, - }, - }, - } as UngroupedSearchQueryResponse; - processUngroupedResults( - results, - alertParams, - alertsMock.createAlertInstanceFactory, - alertInstanceUpdaterMock - ); - // First call, second argument - expect(alertInstanceUpdaterMock.mock.calls[0][1]).toBe(AlertStates.OK); - // First call, third argument - expect(alertInstanceUpdaterMock.mock.calls[0][2]).toBe(undefined); - }); - test('It handles the ALERT state correctly', () => { const alertInstanceUpdaterMock = jest.fn(); const alertParams = { @@ -475,68 +450,6 @@ describe('Log threshold executor', () => { }); describe('Can process grouped results', () => { - test('It handles the OK state correctly', () => { - const alertInstanceUpdaterMock = jest.fn(); - const alertParams = { - ...baseAlertParams, - criteria: [positiveCriteria[0]], - groupBy: ['host.name', 'event.dataset'], - }; - const results = [ - { - key: { - 'host.name': 'i-am-a-host-name', - 'event.dataset': 'i-am-a-dataset', - }, - doc_count: 100, - filtered_results: { - doc_count: 1, - }, - }, - { - key: { - 'host.name': 'i-am-a-host-name', - 'event.dataset': 'i-am-a-dataset', - }, - doc_count: 100, - filtered_results: { - doc_count: 2, - }, - }, - { - key: { - 'host.name': 'i-am-a-host-name', - 'event.dataset': 'i-am-a-dataset', - }, - doc_count: 100, - filtered_results: { - doc_count: 3, - }, - }, - ] as GroupedSearchQueryResponse['aggregations']['groups']['buckets']; - processGroupByResults( - results, - alertParams, - alertsMock.createAlertInstanceFactory, - alertInstanceUpdaterMock - ); - expect(alertInstanceUpdaterMock.mock.calls.length).toBe(3); - // First call, second argument - expect(alertInstanceUpdaterMock.mock.calls[0][1]).toBe(AlertStates.OK); - // First call, third argument - expect(alertInstanceUpdaterMock.mock.calls[0][2]).toBe(undefined); - - // Second call, second argument - expect(alertInstanceUpdaterMock.mock.calls[1][1]).toBe(AlertStates.OK); - // Second call, third argument - expect(alertInstanceUpdaterMock.mock.calls[1][2]).toBe(undefined); - - // Third call, second argument - expect(alertInstanceUpdaterMock.mock.calls[2][1]).toBe(AlertStates.OK); - // Third call, third argument - expect(alertInstanceUpdaterMock.mock.calls[2][2]).toBe(undefined); - }); - test('It handles the ALERT state correctly', () => { const alertInstanceUpdaterMock = jest.fn(); const alertParams = { @@ -583,7 +496,7 @@ describe('Log threshold executor', () => { alertsMock.createAlertInstanceFactory, alertInstanceUpdaterMock ); - expect(alertInstanceUpdaterMock.mock.calls.length).toBe(results.length); + expect(alertInstanceUpdaterMock.mock.calls.length).toBe(2); // First call, second argument expect(alertInstanceUpdaterMock.mock.calls[0][1]).toBe(AlertStates.ALERT); // First call, third argument @@ -600,14 +513,9 @@ describe('Log threshold executor', () => { ]); // Second call, second argument - expect(alertInstanceUpdaterMock.mock.calls[1][1]).toBe(AlertStates.OK); + expect(alertInstanceUpdaterMock.mock.calls[1][1]).toBe(AlertStates.ALERT); // Second call, third argument - expect(alertInstanceUpdaterMock.mock.calls[1][2]).toBe(undefined); - - // Third call, second argument - expect(alertInstanceUpdaterMock.mock.calls[2][1]).toBe(AlertStates.ALERT); - // Third call, third argument - expect(alertInstanceUpdaterMock.mock.calls[2][2]).toEqual([ + expect(alertInstanceUpdaterMock.mock.calls[1][2]).toEqual([ { actionGroup: 'logs.threshold.fired', context: { diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index f4a9e8fdef3ff..0044855a73f5c 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -72,7 +72,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => const sourceConfiguration = await sources.getSourceConfiguration(savedObjectsClient, 'default'); const indexPattern = sourceConfiguration.configuration.logAlias; const timestampField = sourceConfiguration.configuration.fields.timestamp; - const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY); try { const validatedParams = decodeOrThrow(alertParamsRT)(params); @@ -95,10 +94,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => ); } } catch (e) { - alertInstance.replaceState({ - alertState: AlertStates.ERROR, - }); - throw new Error(e); } }; @@ -198,11 +193,10 @@ export const processUngroupedResults = ( alertInstaceUpdater: AlertInstanceUpdater ) => { const { count, criteria } = params; - - const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY); const documentCount = results.hits.total.value; if (checkValueAgainstComparatorMap[count.comparator](documentCount, count.value)) { + const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY); alertInstaceUpdater(alertInstance, AlertStates.ALERT, [ { actionGroup: FIRED_ACTIONS.id, @@ -214,8 +208,6 @@ export const processUngroupedResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }; @@ -228,12 +220,12 @@ export const processUngroupedRatioResults = ( ) => { const { count, criteria } = params; - const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY); const numeratorCount = numeratorResults.hits.total.value; const denominatorCount = denominatorResults.hits.total.value; const ratio = getRatio(numeratorCount, denominatorCount); if (ratio !== undefined && checkValueAgainstComparatorMap[count.comparator](ratio, count.value)) { + const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY); alertInstaceUpdater(alertInstance, AlertStates.ALERT, [ { actionGroup: FIRED_ACTIONS.id, @@ -246,8 +238,6 @@ export const processUngroupedRatioResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }; @@ -286,10 +276,10 @@ export const processGroupByResults = ( const groupResults = getReducedGroupByResults(results); groupResults.forEach((group) => { - const alertInstance = alertInstanceFactory(group.name); const documentCount = group.documentCount; if (checkValueAgainstComparatorMap[count.comparator](documentCount, count.value)) { + const alertInstance = alertInstanceFactory(group.name); alertInstaceUpdater(alertInstance, AlertStates.ALERT, [ { actionGroup: FIRED_ACTIONS.id, @@ -301,8 +291,6 @@ export const processGroupByResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }); }; @@ -320,7 +308,6 @@ export const processGroupByRatioResults = ( const denominatorGroupResults = getReducedGroupByResults(denominatorResults); numeratorGroupResults.forEach((numeratorGroup) => { - const alertInstance = alertInstanceFactory(numeratorGroup.name); const numeratorDocumentCount = numeratorGroup.documentCount; const denominatorGroup = denominatorGroupResults.find( (_group) => _group.name === numeratorGroup.name @@ -333,6 +320,7 @@ export const processGroupByRatioResults = ( ratio !== undefined && checkValueAgainstComparatorMap[count.comparator](ratio, count.value) ) { + const alertInstance = alertInstanceFactory(numeratorGroup.name); alertInstaceUpdater(alertInstance, AlertStates.ALERT, [ { actionGroup: FIRED_ACTIONS.id, @@ -345,8 +333,6 @@ export const processGroupByRatioResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }); };