From 6ab0c68ae67f18f305a8d2eff4ca7ef5ec6f3529 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Thu, 28 Sep 2023 13:51:20 +0200 Subject: [PATCH] [AO] Unskip the custom threshold rule executor unit test (#167120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #162299 Fixes #162540 ## Summary This PR unskips the custom threshold rule executor unit test and removes the warning implementation from the BE. ## 🧪 How to test - Create a custom threshold rule, it should work as before. (Warning implementation logic was already removed from FE; this PR only removes the BE implementation.) --- .../metric_threshold_executor.test.ts | 1 - .../custom_threshold_executor.test.ts | 274 +++++------------- .../custom_threshold_executor.ts | 6 +- .../lib/create_bucket_selector.ts | 31 +- .../custom_threshold/lib/evaluate_rule.ts | 5 +- .../rules/custom_threshold/lib/get_data.ts | 17 +- .../register_custom_threshold_rule_type.ts | 1 - .../lib/rules/custom_threshold/types.ts | 2 - 8 files changed, 73 insertions(+), 264 deletions(-) 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 4b618dbd82e85..6eec0c9f28629 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 @@ -131,7 +131,6 @@ const setEvaluationResults = (response: Array>) => { jest.requireMock('./lib/evaluate_rule').evaluateRule.mockImplementation(() => response); }; -// FAILING: https://github.com/elastic/kibana/issues/155534 describe('The metric threshold alert type', () => { describe('querying the entire infrastructure', () => { afterAll(() => clearInstances()); diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts index 4a4640e6e1f2f..d05af5d609b8c 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts @@ -11,6 +11,8 @@ import { RuleExecutorServicesMock, alertsMock, } from '@kbn/alerting-plugin/server/mocks'; +import { searchSourceCommonMock } from '@kbn/data-plugin/common/search/search_source/mocks'; +import type { ISearchSource } from '@kbn/data-plugin/common'; import { LifecycleAlertServices } from '@kbn/rule-registry-plugin/server'; import { ruleRegistryMocks } from '@kbn/rule-registry-plugin/server/mocks'; import { @@ -69,6 +71,14 @@ const mockOptions = { executionId: '', startedAt: STARTED_AT_MOCK_DATE, previousStartedAt: null, + params: { + searchConfiguration: { + query: { + query: '', + language: 'kuery', + }, + }, + }, state: { wrapped: initialRuleState, trackedAlerts: { @@ -123,8 +133,7 @@ const setEvaluationResults = (response: Array>) => { jest.requireMock('./lib/evaluate_rule').evaluateRule.mockImplementation(() => response); }; -// FAILING: https://github.com/elastic/kibana/issues/155534 -describe.skip('The metric threshold alert type', () => { +describe('The metric threshold alert type', () => { describe('querying the entire infrastructure', () => { afterAll(() => clearInstances()); const instanceID = '*'; @@ -133,6 +142,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, criteria: [ { @@ -147,7 +157,6 @@ describe.skip('The metric threshold alert type', () => { comparator: Comparator, threshold: number[], shouldFire: boolean = false, - shouldWarn: boolean = false, isNoData: boolean = false ) => setEvaluationResults([ @@ -160,7 +169,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire, - shouldWarn, isNoData, bucketKey: { groupBy0: '*' }, }, @@ -224,7 +232,7 @@ describe.skip('The metric threshold alert type', () => { setResults(Comparator.GT, [0.75], true); await execute(Comparator.GT, [0.75]); const { action } = mostRecentAction(instanceID); - expect(action.group).toBe('*'); + expect(action.group).toBeUndefined(); expect(action.reason).toContain('is 1'); expect(action.reason).toContain('Alert when > 0.75'); expect(action.reason).toContain('test.metric.1'); @@ -237,7 +245,7 @@ describe.skip('The metric threshold alert type', () => { const execute = ( comparator: Comparator, threshold: number[], - groupBy: string[] = ['something'], + groupBy: string[] = ['groupByField'], metric?: string, state?: any ) => @@ -245,6 +253,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, groupBy, criteria: [ { @@ -270,7 +279,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -282,7 +290,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -303,7 +310,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -315,7 +321,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -336,7 +341,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -348,7 +352,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -369,7 +372,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -381,15 +383,14 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, }, ]); await execute(Comparator.GT, [0.75]); - expect(mostRecentAction(instanceIdA).action.group).toBe('a'); - expect(mostRecentAction(instanceIdB).action.group).toBe('b'); + expect(mostRecentAction(instanceIdA).action.group).toEqual({ groupByField: 'a' }); + expect(mostRecentAction(instanceIdB).action.group).toEqual({ groupByField: 'b' }); }); test('persists previous groups that go missing, until the groupBy param changes', async () => { setEvaluationResults([ @@ -402,7 +403,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -414,7 +414,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -426,7 +425,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'c' }, }, @@ -435,7 +433,7 @@ describe.skip('The metric threshold alert type', () => { const { state: stateResult1 } = await execute( Comparator.GT, [0.75], - ['something'], + ['groupByField'], 'test.metric.2' ); expect(stateResult1.missingGroups).toEqual(expect.arrayContaining([])); @@ -449,7 +447,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -461,7 +458,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -473,7 +469,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'c' }, }, @@ -482,7 +477,7 @@ describe.skip('The metric threshold alert type', () => { const { state: stateResult2 } = await execute( Comparator.GT, [0.75], - ['something'], + ['groupByField'], 'test.metric.1', stateResult1 ); @@ -499,7 +494,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -511,7 +505,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -520,7 +513,7 @@ describe.skip('The metric threshold alert type', () => { const { state: stateResult3 } = await execute( Comparator.GT, [0.75], - ['something', 'something-else'], + ['groupByField', 'groupByField-else'], 'test.metric.1', stateResult2 ); @@ -538,7 +531,8 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { - groupBy: ['something'], + ...mockOptions.params, + groupBy: ['groupByField'], criteria: [ { ...baseNonCountCriterion, @@ -547,7 +541,12 @@ describe.skip('The metric threshold alert type', () => { metric: metric ?? baseNonCountCriterion.metric, }, ], - filterQuery, + searchConfiguration: { + query: { + query: filterQuery, + language: 'kuery', + }, + }, }, state: state ?? mockOptions.state.wrapped, }); @@ -562,7 +561,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -574,7 +572,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -586,7 +583,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'c' }, }, @@ -609,7 +605,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -621,7 +616,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -633,7 +627,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'c' }, }, @@ -659,7 +652,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -671,7 +663,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -701,6 +692,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, groupBy, criteria: [ { @@ -731,7 +723,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'host-01' }, context: { @@ -746,7 +737,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'host-02' }, context: { @@ -784,6 +774,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, groupBy, criteria: [ { @@ -812,7 +803,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -838,6 +828,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, groupBy, criteria: [ @@ -866,7 +857,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -880,7 +870,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -901,7 +890,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -923,7 +911,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -935,7 +922,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -949,7 +935,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -961,7 +946,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -969,7 +953,7 @@ describe.skip('The metric threshold alert type', () => { ]); const instanceIdA = 'a'; const instanceIdB = 'b'; - await execute(Comparator.GT_OR_EQ, [1.0], [3.0], 'something'); + await execute(Comparator.GT_OR_EQ, [1.0], [3.0], 'groupByField'); expect(mostRecentAction(instanceIdA)).toBeAlertAction(); expect(mostRecentAction(instanceIdB)).toBe(undefined); }); @@ -984,7 +968,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -998,7 +981,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -1027,6 +1009,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, criteria: [ { @@ -1048,7 +1031,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1066,7 +1048,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1086,8 +1067,9 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, - groupBy: 'something', + groupBy: 'groupByField', criteria: [ { ...baseCountCriterion, @@ -1112,7 +1094,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1124,7 +1105,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1143,7 +1123,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1155,7 +1134,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1175,6 +1153,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, criteria: [ { ...baseNonCountCriterion, @@ -1197,7 +1176,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -1215,7 +1193,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -1233,6 +1210,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, criteria: [ { @@ -1256,7 +1234,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -1274,7 +1251,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: '*' }, }, @@ -1292,6 +1268,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, criteria: [ { @@ -1315,7 +1292,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1337,7 +1313,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1356,6 +1331,7 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { + ...mockOptions.params, sourceId, criteria: [ { @@ -1384,7 +1360,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: STARTED_AT_MOCK_DATE.toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1395,15 +1370,9 @@ describe.skip('The metric threshold alert type', () => { const recentAction = mostRecentAction(instanceID); expect(recentAction.action).toEqual({ alertDetailsUrl: '', - alertState: 'NO DATA', - group: '*', - groupByKeys: undefined, - metric: { condition0: 'test.metric.3', condition1: 'count' }, reason: 'test.metric.3 reported no data in the last 1m', - threshold: { condition0: ['1'], condition1: [30] }, timestamp: STARTED_AT_MOCK_DATE.toISOString(), - value: { condition0: '[NO DATA]', condition1: 0 }, - viewInAppUrl: 'http://localhost:5601/app/metrics/explorer', + value: ['[NO DATA]', 0], tags: [], }); expect(recentAction).toBeNoDataAction(); @@ -1421,7 +1390,8 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { - groupBy: 'something', + ...mockOptions.params, + groupBy: 'groupByField', sourceId: 'default', criteria: [ { @@ -1457,7 +1427,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1475,7 +1444,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1493,7 +1461,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1.0, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1505,7 +1472,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1531,7 +1497,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'a' }, }, @@ -1543,7 +1508,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'b' }, }, @@ -1565,7 +1529,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1577,7 +1540,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1589,7 +1551,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'c' }, }, @@ -1610,7 +1571,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1622,7 +1582,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1641,7 +1600,8 @@ describe.skip('The metric threshold alert type', () => { ...mockOptions, services, params: { - groupBy: 'something', + ...mockOptions.params, + groupBy: 'groupByField', sourceId: 'default', criteria: [ { @@ -1673,7 +1633,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1691,7 +1650,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: '*' }, }, @@ -1709,7 +1667,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 1, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'a' }, }, @@ -1721,7 +1678,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: 3, timestamp: new Date().toISOString(), shouldFire: true, - shouldWarn: false, isNoData: false, bucketKey: { groupBy0: 'b' }, }, @@ -1745,7 +1701,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'a' }, }, @@ -1757,7 +1712,6 @@ describe.skip('The metric threshold alert type', () => { currentValue: null, timestamp: new Date().toISOString(), shouldFire: false, - shouldWarn: false, isNoData: true, bucketKey: { groupBy0: 'b' }, }, @@ -1770,112 +1724,6 @@ describe.skip('The metric threshold alert type', () => { }); }); }); - - describe('attempting to use a malformed filterQuery', () => { - afterAll(() => clearInstances()); - const instanceID = '*'; - const execute = () => - executor({ - ...mockOptions, - services, - params: { - criteria: [ - { - ...baseNonCountCriterion, - }, - ], - sourceId: 'default', - filterQuery: - 'host.name:(look.there.is.no.space.after.these.parentheses)and uh.oh: "wow that is bad"', - }, - }); - test('reports an error', async () => { - await execute(); - expect(mostRecentAction(instanceID)).toBeErrorAction(); - }); - }); - - describe('querying the entire infrastructure with warning threshold', () => { - afterAll(() => clearInstances()); - const instanceID = '*'; - - const execute = () => - executor({ - ...mockOptions, - services, - params: { - sourceId: 'default', - criteria: [ - { - ...baseNonCountCriterion, - comparator: Comparator.GT, - threshold: [9999], - }, - ], - }, - }); - - const setResults = ({ - comparator = Comparator.GT, - threshold = [9999], - warningComparator = Comparator.GT, - warningThreshold = [2.49], - metric = 'test.metric.1', - currentValue = 7.59, - shouldWarn = false, - }) => - setEvaluationResults([ - { - '*': { - ...baseNonCountCriterion, - comparator, - threshold, - warningComparator, - warningThreshold, - metric, - currentValue, - timestamp: new Date().toISOString(), - shouldFire: false, - shouldWarn, - isNoData: false, - bucketKey: { groupBy0: '*' }, - }, - }, - ]); - - test('warns as expected with the > comparator', async () => { - setResults({ warningThreshold: [2.49], currentValue: 2.5, shouldWarn: true }); - await execute(); - expect(mostRecentAction(instanceID)).toBeWarnAction(); - - setResults({ warningThreshold: [2.49], currentValue: 1.23, shouldWarn: false }); - await execute(); - expect(mostRecentAction(instanceID)).toBe(undefined); - }); - - test('reports expected warning values to the action context', async () => { - setResults({ warningThreshold: [2.49], currentValue: 2.5, shouldWarn: true }); - await execute(); - - const { action } = mostRecentAction(instanceID); - expect(action.group).toBe('*'); - expect(action.reason).toBe('test.metric.1 is 2.5 in the last 1 min. Alert when > 2.49.'); - }); - - test('reports expected warning values to the action context for percentage metric', async () => { - setResults({ - warningThreshold: [0.81], - currentValue: 0.82, - shouldWarn: true, - metric: 'system.cpu.user.pct', - }); - await execute(); - - const { action } = mostRecentAction(instanceID); - expect(action.group).toBe('*'); - expect(action.reason).toBe('system.cpu.user.pct is 82% in the last 1 min. Alert when > 81%.'); - }); - }); }); const mockLibs: any = { @@ -1897,10 +1745,28 @@ const mockLibs: any = { const executor = createMetricThresholdExecutor(mockLibs); const alertsServices = alertsMock.createRuleExecutorServices(); +const mockedIndex = { + id: 'c34a7c79-a88b-4b4a-ad19-72f6d24104e4', + title: 'metrics-fake_hosts', + fieldFormatMap: {}, + typeMeta: {}, + timeFieldName: '@timestamp', +}; +const mockedDataView = { + getIndexPattern: () => 'mockedIndexPattern', + ...mockedIndex, +}; +const mockedSearchSource = { + getField: jest.fn(() => mockedDataView), +} as any as ISearchSource; const services: RuleExecutorServicesMock & LifecycleAlertServices = { ...alertsServices, ...ruleRegistryMocks.createLifecycleAlertServices(alertsServices), + searchSourceClient: { + ...searchSourceCommonMock, + create: jest.fn(() => Promise.resolve(mockedSearchSource)), + }, }; services.savedObjectsClient.get.mockImplementation(async (type: string, sourceId: string) => { if (sourceId === 'alternate') @@ -1957,12 +1823,12 @@ function clearInstances() { interface Action { id: string; - action: { alertState: string }; + action: { reason: string }; } expect.extend({ toBeAlertAction(action?: Action) { - const pass = action?.id === FIRED_ACTIONS.id && action?.action.alertState === 'ALERT'; + const pass = action?.id === FIRED_ACTIONS.id && !action?.action.reason.includes('no data'); const message = () => `expected ${action} to be an ALERT action`; return { message, @@ -1970,21 +1836,13 @@ expect.extend({ }; }, toBeNoDataAction(action?: Action) { - const pass = action?.id === NO_DATA_ACTIONS.id && action?.action.alertState === 'NO DATA'; + const pass = action?.id === NO_DATA_ACTIONS.id && action?.action.reason.includes('no data'); const message = () => `expected ${action} to be a NO DATA action`; return { message, pass, }; }, - toBeErrorAction(action?: Action) { - const pass = action?.id === FIRED_ACTIONS.id && action?.action.alertState === 'ERROR'; - const message = () => `expected ${action} to be an ERROR action`; - return { - message, - pass, - }; - }, }); declare global { @@ -1992,9 +1850,7 @@ declare global { namespace jest { interface Matchers { toBeAlertAction(action?: Action): R; - toBeWarnAction(action?: Action): R; toBeNoDataAction(action?: Action): R; - toBeErrorAction(action?: Action): R; } } } diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts index bff471aaea2ec..6f0f146f2a267 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts @@ -56,7 +56,7 @@ export type MetricThresholdAlertState = AlertState; // no specific instance stat export interface MetricThresholdAlertContext extends Record { alertDetailsUrl: string; - groupings?: object; + group?: object; reason?: string; timestamp: string; // ISO string value?: Array | null; @@ -306,7 +306,7 @@ export const createMetricThresholdExecutor = ({ alertsLocator, basePath.publicBaseUrl ), - groupings: groupByKeysObjectMapping[group], + group: groupByKeysObjectMapping[group], reason, timestamp, value: alertResults.map((result, index) => { @@ -347,7 +347,7 @@ export const createMetricThresholdExecutor = ({ alertsLocator, basePath.publicBaseUrl ), - groupings: groupByKeysObjectForRecovered[recoveredAlertId], + group: groupByKeysObjectForRecovered[recoveredAlertId], timestamp: startedAt.toISOString(), ...additionalContext, }); diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/create_bucket_selector.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/create_bucket_selector.ts index 6300bfac703f3..83c84af79eda6 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/create_bucket_selector.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/create_bucket_selector.ts @@ -7,19 +7,11 @@ import { Aggregators, - Comparator, MetricExpressionParams, } from '../../../../../common/custom_threshold_rule/types'; import { createConditionScript } from './create_condition_script'; import { createLastPeriod } from './wrap_in_period'; -const EMPTY_SHOULD_WARN = { - bucket_script: { - buckets_path: {}, - script: '0', - }, -}; - export const createBucketSelector = ( condition: MetricExpressionParams, alertOnGroupDisappear: boolean = false, @@ -28,7 +20,6 @@ export const createBucketSelector = ( lastPeriodEnd?: number ) => { const hasGroupBy = groupBy != null; - const hasWarn = condition.warningThreshold != null && condition.warningComparator != null; const isPercentile = [Aggregators.P95, Aggregators.P99].includes(condition.aggType); const isCount = condition.aggType === Aggregators.COUNT; const isRate = condition.aggType === Aggregators.RATE; @@ -42,20 +33,6 @@ export const createBucketSelector = ( }]` : "currentPeriod['all']>aggregatedValue"; - const shouldWarn = hasWarn - ? { - bucket_script: { - buckets_path: { - value: bucketPath, - }, - script: createConditionScript( - condition.warningThreshold as number[], - condition.warningComparator as Comparator - ), - }, - } - : EMPTY_SHOULD_WARN; - const shouldTrigger = { bucket_script: { buckets_path: { @@ -66,7 +43,6 @@ export const createBucketSelector = ( }; const aggs: any = { - shouldWarn, shouldTrigger, }; @@ -97,17 +73,16 @@ export const createBucketSelector = ( const evalutionBucketPath = alertOnGroupDisappear && lastPeriodEnd ? { - shouldWarn: 'shouldWarn', shouldTrigger: 'shouldTrigger', missingGroup: 'missingGroup', newOrRecoveredGroup: 'newOrRecoveredGroup', } - : { shouldWarn: 'shouldWarn', shouldTrigger: 'shouldTrigger' }; + : { shouldTrigger: 'shouldTrigger' }; const evaluationScript = alertOnGroupDisappear && lastPeriodEnd - ? '(params.missingGroup != null && params.missingGroup > 0) || (params.shouldWarn != null && params.shouldWarn > 0) || (params.shouldTrigger != null && params.shouldTrigger > 0) || (params.newOrRecoveredGroup != null && params.newOrRecoveredGroup > 0)' - : '(params.shouldWarn != null && params.shouldWarn > 0) || (params.shouldTrigger != null && params.shouldTrigger > 0)'; + ? '(params.missingGroup != null && params.missingGroup > 0) || (params.shouldTrigger != null && params.shouldTrigger > 0) || (params.newOrRecoveredGroup != null && params.newOrRecoveredGroup > 0)' + : '(params.shouldTrigger != null && params.shouldTrigger > 0)'; aggs.evaluation = { bucket_selector: { diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts index 66007de19b622..33410cbfb9742 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts @@ -28,7 +28,6 @@ export type Evaluation = Omit & { currentValue: number | null; timestamp: string; shouldFire: boolean; - shouldWarn: boolean; isNoData: boolean; bucketKey: Record; context?: AdditionalContext; @@ -91,7 +90,6 @@ export const evaluateRule = async = {}; for (const key of Object.keys(currentValues)) { const result = currentValues[key]; - if (result.trigger || result.warn || result.value === null) { + if (result.trigger || result.value === null) { evaluations[key] = { ...criterion, metric: @@ -114,7 +112,6 @@ export const evaluateRule = async 0) { previous[key] = { trigger: false, - warn: false, value: null, bucketKey: bucket.key, }; @@ -179,7 +172,6 @@ export const getData = async ( previous[key] = { trigger: (shouldTrigger && shouldTrigger.value > 0) || false, - warn: (shouldWarn && shouldWarn.value > 0) || false, value, bucketKey: bucket.key, container: containerList, @@ -210,7 +202,6 @@ export const getData = async ( const { currentPeriod, aggregatedValue: aggregatedValueForRate, - shouldWarn, shouldTrigger, } = aggs.all.buckets.all; @@ -224,20 +215,15 @@ export const getData = async ( : aggregatedValue != null ? getValue(aggregatedValue, params) : null; - // There is an edge case where there is no results and the shouldWarn/shouldTrigger + // There is an edge case where there is no results and the shouldTrigger // bucket scripts will be missing. This is only an issue for document count because // the value will end up being ZERO, for other metrics it will be null. In this case // we need to do the evaluation in Node.js if (aggs.all && params.aggType === Aggregators.COUNT && value === 0) { const trigger = comparatorMap[params.comparator](value, params.threshold); - const warn = - params.warningThreshold && params.warningComparator - ? comparatorMap[params.warningComparator](value, params.warningThreshold) - : false; return { [UNGROUPED_FACTORY_KEY]: { value, - warn, trigger, bucketKey: { groupBy0: UNGROUPED_FACTORY_KEY }, }, @@ -246,7 +232,6 @@ export const getData = async ( return { [UNGROUPED_FACTORY_KEY]: { value, - warn: (shouldWarn && shouldWarn.value > 0) || false, trigger: (shouldTrigger && shouldTrigger.value > 0) || false, bucketKey: { groupBy0: UNGROUPED_FACTORY_KEY }, }, diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/register_custom_threshold_rule_type.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/register_custom_threshold_rule_type.ts index a3bd05e4316a8..a6466deb25a47 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/register_custom_threshold_rule_type.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/register_custom_threshold_rule_type.ts @@ -69,7 +69,6 @@ export function thresholdRuleType( comparator: oneOfLiterals(Object.values(Comparator)), timeUnit: schema.string(), timeSize: schema.number(), - warningComparator: schema.maybe(oneOfLiterals(Object.values(Comparator))), }; const nonCountCriterion = schema.object({ diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/types.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/types.ts index e66042bcb648d..cc63f2cd688d3 100644 --- a/x-pack/plugins/observability/server/lib/rules/custom_threshold/types.ts +++ b/x-pack/plugins/observability/server/lib/rules/custom_threshold/types.ts @@ -51,8 +51,6 @@ interface BaseMetricExpressionParams { timeUnit: TimeUnitChar; threshold: number[]; comparator: Comparator; - warningComparator?: Comparator; - warningThreshold?: number[]; } export interface NonCountMetricExpressionParams extends BaseMetricExpressionParams {