From 047743ee6e9140bf65100c01cdb9e877076f5ea0 Mon Sep 17 00:00:00 2001 From: Zacqary Adam Xeper Date: Thu, 30 Jul 2020 15:52:32 -0500 Subject: [PATCH] [Metrics UI] Fix previewing of No Data results (#73753) # Conflicts: # x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts --- .../common/components/alert_preview.tsx | 2 +- .../inventory/components/expression.tsx | 1 + .../evaluate_condition.ts | 8 ++-- .../inventory_metric_threshold_executor.ts | 9 ++-- ...review_inventory_metric_threshold_alert.ts | 41 ++++++++++--------- .../metric_threshold/lib/evaluate_alert.ts | 6 ++- .../metric_threshold_executor.ts | 2 +- .../preview_metric_threshold_alert.ts | 30 +++++++------- .../infra/server/routes/alerting/preview.ts | 23 +++++++---- 9 files changed, 68 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/infra/public/alerting/common/components/alert_preview.tsx b/x-pack/plugins/infra/public/alerting/common/components/alert_preview.tsx index f3136ca155c78..d5b50fce38718 100644 --- a/x-pack/plugins/infra/public/alerting/common/components/alert_preview.tsx +++ b/x-pack/plugins/infra/public/alerting/common/components/alert_preview.tsx @@ -194,7 +194,7 @@ export const AlertPreview: React.FC = (props) => { plural: previewResult.resultTotals.noData !== 1 ? 's' : '', }} /> - ) : null} + ) : null}{' '} {previewResult.resultTotals.error ? ( = (props) => { validate={validateMetricThreshold} fetch={alertsContext.http.fetch} groupByDisplayName={alertParams.nodeType} + showNoDataResults={alertParams.alertOnNoData} /> diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/evaluate_condition.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/evaluate_condition.ts index c991e482a62e5..4d0c13e366f21 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/evaluate_condition.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/evaluate_condition.ts @@ -23,9 +23,9 @@ import { InfraSourceConfiguration } from '../../sources'; import { UNGROUPED_FACTORY_KEY } from '../common/utils'; type ConditionResult = InventoryMetricConditions & { - shouldFire: boolean | boolean[]; + shouldFire: boolean[]; currentValue: number; - isNoData: boolean; + isNoData: boolean[]; isError: boolean; }; @@ -71,8 +71,8 @@ export const evaluateCondition = async ( value !== null && (Array.isArray(value) ? value.map((v) => comparisonFunction(Number(v), threshold)) - : comparisonFunction(value as number, threshold)), - isNoData: value === null, + : [comparisonFunction(value as number, threshold)]), + isNoData: Array.isArray(value) ? value.map((v) => v === null) : [value === null], isError: value === undefined, currentValue: getCurrentValue(value), }; diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts index 60eee49a5010d..7b816f2f225b5 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { first, get } from 'lodash'; +import { first, get, last } from 'lodash'; import { i18n } from '@kbn/i18n'; import moment from 'moment'; import { toMetricOpt } from '../../../../common/snapshot_metric_i18n'; @@ -56,11 +56,14 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = for (const item of inventoryItems) { const alertInstance = services.alertInstanceFactory(`${item}`); // AND logic; all criteria must be across the threshold - const shouldAlertFire = results.every((result) => result[item].shouldFire); + const shouldAlertFire = results.every((result) => + // Grab the result of the most recent bucket + last(result[item].shouldFire) + ); // AND logic; because we need to evaluate all criteria, if one of them reports no data then the // whole alert is in a No Data/Error state - const isNoData = results.some((result) => result[item].isNoData); + const isNoData = results.some((result) => last(result[item].isNoData)); const isError = results.some((result) => result[item].isError); const nextState = isError diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/preview_inventory_metric_threshold_alert.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/preview_inventory_metric_threshold_alert.ts index 5c654e2f47e78..562f344dbd060 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/preview_inventory_metric_threshold_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/preview_inventory_metric_threshold_alert.ts @@ -59,28 +59,29 @@ export const previewInventoryMetricThresholdAlert = async ({ const inventoryItems = Object.keys(first(results) as any); const previewResults = inventoryItems.map((item) => { - const isNoData = results.some((result) => result[item].isNoData); - if (isNoData) { - return null; - } - const isError = results.some((result) => result[item].isError); - if (isError) { - return undefined; - } - const numberOfResultBuckets = lookbackSize; const numberOfExecutionBuckets = Math.floor(numberOfResultBuckets / alertResultsPerExecution); - return [...Array(numberOfExecutionBuckets)].reduce( - (totalFired, _, i) => - totalFired + - (results.every((result) => { - const shouldFire = result[item].shouldFire as boolean[]; - return shouldFire[Math.floor(i * alertResultsPerExecution)]; - }) - ? 1 - : 0), - 0 - ); + let numberOfTimesFired = 0; + let numberOfNoDataResults = 0; + let numberOfErrors = 0; + for (let i = 0; i < numberOfExecutionBuckets; i++) { + const mappedBucketIndex = Math.floor(i * alertResultsPerExecution); + const allConditionsFiredInMappedBucket = results.every((result) => { + const shouldFire = result[item].shouldFire as boolean[]; + return shouldFire[mappedBucketIndex]; + }); + const someConditionsNoDataInMappedBucket = results.some((result) => { + const hasNoData = result[item].isNoData as boolean[]; + return hasNoData[mappedBucketIndex]; + }); + const someConditionsErrorInMappedBucket = results.some((result) => { + return result[item].isError; + }); + if (allConditionsFiredInMappedBucket) numberOfTimesFired++; + if (someConditionsNoDataInMappedBucket) numberOfNoDataResults++; + if (someConditionsErrorInMappedBucket) numberOfErrors++; + } + return [numberOfTimesFired, numberOfNoDataResults, numberOfErrors]; }); return previewResults; 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 d862f70c47cae..4025cd433675a 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 @@ -69,8 +69,10 @@ export const evaluateAlert = ( shouldFire: Array.isArray(points) ? points.map((point) => comparisonFunction(point.value, threshold)) : [false], - isNoData: points === null, - isError: isNaN(points), + isNoData: Array.isArray(points) + ? points.map((point) => point?.value === null || point === null) + : [points === null], + isError: isNaN(Array.isArray(points) ? last(points)?.value : points), }; }); }) diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index b4754a8624fd5..b2a8f0281b9e2 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -45,7 +45,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => ); // AND logic; because we need to evaluate all criteria, if one of them reports no data then the // whole alert is in a No Data/Error state - const isNoData = alertResults.some((result) => result[group].isNoData); + const isNoData = alertResults.some((result) => last(result[group].isNoData)); const isError = alertResults.some((result) => result[group].isError); const nextState = isError diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.ts index 0ecfa27d0f0a8..5aca7f0890940 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/preview_metric_threshold_alert.ts @@ -36,7 +36,7 @@ export const previewMetricThresholdAlert: ( params: PreviewMetricThresholdAlertParams, iterations?: number, precalculatedNumberOfGroups?: number -) => Promise> = async ( +) => Promise = async ( { callCluster, params, @@ -77,15 +77,6 @@ export const previewMetricThresholdAlert: ( const alertResultsPerExecution = alertIntervalInSeconds / bucketIntervalInSeconds; const previewResults = await Promise.all( groups.map(async (group) => { - const isNoData = alertResults.some((alertResult) => alertResult[group].isNoData); - if (isNoData) { - return null; - } - const isError = alertResults.some((alertResult) => alertResult[group].isError); - if (isError) { - return NaN; - } - // Interpolate the buckets returned by evaluateAlert and return a count of how many of these // buckets would have fired the alert. If the alert interval and bucket interval are the same, // this will be a 1:1 evaluation of the alert results. If these are different, the interpolation @@ -95,14 +86,25 @@ export const previewMetricThresholdAlert: ( numberOfResultBuckets / alertResultsPerExecution ); let numberOfTimesFired = 0; + let numberOfNoDataResults = 0; + let numberOfErrors = 0; for (let i = 0; i < numberOfExecutionBuckets; i++) { const mappedBucketIndex = Math.floor(i * alertResultsPerExecution); const allConditionsFiredInMappedBucket = alertResults.every( (alertResult) => alertResult[group].shouldFire[mappedBucketIndex] ); + const someConditionsNoDataInMappedBucket = alertResults.some((alertResult) => { + const hasNoData = alertResult[group].isNoData as boolean[]; + return hasNoData[mappedBucketIndex]; + }); + const someConditionsErrorInMappedBucket = alertResults.some((alertResult) => { + return alertResult[group].isError; + }); if (allConditionsFiredInMappedBucket) numberOfTimesFired++; + if (someConditionsNoDataInMappedBucket) numberOfNoDataResults++; + if (someConditionsErrorInMappedBucket) numberOfErrors++; } - return numberOfTimesFired; + return [numberOfTimesFired, numberOfNoDataResults, numberOfErrors]; }) ); return previewResults; @@ -152,9 +154,9 @@ export const previewMetricThresholdAlert: ( // so filter these results out entirely and only regard the resultA portion .filter((value) => typeof value !== 'undefined') .reduce((a, b) => { - if (typeof a !== 'number') return a; - if (typeof b !== 'number') return b; - return a + b; + if (!a) return b; + if (!b) return a; + return [a[0] + b[0], a[1] + b[1], a[2] + b[2]]; }) ); return zippedResult as any; diff --git a/x-pack/plugins/infra/server/routes/alerting/preview.ts b/x-pack/plugins/infra/server/routes/alerting/preview.ts index 8a3e9e4d0bedc..5594323d706de 100644 --- a/x-pack/plugins/infra/server/routes/alerting/preview.ts +++ b/x-pack/plugins/infra/server/routes/alerting/preview.ts @@ -55,10 +55,13 @@ export const initAlertPreviewRoute = ({ framework, sources }: InfraBackendLibs) const numberOfGroups = previewResult.length; const resultTotals = previewResult.reduce( - (totals, groupResult) => { - if (groupResult === null) return { ...totals, noData: totals.noData + 1 }; - if (isNaN(groupResult)) return { ...totals, error: totals.error + 1 }; - return { ...totals, fired: totals.fired + groupResult }; + (totals, [firedResult, noDataResult, errorResult]) => { + return { + ...totals, + fired: totals.fired + firedResult, + noData: totals.noData + noDataResult, + error: totals.error + errorResult, + }; }, { fired: 0, @@ -66,7 +69,6 @@ export const initAlertPreviewRoute = ({ framework, sources }: InfraBackendLibs) error: 0, } ); - return response.ok({ body: alertPreviewSuccessResponsePayloadRT.encode({ numberOfGroups, @@ -86,10 +88,13 @@ export const initAlertPreviewRoute = ({ framework, sources }: InfraBackendLibs) const numberOfGroups = previewResult.length; const resultTotals = previewResult.reduce( - (totals, groupResult) => { - if (groupResult === null) return { ...totals, noData: totals.noData + 1 }; - if (isNaN(groupResult)) return { ...totals, error: totals.error + 1 }; - return { ...totals, fired: totals.fired + groupResult }; + (totals, [firedResult, noDataResult, errorResult]) => { + return { + ...totals, + fired: totals.fired + firedResult, + noData: totals.noData + noDataResult, + error: totals.error + errorResult, + }; }, { fired: 0,