From 999eed43333428538b1adfef6419429202cf3868 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 7 Jul 2021 10:59:48 -0600 Subject: [PATCH 01/15] [Metrics UI] Change dropLastBucket to dropPartialBuckets - Change offset calculation to millisecond percission - Change dropLastBucket to dropPartialBuckets - Impliment partial bucket filter - Adding partial bucket filter to metric threshold alerts --- .../infra/common/http_api/metrics_api.ts | 2 +- .../metric_threshold/lib/evaluate_alert.ts | 87 +++++++++++++++---- .../metric_threshold/lib/metric_query.ts | 2 +- .../plugins/infra/server/lib/metrics/index.ts | 10 ++- ...stogram_buckets_to_timeseries.test.ts.snap | 50 +++++++++-- .../calculate_date_histogram_offset.test.ts | 32 ++++++- .../lib/calculate_date_histogram_offset.ts | 7 +- ...rt_histogram_buckets_to_timeseries.test.ts | 53 ++++++++--- ...convert_histogram_buckets_to_timeseries.ts | 13 ++- ...ert_request_to_metrics_api_options.test.ts | 2 +- .../convert_request_to_metrics_api_options.ts | 2 +- ...orm_request_to_metrics_api_request.test.ts | 1 + ...ransform_request_to_metrics_api_request.ts | 1 + 13 files changed, 215 insertions(+), 47 deletions(-) diff --git a/x-pack/plugins/infra/common/http_api/metrics_api.ts b/x-pack/plugins/infra/common/http_api/metrics_api.ts index e57efd84d22995..94d26abc701fe5 100644 --- a/x-pack/plugins/infra/common/http_api/metrics_api.ts +++ b/x-pack/plugins/infra/common/http_api/metrics_api.ts @@ -35,7 +35,7 @@ export const MetricsAPIRequestRT = rt.intersection([ afterKey: rt.union([rt.null, afterKeyObjectRT]), limit: rt.union([rt.number, rt.null, rt.undefined]), filters: rt.array(rt.object), - dropLastBucket: rt.boolean, + dropPartialBuckets: rt.boolean, alignDataToEnd: rt.boolean, }), ]); diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts index 144ee6505c5931..408b5bfdfbad49 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts @@ -11,6 +11,8 @@ import { isTooManyBucketsPreviewException, TOO_MANY_BUCKETS_PREVIEW_EXCEPTION, } from '../../../../../common/alerting/metrics'; +import { getIntervalInSeconds } from '../../../../utils/get_interval_in_seconds'; +import { roundTimestamp } from '../../../../utils/round_timestamp'; import { InfraSource } from '../../../../../common/source_configuration/source_configuration'; import { InfraDatabaseSearchResponse } from '../../../adapters/framework/adapter_types'; import { createAfterKeyHandler } from '../../../../utils/create_afterkey_handler'; @@ -26,6 +28,7 @@ interface Aggregation { aggregatedValue: { value: number; values?: Array<{ key: number; value: number }> }; doc_count: number; to_as_string: string; + from_as_string: string; key_as_string: string; }>; }; @@ -92,6 +95,8 @@ export const evaluateAlert = value) - .join(', ')]: getValuesFromAggregations(bucket, aggType), + .join(', ')]: getValuesFromAggregations(bucket, aggType, { + from, + to, + bucketSizeInMillis: intervalAsMS, + }), }), {} ); @@ -153,7 +177,8 @@ const getMetric: ( return { [UNGROUPED_FACTORY_KEY]: getValuesFromAggregations( (result.aggregations! as unknown) as Aggregation, - aggType + aggType, + { from, to, bucketSizeInMillis: intervalAsMS } ), }; } catch (e) { @@ -173,31 +198,55 @@ const getMetric: ( } }; +interface DropPartialBucketOptions { + from: number; + to: number; + bucketSizeInMillis: number; +} + +const dropPartialBuckets = ({ from, to, bucketSizeInMillis }: DropPartialBucketOptions) => ( + row: { + key: string; + value: number; + } | null +) => { + if (row == null) return null; + const timestamp = new Date(row.key).valueOf(); + return timestamp >= from && timestamp + bucketSizeInMillis <= to; +}; + const getValuesFromAggregations = ( aggregations: Aggregation, - aggType: MetricExpressionParams['aggType'] + aggType: MetricExpressionParams['aggType'], + dropPartialBucketsOptions: DropPartialBucketOptions ) => { try { const { buckets } = aggregations.aggregatedIntervals; if (!buckets.length) return null; // No Data state if (aggType === Aggregators.COUNT) { - return buckets.map((bucket) => ({ - key: bucket.to_as_string, - value: bucket.doc_count, - })); + return buckets + .map((bucket) => ({ + key: bucket.to_as_string, + value: bucket.doc_count, + })) + .filter(dropPartialBuckets(dropPartialBucketsOptions)); } if (aggType === Aggregators.P95 || aggType === Aggregators.P99) { - return buckets.map((bucket) => { - const values = bucket.aggregatedValue?.values || []; - const firstValue = first(values); - if (!firstValue) return null; - return { key: bucket.to_as_string, value: firstValue.value }; - }); + return buckets + .map((bucket) => { + const values = bucket.aggregatedValue?.values || []; + const firstValue = first(values); + if (!firstValue) return null; + return { key: bucket.to_as_string, value: firstValue.value }; + }) + .filter(dropPartialBuckets(dropPartialBucketsOptions)); } - return buckets.map((bucket) => ({ - key: bucket.key_as_string ?? bucket.to_as_string, - value: bucket.aggregatedValue?.value ?? null, - })); + return buckets + .map((bucket) => ({ + key: bucket.key_as_string ?? bucket.from_as_string, + value: bucket.aggregatedValue?.value ?? null, + })) + .filter(dropPartialBuckets(dropPartialBucketsOptions)); } catch (e) { return NaN; // Error state } diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts index 0e495c08cc9fd7..6a27fcfb7d4098 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts @@ -50,7 +50,7 @@ export const getElasticsearchMetricQuery = ( ); const offset = calculateDateHistogramOffset({ from, to, interval, field: timefield }); - const offsetInMS = parseInt(offset, 10) * 1000; + const offsetInMS = parseInt(offset, 10); const aggregations = aggType === Aggregators.COUNT diff --git a/x-pack/plugins/infra/server/lib/metrics/index.ts b/x-pack/plugins/infra/server/lib/metrics/index.ts index e436ad2ba0b05f..a2d9ad2b401d47 100644 --- a/x-pack/plugins/infra/server/lib/metrics/index.ts +++ b/x-pack/plugins/infra/server/lib/metrics/index.ts @@ -91,7 +91,12 @@ export const query = async ( return { series: groupings.buckets.map((bucket) => { const keys = Object.values(bucket.key); - return convertHistogramBucketsToTimeseries(keys, options, bucket.histogram.buckets); + return convertHistogramBucketsToTimeseries( + keys, + options, + bucket.histogram.buckets, + bucketSize * 1000 + ); }), info: { afterKey: returnAfterKey ? afterKey : null, @@ -108,7 +113,8 @@ export const query = async ( convertHistogramBucketsToTimeseries( ['*'], options, - response.aggregations.histogram.buckets + response.aggregations.histogram.buckets, + bucketSize * 1000 ), ], info: { diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/convert_histogram_buckets_to_timeseries.test.ts.snap b/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/convert_histogram_buckets_to_timeseries.test.ts.snap index 6d3c0bf71cae78..5ec5166c8deb44 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/convert_histogram_buckets_to_timeseries.test.ts.snap +++ b/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/convert_histogram_buckets_to_timeseries.test.ts.snap @@ -29,6 +29,14 @@ Object { "metric_0": 1, "timestamp": 1577836920000, }, + Object { + "metric_0": 1, + "timestamp": 1577836980000, + }, + Object { + "metric_0": 1, + "timestamp": 1577837040000, + }, ], } `; @@ -62,9 +70,17 @@ Object { "metric_0": 1, "timestamp": 1577836920000, }, + Object { + "metric_0": 1, + "timestamp": 1577836980000, + }, + Object { + "metric_0": 1, + "timestamp": 1577837040000, + }, Object { "metric_0": null, - "timestamp": 1577836920000, + "timestamp": 1577837100000, }, ], } @@ -81,7 +97,7 @@ Object { } `; -exports[`convertHistogramBucketsToTimeseies(keys, options, buckets) should tranform top_metric aggregations 1`] = ` +exports[`convertHistogramBucketsToTimeseies(keys, options, buckets) should transform top_metric aggregations 1`] = ` Object { "columns": Array [ Object { @@ -152,7 +168,15 @@ Object { }, Object { "metric_0": 4, - "timestamp": 1577836920000, + "timestamp": 1577836980000, + }, + Object { + "metric_0": 4, + "timestamp": 1577837040000, + }, + Object { + "metric_0": 4, + "timestamp": 1577837100000, }, ], } @@ -187,9 +211,17 @@ Object { "metric_0": 2, "timestamp": 1577836920000, }, + Object { + "metric_0": 2, + "timestamp": 1577836980000, + }, + Object { + "metric_0": 2, + "timestamp": 1577837040000, + }, Object { "metric_0": null, - "timestamp": 1577836920000, + "timestamp": 1577837100000, }, ], } @@ -226,7 +258,15 @@ Object { }, Object { "metric_0": 3, - "timestamp": 1577836920000, + "timestamp": 1577836980000, + }, + Object { + "metric_0": 3, + "timestamp": 1577837040000, + }, + Object { + "metric_0": 3, + "timestamp": 1577837100000, }, ], } diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.test.ts b/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.test.ts index 7cb76ee088c1a4..ef0f64855acd86 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.test.ts +++ b/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.test.ts @@ -17,6 +17,36 @@ describe('calculateDateHistogramOffset(timerange)', () => { field: '@timestamp', }; const offset = calculateDateHistogramOffset(timerange); - expect(offset).toBe('-28s'); + expect(offset).toBe('-28000ms'); + }); + it('should work with un-even timeranges (60s buckets)', () => { + const timerange = { + from: 1625057349373, + to: 1625057649373, + interval: '60s', + field: '@timestamp', + }; + const offset = calculateDateHistogramOffset(timerange); + expect(offset).toBe('-51373ms'); + }); + it('should work with un-even timeranges (5m buckets)', () => { + const timerange = { + from: 1625516185059, + to: 1625602885059, + interval: '5m', + field: '@timestamp', + }; + const offset = calculateDateHistogramOffset(timerange); + expect(offset).toBe('-215059ms'); + }); + it('should work with un-even timeranges (>=10s buckets)', () => { + const timerange = { + from: 1625516185059, + to: 1625602885059, + interval: '>=10s', + field: '@timestamp', + }; + const offset = calculateDateHistogramOffset(timerange); + expect(offset).toBe('-215059ms'); }); }); diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.ts b/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.ts index 1124b95e606de5..6f35a624a11dac 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.ts +++ b/x-pack/plugins/infra/server/lib/metrics/lib/calculate_date_histogram_offset.ts @@ -14,5 +14,10 @@ export const calculateDateHistogramOffset = (timerange: MetricsAPITimerange): st // negative offset to align buckets with full intervals (e.g. minutes) const offset = (fromInSeconds % bucketSize) - bucketSize; - return `${offset}s`; + + // Because everything is being rounded to the nearest second, except the timerange, + // we need to adjust the buckets to account for the millisecond offset otherwise + // the last bucket will be only contain the difference. + const millisOffset = timerange.to % 1000; + return `${offset * 1000 - millisOffset}ms`; }; diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.test.ts b/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.test.ts index 31478c5f1aa4ce..b49560f8c25f6e 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.test.ts +++ b/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.test.ts @@ -15,7 +15,7 @@ const options: MetricsAPIRequest = { timerange: { field: '@timestamp', from: moment('2020-01-01T00:00:00Z').valueOf(), - to: moment('2020-01-01T01:00:00Z').valueOf(), + to: moment('2020-01-01T00:00:00Z').add(5, 'minute').valueOf(), interval: '1m', }, limit: 9, @@ -45,8 +45,20 @@ const buckets = [ metric_0: { value: 1 }, }, { - key: moment('2020-01-01T00:00:00Z').add(2, 'minute').valueOf(), - key_as_string: moment('2020-01-01T00:00:00Z').add(2, 'minute').toISOString(), + key: moment('2020-01-01T00:00:00Z').add(3, 'minute').valueOf(), + key_as_string: moment('2020-01-01T00:00:00Z').add(3, 'minute').toISOString(), + doc_count: 1, + metric_0: { value: 1 }, + }, + { + key: moment('2020-01-01T00:00:00Z').add(4, 'minute').valueOf(), + key_as_string: moment('2020-01-01T00:00:00Z').add(4, 'minute').toISOString(), + doc_count: 1, + metric_0: { value: 1 }, + }, + { + key: moment('2020-01-01T00:00:00Z').add(5, 'minute').valueOf(), + key_as_string: moment('2020-01-01T00:00:00Z').add(5, 'minute').toISOString(), doc_count: 1, metric_0: { value: null }, }, @@ -54,16 +66,21 @@ const buckets = [ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { it('should just work', () => { - expect(convertHistogramBucketsToTimeseries(keys, options, buckets)).toMatchSnapshot(); + expect(convertHistogramBucketsToTimeseries(keys, options, buckets, 60000)).toMatchSnapshot(); }); it('should drop the last bucket', () => { expect( - convertHistogramBucketsToTimeseries(keys, { ...options, dropLastBucket: true }, buckets) + convertHistogramBucketsToTimeseries( + keys, + { ...options, dropPartialBuckets: true }, + buckets, + 60000 + ) ).toMatchSnapshot(); }); it('should return empty timeseries for empty metrics', () => { expect( - convertHistogramBucketsToTimeseries(keys, { ...options, metrics: [] }, buckets) + convertHistogramBucketsToTimeseries(keys, { ...options, metrics: [] }, buckets, 60000) ).toMatchSnapshot(); }); it('should work with normalized_values', () => { @@ -75,7 +92,7 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { return bucket; }); expect( - convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithNormalizedValue) + convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithNormalizedValue, 60000) ).toMatchSnapshot(); }); it('should work with percentiles', () => { @@ -83,7 +100,7 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { return { ...bucket, metric_0: { values: { '95.0': 3 } } }; }); expect( - convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithPercentiles) + convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithPercentiles, 60000) ).toMatchSnapshot(); }); it('should throw error with multiple percentiles', () => { @@ -91,7 +108,12 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { return { ...bucket, metric_0: { values: { '95.0': 3, '99.0': 4 } } }; }); expect(() => - convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithMultiplePercentiles) + convertHistogramBucketsToTimeseries( + keys, + { ...options }, + bucketsWithMultiplePercentiles, + 60000 + ) ).toThrow(); }); it('should work with keyed percentiles', () => { @@ -99,7 +121,7 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { return { ...bucket, metric_0: { values: [{ key: '99.0', value: 4 }] } }; }); expect( - convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithKeyedPercentiles) + convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithKeyedPercentiles, 60000) ).toMatchSnapshot(); }); it('should throw error with multiple keyed percentiles', () => { @@ -115,11 +137,16 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { }; }); expect(() => - convertHistogramBucketsToTimeseries(keys, { ...options }, bucketsWithMultipleKeyedPercentiles) + convertHistogramBucketsToTimeseries( + keys, + { ...options }, + bucketsWithMultipleKeyedPercentiles, + 60000 + ) ).toThrow(); }); - it('should tranform top_metric aggregations', () => { + it('should transform top_metric aggregations', () => { const topMetricOptions: MetricsAPIRequest = { ...options, metrics: [ @@ -167,7 +194,7 @@ describe('convertHistogramBucketsToTimeseies(keys, options, buckets)', () => { ]; expect( - convertHistogramBucketsToTimeseries(keys, topMetricOptions, bucketsWithTopAggregation) + convertHistogramBucketsToTimeseries(keys, topMetricOptions, bucketsWithTopAggregation, 60000) ).toMatchSnapshot(); }); }); diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.ts b/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.ts index ed1ffe4e364d2e..f6761f72fabb92 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.ts +++ b/x-pack/plugins/infra/server/lib/metrics/lib/convert_histogram_buckets_to_timeseries.ts @@ -64,6 +64,10 @@ const getValue = (valueObject: ValueObjectType) => { return null; }; +const dropOutOfBoundsBuckets = (from: number, to: number, bucketSizeInMillis: number) => ( + row: MetricsAPIRow +) => row.timestamp >= from && row.timestamp + bucketSizeInMillis <= to; + const convertBucketsToRows = ( options: MetricsAPIRequest, buckets: HistogramBucket[] @@ -81,7 +85,8 @@ const convertBucketsToRows = ( export const convertHistogramBucketsToTimeseries = ( keys: string[], options: MetricsAPIRequest, - buckets: HistogramBucket[] + buckets: HistogramBucket[], + bucketSizeInMillis: number ): MetricsAPISeries => { const id = keys.join(':'); // If there are no metrics then we just return the empty series @@ -94,7 +99,11 @@ export const convertHistogramBucketsToTimeseries = ( type: 'number', })) as MetricsAPIColumn[]; const allRows = convertBucketsToRows(options, buckets); - const rows = options.dropLastBucket ? allRows.slice(0, allRows.length - 1) : allRows; + const rows = options.dropPartialBuckets + ? allRows.filter( + dropOutOfBoundsBuckets(options.timerange.from, options.timerange.to, bucketSizeInMillis) + ) + : allRows; return { id, keys, diff --git a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.test.ts b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.test.ts index 0796433c29018c..539e9a1fee6ef0 100644 --- a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.test.ts +++ b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.test.ts @@ -28,7 +28,7 @@ const BASE_METRICS_UI_OPTIONS: MetricsAPIRequest = { interval: '1m', }, limit: 9, - dropLastBucket: true, + dropPartialBuckets: true, indexPattern: 'metrics-*', metrics: [ { id: 'metric_0', aggregations: { metric_0: { avg: { field: 'system.cpu.user.pct' } } } }, diff --git a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts index 6815dd31b2bfd3..b1a19bc6a8d322 100644 --- a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts +++ b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts @@ -26,7 +26,7 @@ export const convertRequestToMetricsAPIOptions = ( indexPattern, limit, metrics, - dropLastBucket: true, + dropPartialBuckets: true, }; if (options.afterKey) { diff --git a/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.test.ts b/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.test.ts index de44cf016886fa..b4e6983a099004 100644 --- a/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.test.ts +++ b/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.test.ts @@ -114,5 +114,6 @@ const metricsApiRequest = { ], limit: 3000, alignDataToEnd: true, + dropPartialBuckets: true, groupBy: ['kubernetes.pod.uid'], }; diff --git a/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.ts b/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.ts index 811b0da952456c..3901c8677ae9bf 100644 --- a/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.ts +++ b/x-pack/plugins/infra/server/routes/snapshot/lib/transform_request_to_metrics_api_request.ts @@ -47,6 +47,7 @@ export const transformRequestToMetricsAPIRequest = async ({ ? snapshotRequest.overrideCompositeSize : compositeSize, alignDataToEnd: true, + dropPartialBuckets: true, }; const filters = []; From ff598db1675c5c13a0a5e202d7c5449a1b01922d Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 7 Jul 2021 17:41:04 -0600 Subject: [PATCH 02/15] Cleaning up getElasticsearchMetricQuery --- .../metric_threshold/lib/evaluate_alert.ts | 4 ++-- .../metric_threshold/lib/metric_query.ts | 17 ++++------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts index 408b5bfdfbad49..9ceb2c8f2bef77 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts @@ -134,9 +134,9 @@ const getMetric: ( const searchBody = getElasticsearchMetricQuery( params, timefield, + { start: from, end: to }, hasGroupBy ? groupBy : undefined, - filterQuery, - { start: from, end: to } + filterQuery ); try { diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts index 6a27fcfb7d4098..9a8d728b01c8a7 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts @@ -25,9 +25,9 @@ const getParsedFilterQuery: (filterQuery: string | undefined) => Record { if (aggType === Aggregators.COUNT && metric) { throw new Error('Cannot aggregate document count with a metric'); @@ -38,17 +38,8 @@ export const getElasticsearchMetricQuery = ( const interval = `${timeSize}${timeUnit}`; const intervalAsSeconds = getIntervalInSeconds(interval); const intervalAsMS = intervalAsSeconds * 1000; - - const to = roundTimestamp(timeframe ? timeframe.end : Date.now(), timeUnit); - // We need enough data for 5 buckets worth of data. We also need - // to convert the intervalAsSeconds to milliseconds. - const minimumFrom = to - intervalAsMS * MINIMUM_BUCKETS; - - const from = roundTimestamp( - timeframe && timeframe.start <= minimumFrom ? timeframe.start : minimumFrom, - timeUnit - ); - + const to = timeframe.end; + const from = timeframe.start; const offset = calculateDateHistogramOffset({ from, to, interval, field: timefield }); const offsetInMS = parseInt(offset, 10); From a7089730bd1a18e34a2404fbdb1790f0fe69bdde Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Thu, 8 Jul 2021 13:06:46 -0600 Subject: [PATCH 03/15] Change timestamp to from_as_string to align to how date_histgram works --- .../lib/alerting/metric_threshold/lib/evaluate_alert.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts index 9ceb2c8f2bef77..aeeb705f9b25a6 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts @@ -226,7 +226,7 @@ const getValuesFromAggregations = ( if (aggType === Aggregators.COUNT) { return buckets .map((bucket) => ({ - key: bucket.to_as_string, + key: bucket.from_as_string, value: bucket.doc_count, })) .filter(dropPartialBuckets(dropPartialBucketsOptions)); @@ -237,7 +237,7 @@ const getValuesFromAggregations = ( const values = bucket.aggregatedValue?.values || []; const firstValue = first(values); if (!firstValue) return null; - return { key: bucket.to_as_string, value: firstValue.value }; + return { key: bucket.from_as_string, value: firstValue.value }; }) .filter(dropPartialBuckets(dropPartialBucketsOptions)); } From e493f0bdaa8b171585c7b938902b2517a0197f5e Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Thu, 8 Jul 2021 13:08:02 -0600 Subject: [PATCH 04/15] Fixing tests to be more realistic --- .../metric_threshold_executor.test.ts | 20 +-- .../alerting/metric_threshold/test_mocks.ts | 148 +++++++++++++----- 2 files changed, 117 insertions(+), 51 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 e5cad99dcb4ed4..e0a7c7428ae603 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 @@ -18,6 +18,7 @@ import { InfraSources } from '../../sources'; import { MetricThresholdAlertExecutorOptions } from './register_metric_threshold_alert_type'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; +import moment from 'moment'; interface AlertTestInstance { instance: AlertInstanceMock; @@ -119,14 +120,12 @@ describe('The metric threshold alert type', () => { expect(mostRecentAction(instanceID)).toBe(undefined); }); test('reports expected values to the action context', async () => { - const now = 1577858400000; await execute(Comparator.GT, [0.75]); const { action } = mostRecentAction(instanceID); expect(action.group).toBe('*'); expect(action.reason).toContain('current value is 1'); expect(action.reason).toContain('threshold of 0.75'); expect(action.reason).toContain('test.metric.1'); - expect(action.timestamp).toBe(new Date(now).toISOString()); }); }); @@ -428,7 +427,6 @@ describe('The metric threshold alert type', () => { }, }); test('reports values converted from decimals to percentages to the action context', async () => { - const now = 1577858400000; await execute(); const { action } = mostRecentAction(instanceID); expect(action.group).toBe('*'); @@ -436,7 +434,6 @@ describe('The metric threshold alert type', () => { expect(action.reason).toContain('threshold of 75%'); expect(action.threshold.condition0[0]).toBe('75%'); expect(action.value.condition0).toBe('100%'); - expect(action.timestamp).toBe(new Date(now).toISOString()); }); }); }); @@ -460,7 +457,8 @@ const executor = createMetricThresholdExecutor(mockLibs); const services: AlertServicesMock = alertsMock.createAlertServices(); services.scopedClusterClient.asCurrentUser.search.mockImplementation((params?: any): any => { - if (params.index === 'alternatebeat-*') return mocks.changedSourceIdResponse; + const from = params?.body.query.bool.filter[0]?.range['@timestamp'].gte; + if (params.index === 'alternatebeat-*') return mocks.changedSourceIdResponse(from); const metric = params?.body.query.bool.filter[1]?.exists.field; if (params?.body.aggs.groupings) { if (params?.body.aggs.groupings.composite.after) { @@ -470,25 +468,27 @@ services.scopedClusterClient.asCurrentUser.search.mockImplementation((params?: a } if (metric === 'test.metric.2') { return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.alternateCompositeResponse + mocks.alternateCompositeResponse(from) ); } return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.basicCompositeResponse + mocks.basicCompositeResponse(from) ); } if (metric === 'test.metric.2') { return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.alternateMetricResponse + mocks.alternateMetricResponse(from) ); } else if (metric === 'test.metric.3') { return elasticsearchClientMock.createSuccessTransportRequestPromise( - params?.body.aggs.aggregatedIntervals.aggregations.aggregatedValue_max + params?.body.aggs.aggregatedIntervals.aggregations.aggregatedValueMax ? mocks.emptyRateResponse : mocks.emptyMetricResponse ); } - return elasticsearchClientMock.createSuccessTransportRequestPromise(mocks.basicMetricResponse); + return elasticsearchClientMock.createSuccessTransportRequestPromise( + mocks.basicMetricResponse(from) + ); }); services.savedObjectsClient.get.mockImplementation(async (type: string, sourceId: string) => { if (sourceId === 'alternate') diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts index 47da539afea191..c15f14db25d464 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts @@ -5,63 +5,129 @@ * 2.0. */ -const bucketsA = [ +const bucketsA = (from: number) => [ + { + doc_count: null, + aggregatedValue: { value: null, values: [{ key: 95.0, value: null }] }, + from_as_string: new Date(from).toISOString(), + }, + { + doc_count: 2, + aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] }, + from_as_string: new Date(from + 60000).toISOString(), + }, { doc_count: 2, aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] }, + from_as_string: new Date(from + 120000).toISOString(), + }, + { + doc_count: 2, + aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] }, + from_as_string: new Date(from + 180000).toISOString(), }, { doc_count: 3, aggregatedValue: { value: 1.0, values: [{ key: 95.0, value: 1.0 }] }, - to_as_string: new Date(1577858400000).toISOString(), + from_as_string: new Date(from + 240000).toISOString(), + }, + { + doc_count: 1, + aggregatedValue: { value: 1.0, values: [{ key: 95.0, value: 1.0 }] }, + from_as_string: new Date(from + 300000).toISOString(), }, ]; -const bucketsB = [ +const bucketsB = (from: number) => [ + { + doc_count: 0, + aggregatedValue: { value: null, values: [{ key: 99.0, value: null }] }, + from_as_string: new Date(from).toISOString(), + }, + { + doc_count: 4, + aggregatedValue: { value: 2.5, values: [{ key: 99.0, value: 2.5 }] }, + from_as_string: new Date(from + 60000).toISOString(), + }, + { + doc_count: 4, + aggregatedValue: { value: 2.5, values: [{ key: 99.0, value: 2.5 }] }, + from_as_string: new Date(from + 120000).toISOString(), + }, { doc_count: 4, aggregatedValue: { value: 2.5, values: [{ key: 99.0, value: 2.5 }] }, + from_as_string: new Date(from + 180000).toISOString(), }, { doc_count: 5, aggregatedValue: { value: 3.5, values: [{ key: 99.0, value: 3.5 }] }, + from_as_string: new Date(from + 240000).toISOString(), + }, + { + doc_count: 1, + aggregatedValue: { value: 3, values: [{ key: 99.0, value: 3 }] }, + from_as_string: new Date(from + 300000).toISOString(), }, ]; -const bucketsC = [ +const bucketsC = (from: number) => [ + { + doc_count: 0, + aggregatedValue: { value: null }, + from_as_string: new Date(from).toISOString(), + }, + { + doc_count: 2, + aggregatedValue: { value: 0.5 }, + from_as_string: new Date(from + 60000).toISOString(), + }, + { + doc_count: 2, + aggregatedValue: { value: 0.5 }, + from_as_string: new Date(from + 120000).toISOString(), + }, { doc_count: 2, aggregatedValue: { value: 0.5 }, + from_as_string: new Date(from + 180000).toISOString(), }, { doc_count: 3, - aggregatedValue: { value: 16.0 }, + aggregatedValue: { value: 16 }, + from_as_string: new Date(from + 240000).toISOString(), + }, + { + doc_count: 1, + aggregatedValue: { value: 3 }, + from_as_string: new Date(from + 300000).toISOString(), }, ]; -const previewBucketsA = Array.from(Array(60), (_, i) => bucketsA[i % 2]); // Repeat bucketsA to a total length of 60 -const previewBucketsB = Array.from(Array(60), (_, i) => bucketsB[i % 2]); -const previewBucketsWithNulls = [ +const previewBucketsA = (from: number) => Array.from(Array(60), (_, i) => bucketsA(from)[i % 2]); // Repeat bucketsA to a total length of 60 +const previewBucketsB = (from: number) => Array.from(Array(60), (_, i) => bucketsB(from)[i % 2]); +const previewBucketsWithNulls = (from: number) => [ ...Array.from(Array(10), (_, i) => ({ aggregatedValue: { value: null } })), - ...previewBucketsA.slice(10), + ...previewBucketsA(from).slice(10), ]; -const previewBucketsRepeat = Array.from(Array(60), (_, i) => bucketsA[Math.max(0, (i % 3) - 1)]); +const previewBucketsRepeat = (from: number) => + Array.from(Array(60), (_, i) => bucketsA(from)[Math.max(0, (i % 3) - 1)]); -export const basicMetricResponse = { +export const basicMetricResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: bucketsA, + buckets: bucketsA(from), }, }, -}; +}); -export const alternateMetricResponse = { +export const alternateMetricResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: bucketsB, + buckets: bucketsB(from), }, }, -}; +}); export const emptyMetricResponse = { aggregations: { @@ -71,21 +137,21 @@ export const emptyMetricResponse = { }, }; -export const emptyRateResponse = { +export const emptyRateResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { buckets: [ { doc_count: 2, - // eslint-disable-next-line @typescript-eslint/naming-convention - aggregatedValue_max: { value: null }, + aggregatedValueMax: { value: null }, + from_as_string: new Date(from).toISOString(), }, ], }, }, -}; +}); -export const basicCompositeResponse = { +export const basicCompositeResponse = (from: number) => ({ aggregations: { groupings: { after_key: { groupBy0: 'foo' }, @@ -95,7 +161,7 @@ export const basicCompositeResponse = { groupBy0: 'a', }, aggregatedIntervals: { - buckets: bucketsA, + buckets: bucketsA(from), }, }, { @@ -103,7 +169,7 @@ export const basicCompositeResponse = { groupBy0: 'b', }, aggregatedIntervals: { - buckets: bucketsB, + buckets: bucketsB(from), }, }, ], @@ -114,9 +180,9 @@ export const basicCompositeResponse = { value: 2, }, }, -}; +}); -export const alternateCompositeResponse = { +export const alternateCompositeResponse = (from: number) => ({ aggregations: { groupings: { after_key: { groupBy0: 'foo' }, @@ -126,7 +192,7 @@ export const alternateCompositeResponse = { groupBy0: 'a', }, aggregatedIntervals: { - buckets: bucketsB, + buckets: bucketsB(from), }, }, { @@ -134,7 +200,7 @@ export const alternateCompositeResponse = { groupBy0: 'b', }, aggregatedIntervals: { - buckets: bucketsA, + buckets: bucketsA(from), }, }, ], @@ -145,28 +211,28 @@ export const alternateCompositeResponse = { value: 2, }, }, -}; +}); export const compositeEndResponse = { aggregations: {}, hits: { total: { value: 0 } }, }; -export const changedSourceIdResponse = { +export const changedSourceIdResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: bucketsC, + buckets: bucketsC(from), }, }, -}; +}); -export const basicMetricPreviewResponse = { +export const basicMetricPreviewResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: previewBucketsA, + buckets: previewBucketsA(from), }, }, -}; +}); export const alternateMetricPreviewResponse = { aggregations: { @@ -176,15 +242,15 @@ export const alternateMetricPreviewResponse = { }, }; -export const repeatingMetricPreviewResponse = { +export const repeatingMetricPreviewResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: previewBucketsRepeat, + buckets: previewBucketsRepeat(from), }, }, -}; +}); -export const basicCompositePreviewResponse = { +export const basicCompositePreviewResponse = (from: number) => ({ aggregations: { groupings: { after_key: { groupBy0: 'foo' }, @@ -194,7 +260,7 @@ export const basicCompositePreviewResponse = { groupBy0: 'a', }, aggregatedIntervals: { - buckets: previewBucketsA, + buckets: previewBucketsA(from), }, }, { @@ -202,7 +268,7 @@ export const basicCompositePreviewResponse = { groupBy0: 'b', }, aggregatedIntervals: { - buckets: previewBucketsB, + buckets: previewBucketsB(from), }, }, ], @@ -213,4 +279,4 @@ export const basicCompositePreviewResponse = { value: 2, }, }, -}; +}); From a127bda0149c5bab3219127b4e8c02ab8f3b22d4 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Thu, 8 Jul 2021 15:51:45 -0600 Subject: [PATCH 05/15] fixing types; removing extra imports --- .../metric_threshold/lib/metric_query.test.ts | 12 ++++++--- .../metric_threshold/lib/metric_query.ts | 2 -- .../metric_threshold_executor.test.ts | 1 - .../apis/metrics_ui/metrics_alerting.ts | 27 +++++++++++++++++-- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts index 79aa94f98d2ada..5293d39f6ddc0e 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts @@ -7,6 +7,7 @@ import { MetricExpressionParams } from '../types'; import { getElasticsearchMetricQuery } from './metric_query'; +import moment from 'moment'; describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { const expressionParams = { @@ -18,9 +19,13 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { const timefield = '@timestamp'; const groupBy = 'host.doggoname'; + const timeframe = { + start: moment().subtract(5, 'minutes').valueOf(), + end: moment().valueOf(), + }; describe('when passed no filterQuery', () => { - const searchBody = getElasticsearchMetricQuery(expressionParams, timefield, groupBy); + const searchBody = getElasticsearchMetricQuery(expressionParams, timefield, timeframe, groupBy); test('includes a range filter', () => { expect( searchBody.query.bool.filter.find((filter) => filter.hasOwnProperty('range')) @@ -43,6 +48,7 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { const searchBody = getElasticsearchMetricQuery( expressionParams, timefield, + timeframe, groupBy, filterQuery ); @@ -68,9 +74,9 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { const searchBody = getElasticsearchMetricQuery( expressionParams, timefield, + timerange, undefined, - undefined, - timerange + undefined ); test('by rounding timestamps to the nearest timeUnit', () => { const rangeFilter = searchBody.query.bool.filter.find((filter) => diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts index 9a8d728b01c8a7..a9fefd57d01e6e 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts @@ -8,11 +8,9 @@ import { networkTraffic } from '../../../../../common/inventory_models/shared/metrics/snapshot/network_traffic'; import { MetricExpressionParams, Aggregators } from '../types'; import { getIntervalInSeconds } from '../../../../utils/get_interval_in_seconds'; -import { roundTimestamp } from '../../../../utils/round_timestamp'; import { createPercentileAggregation } from './create_percentile_aggregation'; import { calculateDateHistogramOffset } from '../../../metrics/lib/calculate_date_histogram_offset'; -const MINIMUM_BUCKETS = 5; const COMPOSITE_RESULTS_PER_PAGE = 100; const getParsedFilterQuery: (filterQuery: string | undefined) => Record | null = ( 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 e0a7c7428ae603..0dda299704d528 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 @@ -18,7 +18,6 @@ import { InfraSources } from '../../sources'; import { MetricThresholdAlertExecutorOptions } from './register_metric_threshold_alert_type'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; -import moment from 'moment'; interface AlertTestInstance { instance: AlertInstanceMock; diff --git a/x-pack/test/api_integration/apis/metrics_ui/metrics_alerting.ts b/x-pack/test/api_integration/apis/metrics_ui/metrics_alerting.ts index 7cf1019a0a325b..6f9c0f5fc708f0 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/metrics_alerting.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/metrics_alerting.ts @@ -6,11 +6,11 @@ */ import expect from '@kbn/expect'; +import moment from 'moment'; import { getElasticsearchMetricQuery } from '../../../../plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query'; import { MetricExpressionParams } from '../../../../plugins/infra/server/lib/alerting/metric_threshold/types'; import { FtrProviderContext } from '../../ftr_provider_context'; - export default function ({ getService }: FtrProviderContext) { const client = getService('legacyEs'); const index = 'test-index'; @@ -33,7 +33,15 @@ export default function ({ getService }: FtrProviderContext) { describe('querying the entire infrastructure', () => { for (const aggType of aggs) { it(`should work with the ${aggType} aggregator`, async () => { - const searchBody = getElasticsearchMetricQuery(getSearchParams(aggType), '@timestamp'); + const timeframe = { + start: moment().subtract(25, 'minutes').valueOf(), + end: moment().valueOf(), + }; + const searchBody = getElasticsearchMetricQuery( + getSearchParams(aggType), + '@timestamp', + timeframe + ); const result = await client.search({ index, body: searchBody, @@ -44,9 +52,14 @@ export default function ({ getService }: FtrProviderContext) { }); } it('should work with a filterQuery', async () => { + const timeframe = { + start: moment().subtract(25, 'minutes').valueOf(), + end: moment().valueOf(), + }; const searchBody = getElasticsearchMetricQuery( getSearchParams('avg'), '@timestamp', + timeframe, undefined, '{"bool":{"should":[{"match_phrase":{"agent.hostname":"foo"}}],"minimum_should_match":1}}' ); @@ -62,9 +75,14 @@ export default function ({ getService }: FtrProviderContext) { describe('querying with a groupBy parameter', () => { for (const aggType of aggs) { it(`should work with the ${aggType} aggregator`, async () => { + const timeframe = { + start: moment().subtract(25, 'minutes').valueOf(), + end: moment().valueOf(), + }; const searchBody = getElasticsearchMetricQuery( getSearchParams(aggType), '@timestamp', + timeframe, 'agent.id' ); const result = await client.search({ @@ -77,9 +95,14 @@ export default function ({ getService }: FtrProviderContext) { }); } it('should work with a filterQuery', async () => { + const timeframe = { + start: moment().subtract(25, 'minutes').valueOf(), + end: moment().valueOf(), + }; const searchBody = getElasticsearchMetricQuery( getSearchParams('avg'), '@timestamp', + timeframe, 'agent.id', '{"bool":{"should":[{"match_phrase":{"agent.hostname":"foo"}}],"minimum_should_match":1}}' ); From 4d40c1e51aac14e36a41f87d171b8e857fbc4530 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 11:56:42 -0600 Subject: [PATCH 06/15] Fixing new mock data to work with previews --- .../preview_metric_threshold_alert.test.ts | 9 +-- .../alerting/metric_threshold/test_mocks.ts | 64 ++++++++++++++++--- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.test.ts index 49cb8d70f6020a..e03b475191dff1 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.test.ts @@ -167,6 +167,7 @@ describe('Previewing the metric threshold alert type', () => { const services: AlertServicesMock = alertsMock.createAlertServices(); services.scopedClusterClient.asCurrentUser.search.mockImplementation((params?: any): any => { + const from = params?.body.query.bool.filter[0]?.range['@timestamp'].gte; const metric = params?.body.query.bool.filter[1]?.exists.field; if (params?.body.aggs.groupings) { if (params?.body.aggs.groupings.composite.after) { @@ -175,21 +176,21 @@ services.scopedClusterClient.asCurrentUser.search.mockImplementation((params?: a ); } return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.basicCompositePreviewResponse + mocks.basicCompositePreviewResponse(from) ); } if (metric === 'test.metric.2') { return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.alternateMetricPreviewResponse + mocks.alternateMetricPreviewResponse(from) ); } if (metric === 'test.metric.3') { return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.repeatingMetricPreviewResponse + mocks.repeatingMetricPreviewResponse(from) ); } return elasticsearchClientMock.createSuccessTransportRequestPromise( - mocks.basicMetricPreviewResponse + mocks.basicMetricPreviewResponse(from) ); }); diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts index c15f14db25d464..5af14b3fbd17d6 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - +import { range } from 'lodash'; const bucketsA = (from: number) => [ { doc_count: null, @@ -104,14 +104,60 @@ const bucketsC = (from: number) => [ }, ]; -const previewBucketsA = (from: number) => Array.from(Array(60), (_, i) => bucketsA(from)[i % 2]); // Repeat bucketsA to a total length of 60 -const previewBucketsB = (from: number) => Array.from(Array(60), (_, i) => bucketsB(from)[i % 2]); +const previewBucketsA = (from: number) => + range(from, from + 3600000, 60000).map((timestamp, i) => { + return { + doc_count: i % 2 ? 3 : 2, + aggregatedValue: { value: i % 2 ? 16 : 0.5 }, + from_as_string: new Date(timestamp).toISOString(), + }; + }); + +const previewBucketsB = (from: number) => + range(from, from + 3600000, 60000).map((timestamp, i) => { + const value = i % 2 ? 3.5 : 2.5; + return { + doc_count: i % 2 ? 3 : 2, + aggregatedValue: { value, values: [{ key: 99.0, value }] }, + from_as_string: new Date(timestamp).toISOString(), + }; + }); + const previewBucketsWithNulls = (from: number) => [ - ...Array.from(Array(10), (_, i) => ({ aggregatedValue: { value: null } })), - ...previewBucketsA(from).slice(10), + // 25 Fired + ...range(from, from + 1500000, 60000).map((timestamp) => { + return { + doc_count: 2, + aggregatedValue: { value: 1, values: [{ key: 95.0, value: 1 }] }, + from_as_string: new Date(timestamp).toISOString(), + }; + }), + // 25 OK + ...range(from + 2100000, from + 2940000, 60000).map((timestamp) => { + return { + doc_count: 2, + aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] }, + from_as_string: new Date(timestamp).toISOString(), + }; + }), + // 10 No Data + ...range(from + 3000000, from + 3600000, 60000).map((timestamp) => { + return { + doc_count: 0, + aggregatedValue: { value: null, values: [{ key: 95.0, value: null }] }, + from_as_string: new Date(timestamp).toISOString(), + }; + }), ]; + const previewBucketsRepeat = (from: number) => - Array.from(Array(60), (_, i) => bucketsA(from)[Math.max(0, (i % 3) - 1)]); + range(from, from + 3600000, 60000).map((timestamp, i) => { + return { + doc_count: i % 3 ? 3 : 2, + aggregatedValue: { value: i % 3 ? 0.5 : 16 }, + from_as_string: new Date(timestamp).toISOString(), + }; + }); export const basicMetricResponse = (from: number) => ({ aggregations: { @@ -234,13 +280,13 @@ export const basicMetricPreviewResponse = (from: number) => ({ }, }); -export const alternateMetricPreviewResponse = { +export const alternateMetricPreviewResponse = (from: number) => ({ aggregations: { aggregatedIntervals: { - buckets: previewBucketsWithNulls, + buckets: previewBucketsWithNulls(from), }, }, -}; +}); export const repeatingMetricPreviewResponse = (from: number) => ({ aggregations: { From cfa123629ccb592b6b40cbe6b754e618586acd9e Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 15:12:05 -0600 Subject: [PATCH 07/15] Removing value checks since they don't really provide much value --- .../apis/metrics_ui/metrics_explorer.ts | 27 +++---------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/x-pack/test/api_integration/apis/metrics_ui/metrics_explorer.ts b/x-pack/test/api_integration/apis/metrics_ui/metrics_explorer.ts index 8d5c677f56ea2f..1eaae741f7e05b 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/metrics_explorer.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/metrics_explorer.ts @@ -57,12 +57,7 @@ export default function ({ getService }: FtrProviderContext) { { name: 'metric_0', type: 'number' }, { name: 'metric_1', type: 'number' }, ]); - expect(firstSeries.rows).to.have.length(9); - expect(firstSeries.rows![1]).to.eql({ - metric_0: 0.005333333333333333, - metric_1: 131, - timestamp: 1547571300000, - }); + expect(firstSeries.rows).to.have.length(8); }); it('should apply filterQuery to data', async () => { @@ -96,11 +91,7 @@ export default function ({ getService }: FtrProviderContext) { { name: 'timestamp', type: 'date' }, { name: 'metric_0', type: 'number' }, ]); - expect(firstSeries.rows).to.have.length(9); - expect(firstSeries.rows![1]).to.eql({ - metric_0: 0.024, - timestamp: 1547571300000, - }); + expect(firstSeries.rows).to.have.length(8); }); it('should work for empty metrics', async () => { @@ -159,12 +150,7 @@ export default function ({ getService }: FtrProviderContext) { { name: 'metric_0', type: 'number' }, { name: 'groupBy', type: 'string' }, ]); - expect(firstSeries.rows).to.have.length(9); - expect(firstSeries.rows![1]).to.eql({ - groupBy: 'system.diskio', - metric_0: 24, - timestamp: 1547571300000, - }); + expect(firstSeries.rows).to.have.length(8); expect(body.pageInfo).to.eql({ afterKey: { groupBy0: 'system.fsstat' }, total: 12, @@ -204,12 +190,7 @@ export default function ({ getService }: FtrProviderContext) { { name: 'metric_0', type: 'number' }, { name: 'groupBy', type: 'string' }, ]); - expect(firstSeries.rows).to.have.length(9); - expect(firstSeries.rows![1]).to.eql({ - groupBy: 'demo-stack-mysql-01 / eth0', - metric_0: 53577.683333333334, - timestamp: 1547571300000, - }); + expect(firstSeries.rows).to.have.length(8); expect(body.pageInfo).to.eql({ afterKey: { groupBy0: 'demo-stack-mysql-01', groupBy1: 'eth2' }, total: 4, From 1f04f75fa29cfd60e74f49369d199cef3f38f910 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 15:12:47 -0600 Subject: [PATCH 08/15] Removing test for refactored functinality --- .../lib/alerting/metric_threshold/lib/metric_query.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts index 5293d39f6ddc0e..dda5b54aeace6f 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts @@ -78,12 +78,5 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { undefined, undefined ); - test('by rounding timestamps to the nearest timeUnit', () => { - const rangeFilter = searchBody.query.bool.filter.find((filter) => - filter.hasOwnProperty('range') - )?.range[timefield]; - expect(rangeFilter?.lte).toBe(1594246020000); - expect(rangeFilter?.gte).toBe(1594245720000); - }); }); }); From bba878285b6d4612051d29f962456f59794c6899 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 15:13:30 -0600 Subject: [PATCH 09/15] Change value to match millisecond resolution --- .../metrics/lib/__snapshots__/create_aggregations.test.ts.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/create_aggregations.test.ts.snap b/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/create_aggregations.test.ts.snap index 2cbbc623aed38f..c7acfb147aa7e1 100644 --- a/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/create_aggregations.test.ts.snap +++ b/x-pack/plugins/infra/server/lib/metrics/lib/__snapshots__/create_aggregations.test.ts.snap @@ -17,7 +17,7 @@ Object { }, "field": "@timestamp", "fixed_interval": "1m", - "offset": "-60s", + "offset": "-60000ms", }, }, } From 5d0da2eeb4117c2c7bfa23b40880812b570026a9 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 15:14:07 -0600 Subject: [PATCH 10/15] Fixing values for new partial bucket scheme --- x-pack/test/api_integration/apis/metrics_ui/snapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts index 8fd8a5a6d3ec8e..d40f487984b27f 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts @@ -187,7 +187,7 @@ export default function ({ getService }: FtrProviderContext) { name: 'cpu', value: 0.0032, max: 0.0038333333333333336, - avg: 0.002794444444444445, + avg: 0.003341666666666667, }, ]); } From c89fdf6a9c74b022daf15263e35bbce072f27d91 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 16:40:27 -0600 Subject: [PATCH 11/15] removing unused var --- .../alerting/metric_threshold/lib/metric_query.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts index dda5b54aeace6f..4ccda8094811e5 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts @@ -71,12 +71,6 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { end, start: end - 5 * 60 * 1000, }; - const searchBody = getElasticsearchMetricQuery( - expressionParams, - timefield, - timerange, - undefined, - undefined - ); + getElasticsearchMetricQuery(expressionParams, timefield, timerange, undefined, undefined); }); }); From 509403c7557ba0458d48709acf3b6fc0cc6376ab Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 9 Jul 2021 16:42:35 -0600 Subject: [PATCH 12/15] Fixing lookback since drops more than last buckets --- x-pack/test/api_integration/apis/metrics_ui/snapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts index d40f487984b27f..b3c33020456d45 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts @@ -222,7 +222,7 @@ export default function ({ getService }: FtrProviderContext) { expect(first(firstNode.path)).to.have.property('label', 'demo-stack-mysql-01'); expect(firstNode).to.have.property('metrics'); expect(firstNode.metrics[0]).to.have.property('timeseries'); - expect(firstNode.metrics[0].timeseries?.rows.length).to.equal(58); + expect(firstNode.metrics[0].timeseries?.rows.length).to.equal(56); const rows = firstNode.metrics[0].timeseries?.rows; const rowInterval = (rows?.[1]?.timestamp || 0) - (rows?.[0]?.timestamp || 0); expect(rowInterval).to.equal(10000); From 9ea49e69187270b43f4afd2fe5a1a1f3f6ab873d Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Mon, 12 Jul 2021 12:43:53 -0600 Subject: [PATCH 13/15] Changing results count --- x-pack/test/api_integration/apis/metrics_ui/snapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts index b3c33020456d45..48a635fde60550 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts @@ -257,7 +257,7 @@ export default function ({ getService }: FtrProviderContext) { expect(first(firstNode.path)).to.have.property('label', 'demo-stack-mysql-01'); expect(firstNode).to.have.property('metrics'); expect(firstNode.metrics[0]).to.have.property('timeseries'); - expect(firstNode.metrics[0].timeseries?.rows.length).to.equal(7); + expect(firstNode.metrics[0].timeseries?.rows.length).to.equal(5); } }); }); From 2e9fee7fa42b0672b33fdf1968c80952f03db0d9 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Mon, 12 Jul 2021 17:14:43 -0600 Subject: [PATCH 14/15] fixing more tests --- x-pack/test/api_integration/apis/metrics_ui/snapshot.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts index 48a635fde60550..4181344f064b9b 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/snapshot.ts @@ -298,7 +298,7 @@ export default function ({ getService }: FtrProviderContext) { name: 'custom_0', value: 0.0016, max: 0.0018333333333333333, - avg: 0.0013666666666666669, + avg: 0.00165, }, ]); } @@ -389,7 +389,7 @@ export default function ({ getService }: FtrProviderContext) { name: 'cpu', value: 0.0032, max: 0.0038333333333333336, - avg: 0.002794444444444445, + avg: 0.003341666666666667, }, ]); const secondNode = nodes[1] as any; @@ -403,7 +403,7 @@ export default function ({ getService }: FtrProviderContext) { name: 'cpu', value: 0.0032, max: 0.0038333333333333336, - avg: 0.002794444444444445, + avg: 0.003341666666666667, }, ]); } From 14bc50cbd845090d9bbbb39f6b95bb485fabfe5f Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 16 Jul 2021 09:27:36 -0600 Subject: [PATCH 15/15] Removing empty describe --- .../alerting/metric_threshold/lib/metric_query.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts index 4ccda8094811e5..2ba8365d6b4a91 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts @@ -64,13 +64,4 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => { ); }); }); - - describe('handles time', () => { - const end = new Date('2020-07-08T22:07:27.235Z').valueOf(); - const timerange = { - end, - start: end - 5 * 60 * 1000, - }; - getElasticsearchMetricQuery(expressionParams, timefield, timerange, undefined, undefined); - }); });