From eb0e6bae5c04baf315764df635167f4bb2a3cd8e Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 7 Jan 2021 15:53:00 +0000 Subject: [PATCH] [Logs UI] Fix alerts recovery (#87369) (#87625) * 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 70633ec03795f..6c6b75baa5ce9 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 @@ -53,7 +53,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); @@ -76,10 +75,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => ); } } catch (e) { - alertInstance.replaceState({ - alertState: AlertStates.ERROR, - }); - throw new Error(e); } }; @@ -179,11 +174,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, @@ -195,8 +189,6 @@ export const processUngroupedResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }; @@ -209,12 +201,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, @@ -227,8 +219,6 @@ export const processUngroupedRatioResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }; @@ -267,10 +257,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, @@ -282,8 +272,6 @@ export const processGroupByResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }); }; @@ -301,7 +289,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 @@ -314,6 +301,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, @@ -326,8 +314,6 @@ export const processGroupByRatioResults = ( }, }, ]); - } else { - alertInstaceUpdater(alertInstance, AlertStates.OK); } }); };