Skip to content

Commit

Permalink
[Metrics UI] Fix previewing of No Data results (elastic#73753)
Browse files Browse the repository at this point in the history
# Conflicts:
#	x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts
  • Loading branch information
Zacqary committed Jul 31, 2020
1 parent 36ad1cb commit b371b1b
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export const AlertPreview: React.FC<Props> = (props) => {
plural: previewResult.resultTotals.noData !== 1 ? 's' : '',
}}
/>
) : null}
) : null}{' '}
{previewResult.resultTotals.error ? (
<FormattedMessage
id="xpack.infra.metrics.alertFlyout.alertPreviewErrorResult"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ export const Expressions: React.FC<Props> = (props) => {
validate={validateMetricThreshold}
fetch={alertsContext.http.fetch}
groupByDisplayName={alertParams.nodeType}
showNoDataResults={alertParams.alertOnNoData}
/>
<EuiSpacer size={'m'} />
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
});
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const previewMetricThresholdAlert: (
params: PreviewMetricThresholdAlertParams,
iterations?: number,
precalculatedNumberOfGroups?: number
) => Promise<Array<number | null>> = async (
) => Promise<number[][]> = async (
{
callCluster,
params,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 14 additions & 9 deletions x-pack/plugins/infra/server/routes/alerting/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,20 @@ 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,
noData: 0,
error: 0,
}
);

return response.ok({
body: alertPreviewSuccessResponsePayloadRT.encode({
numberOfGroups,
Expand All @@ -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,
Expand Down

0 comments on commit b371b1b

Please sign in to comment.