From 118d83efeb83288876650f6c7a2ac1e176ff44b9 Mon Sep 17 00:00:00 2001 From: Ester Marti Date: Wed, 3 Mar 2021 19:11:06 +0100 Subject: [PATCH] Save inventory alert thresholds in the same unit as ES --- .../threshold_annotations.tsx | 16 ++++---- .../inventory/components/expression.tsx | 40 +++++++++++++++++-- .../inventory/components/expression_chart.tsx | 27 ++----------- .../evaluate_condition.ts | 26 ++++-------- 4 files changed, 55 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/infra/public/alerting/common/criterion_preview_chart/threshold_annotations.tsx b/x-pack/plugins/infra/public/alerting/common/criterion_preview_chart/threshold_annotations.tsx index 59b3e583b6f8..5098eb2c23f5 100644 --- a/x-pack/plugins/infra/public/alerting/common/criterion_preview_chart/threshold_annotations.tsx +++ b/x-pack/plugins/infra/public/alerting/common/criterion_preview_chart/threshold_annotations.tsx @@ -20,7 +20,6 @@ interface ThresholdAnnotationsProps { comparator: Comparator; color: Color; id: string; - formatter?: Function; firstTimestamp: number; lastTimestamp: number; domain: { min: number; max: number }; @@ -34,7 +33,6 @@ export const ThresholdAnnotations = ({ comparator, color, id, - formatter = (value: any) => value, firstTimestamp, lastTimestamp, domain, @@ -49,7 +47,7 @@ export const ThresholdAnnotations = ({ domainType={AnnotationDomainTypes.YDomain} data-test-subj="threshold-line" dataValues={sortedThresholds.map((t) => ({ - dataValue: formatter(t), + dataValue: t, }))} style={{ line: { @@ -73,8 +71,8 @@ export const ThresholdAnnotations = ({ coordinates: { x0: firstTimestamp, x1: lastTimestamp, - y0: formatter(first(threshold)), - y1: formatter(last(threshold)), + y0: first(threshold), + y1: last(threshold), }, }, ]} @@ -96,7 +94,7 @@ export const ThresholdAnnotations = ({ x0: firstTimestamp, x1: lastTimestamp, y0: domain.min, - y1: formatter(first(threshold)), + y1: first(threshold), }, }, ]} @@ -113,7 +111,7 @@ export const ThresholdAnnotations = ({ coordinates: { x0: firstTimestamp, x1: lastTimestamp, - y0: formatter(last(threshold)), + y0: last(threshold), y1: domain.max, }, }, @@ -135,7 +133,7 @@ export const ThresholdAnnotations = ({ x0: firstTimestamp, x1: lastTimestamp, y0: domain.min, - y1: formatter(first(threshold)), + y1: first(threshold), }, }, ]} @@ -154,7 +152,7 @@ export const ThresholdAnnotations = ({ coordinates: { x0: firstTimestamp, x1: lastTimestamp, - y0: formatter(first(threshold)), + y0: first(threshold), y1: domain.max, }, }, diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx index e79343de1ec5..925cb74dc5bf 100644 --- a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx @@ -72,6 +72,10 @@ import { validateMetricThreshold } from './validation'; import { useKibanaContextForPlugin } from '../../../hooks/use_kibana'; import { ExpressionChart } from './expression_chart'; +import { pctToDecimal, decimalToPct } from '../../../../common/utils/corrected_percent_convert'; +import { METRIC_FORMATTERS } from '../../../../common/formatters/snapshot_metric_formats'; +import { InfraFormatterType } from '../../../lib/lib'; + const FILTER_TYPING_DEBOUNCE_MS = 500; export interface AlertContextMeta { @@ -477,6 +481,11 @@ export const ExpressionRow: React.FC = (props) => { warningComparator, } = expression; + const isMetricPct = useMemo(() => { + const metricFormatter = METRIC_FORMATTERS[expression.metric]; + return Boolean(metricFormatter?.formatter === InfraFormatterType.percent); + }, [expression.metric]); + const [displayWarningThreshold, setDisplayWarningThreshold] = useState( Boolean(warningThreshold?.length) ); @@ -513,13 +522,20 @@ export const ExpressionRow: React.FC = (props) => { [expressionId, expression, setAlertParams] ); + const convertThreshold = useCallback( + (enteredThreshold) => + isMetricPct ? enteredThreshold.map((v: number) => pctToDecimal(v)) : enteredThreshold, + [isMetricPct] + ); + const updateThreshold = useCallback( - (t) => { + (enteredThreshold) => { + const t = convertThreshold(enteredThreshold); if (t.join() !== expression.threshold.join()) { setAlertParams(expressionId, { ...expression, threshold: t }); } }, - [expressionId, expression, setAlertParams] + [expressionId, expression, convertThreshold, setAlertParams] ); const updateWarningThreshold = useCallback( @@ -560,6 +576,7 @@ export const ExpressionRow: React.FC = (props) => { updateThreshold={updateThreshold} errors={(errors.critical as IErrorObject) ?? {}} metric={metric} + isMetricPct={isMetricPct} /> ); @@ -571,6 +588,7 @@ export const ExpressionRow: React.FC = (props) => { updateThreshold={updateWarningThreshold} errors={(errors.warning as IErrorObject) ?? {}} metric={metric} + isMetricPct={isMetricPct} /> ); @@ -718,16 +736,30 @@ const ThresholdElement: React.FC<{ updateComparator: (c?: string) => void; updateThreshold: (t?: number[]) => void; threshold: InventoryMetricConditions['threshold']; + isMetricPct: boolean; comparator: InventoryMetricConditions['comparator']; errors: IErrorObject; metric?: SnapshotMetricType; -}> = ({ updateComparator, updateThreshold, threshold, metric, comparator, errors }) => { +}> = ({ + updateComparator, + updateThreshold, + threshold, + isMetricPct, + metric, + comparator, + errors, +}) => { + const displayedThreshold = useMemo(() => { + if (isMetricPct) return threshold.map((v) => decimalToPct(v)); + return threshold; + }, [threshold, isMetricPct]); + return ( <> = ({ return ; } - const thresholdFormatter = (value: number | undefined) => { - const metricFormatter = METRIC_FORMATTERS[expression.metric]; - - if (!value || !metricFormatter) { - return value; - } - - switch (metricFormatter.formatter) { - case InfraFormatterType.percent: - return value / 100; - case InfraFormatterType.bits: - return value / 1000; - default: - return value; - } - }; - const series = { ...firstSeries, id: nodes[0]?.name, rows: firstSeries.rows.map((row) => { const newRow: MetricsExplorerRow = { ...row }; thresholds.forEach((thresholdValue, index) => { - newRow[getMetricId(metric, `threshold_${index}`)] = thresholdFormatter(thresholdValue); + newRow[getMetricId(metric, `threshold_${index}`)] = thresholdValue; }); return newRow; }), @@ -160,11 +143,11 @@ export const ExpressionChart: React.FC = ({ const lastTimestamp = last(firstSeries.rows)!.timestamp; const dataDomain = calculateDomain(series, [metric], false); const domain = { - max: Math.max(dataDomain.max, thresholdFormatter(last(thresholds)) || dataDomain.max) * 1.1, // add 10% headroom. - min: Math.min(dataDomain.min, thresholdFormatter(first(thresholds)) || dataDomain.min) * 0.9, // add 10% floor + max: Math.max(dataDomain.max, last(thresholds) || dataDomain.max) * 1.1, // add 10% headroom. + min: Math.min(dataDomain.min, first(thresholds) || dataDomain.min) * 0.9, // add 10% floor }; - if (domain.min === thresholdFormatter(first(expression.threshold))) { + if (domain.min === first(expression.threshold)) { domain.min = domain.min * 0.9; } @@ -188,7 +171,6 @@ export const ExpressionChart: React.FC = ({ sortedThresholds={criticalThresholds} color={Color.color1} id="critical" - formatter={thresholdFormatter} firstTimestamp={firstTimestamp} lastTimestamp={lastTimestamp} domain={domain} @@ -200,7 +182,6 @@ export const ExpressionChart: React.FC = ({ sortedThresholds={warningThresholds} color={Color.color5} id="warning" - formatter={thresholdFormatter} firstTimestamp={firstTimestamp} lastTimestamp={lastTimestamp} domain={domain} 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 ea37f7adda7c..b231ef423a96 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 @@ -40,8 +40,14 @@ export const evaluateCondition = async ( filterQuery?: string, lookbackSize?: number ): Promise> => { - const { comparator, warningComparator, metric, customMetric } = condition; - let { threshold, warningThreshold } = condition; + const { + comparator, + warningComparator, + metric, + customMetric, + threshold, + warningThreshold, + } = condition; const timerange = { to: Date.now(), @@ -62,9 +68,6 @@ export const evaluateCondition = async ( customMetric ); - threshold = threshold.map((n) => convertMetricValue(metric, n)); - warningThreshold = warningThreshold?.map((n) => convertMetricValue(metric, n)); - const valueEvaluator = (value?: DataValue, t?: number[], c?: Comparator) => { if (value === undefined || value === null || !t || !c) return [false]; const comparisonFunction = comparatorMap[c]; @@ -166,16 +169,3 @@ const comparatorMap = { [Comparator.GT_OR_EQ]: (a: number, [b]: number[]) => a >= b, [Comparator.LT_OR_EQ]: (a: number, [b]: number[]) => a <= b, }; - -// Some metrics in the UI are in a different unit that what we store in ES. -const convertMetricValue = (metric: SnapshotMetricType, value: number) => { - if (converters[metric]) { - return converters[metric](value); - } else { - return value; - } -}; -const converters: Record number> = { - cpu: (n) => Number(n) / 100, - memory: (n) => Number(n) / 100, -};