diff --git a/x-pack/plugins/infra/common/utils/corrected_percent_convert.test.ts b/x-pack/plugins/infra/common/utils/corrected_percent_convert.test.ts new file mode 100644 index 00000000000000..ab608f331b8843 --- /dev/null +++ b/x-pack/plugins/infra/common/utils/corrected_percent_convert.test.ts @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { decimalToPct, pctToDecimal } from './corrected_percent_convert'; + +describe('decimalToPct', () => { + test('should retain correct floating point precision up to 10 decimal places', () => { + // Most of these cases would still work fine just doing x * 100 instead of passing it through + // decimalToPct, but the function still needs to work regardless + expect(decimalToPct(0)).toBe(0); + expect(decimalToPct(0.1)).toBe(10); + expect(decimalToPct(0.01)).toBe(1); + expect(decimalToPct(0.014)).toBe(1.4); + expect(decimalToPct(0.0141)).toBe(1.41); + expect(decimalToPct(0.01414)).toBe(1.414); + // This case is known to fail without decimalToPct; vanilla JS 0.014141 * 100 === 1.4141000000000001 + expect(decimalToPct(0.014141)).toBe(1.4141); + expect(decimalToPct(0.0141414)).toBe(1.41414); + expect(decimalToPct(0.01414141)).toBe(1.414141); + expect(decimalToPct(0.014141414)).toBe(1.4141414); + }); + test('should also work with values greater than 1', () => { + expect(decimalToPct(2)).toBe(200); + expect(decimalToPct(2.1)).toBe(210); + expect(decimalToPct(2.14)).toBe(214); + expect(decimalToPct(2.14141414)).toBe(214.141414); + }); +}); + +describe('pctToDecimal', () => { + test('should retain correct floating point precision up to 10 decimal places', () => { + expect(pctToDecimal(0)).toBe(0); + expect(pctToDecimal(10)).toBe(0.1); + expect(pctToDecimal(1)).toBe(0.01); + expect(pctToDecimal(1.4)).toBe(0.014); + expect(pctToDecimal(1.41)).toBe(0.0141); + expect(pctToDecimal(1.414)).toBe(0.01414); + expect(pctToDecimal(1.4141)).toBe(0.014141); + expect(pctToDecimal(1.41414)).toBe(0.0141414); + expect(pctToDecimal(1.414141)).toBe(0.01414141); + expect(pctToDecimal(1.4141414)).toBe(0.014141414); + }); + test('should also work with values greater than 100%', () => { + expect(pctToDecimal(200)).toBe(2); + expect(pctToDecimal(210)).toBe(2.1); + expect(pctToDecimal(214)).toBe(2.14); + expect(pctToDecimal(214.141414)).toBe(2.14141414); + }); +}); diff --git a/x-pack/plugins/infra/common/utils/corrected_percent_convert.ts b/x-pack/plugins/infra/common/utils/corrected_percent_convert.ts new file mode 100644 index 00000000000000..a8e3db5133cf57 --- /dev/null +++ b/x-pack/plugins/infra/common/utils/corrected_percent_convert.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +const correctedPctConvert = (v: number, decimalToPct: boolean) => { + // Correct floating point precision + const replacementPattern = decimalToPct ? new RegExp(/0?\./) : '.'; + const numberOfDigits = String(v).replace(replacementPattern, '').length; + const multipliedValue = decimalToPct ? v * 100 : v / 100; + return parseFloat(multipliedValue.toPrecision(numberOfDigits)); +}; + +export const decimalToPct = (v: number) => correctedPctConvert(v, true); +export const pctToDecimal = (v: number) => correctedPctConvert(v, false); diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.test.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.test.tsx new file mode 100644 index 00000000000000..d5be8a2ec2675e --- /dev/null +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.test.tsx @@ -0,0 +1,84 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; +import { MetricExpression } from '../types'; +import React from 'react'; +import { ExpressionRow } from './expression_row'; +import { act } from 'react-dom/test-utils'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Comparator } from '../../../../server/lib/alerting/metric_threshold/types'; + +jest.mock('../../../containers/source/use_source_via_http', () => ({ + useSourceViaHttp: () => ({ + source: { id: 'default' }, + createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }), + }), +})); + +describe('ExpressionRow', () => { + async function setup(expression: MetricExpression) { + const wrapper = mountWithIntl( + {}} + addExpression={() => {}} + key={1} + expressionId={1} + setAlertParams={() => {}} + errors={{ + aggField: [], + timeSizeUnit: [], + timeWindowSize: [], + }} + expression={expression} + /> + ); + + const update = async () => + await act(async () => { + await nextTick(); + wrapper.update(); + }); + + await update(); + + return { wrapper, update }; + } + + it('should display thresholds as a percentage for pct metrics', async () => { + const expression = { + metric: 'system.cpu.user.pct', + comparator: Comparator.GT, + threshold: [0.5], + timeSize: 1, + timeUnit: 'm', + aggType: 'avg', + }; + const { wrapper } = await setup(expression as MetricExpression); + const [valueMatch] = wrapper.html().match('50') ?? []; + expect(valueMatch).toBeTruthy(); + }); + + it('should display thresholds as a decimal for all other metrics', async () => { + const expression = { + metric: 'system.load.1', + comparator: Comparator.GT, + threshold: [0.5], + timeSize: 1, + timeUnit: 'm', + aggType: 'avg', + }; + const { wrapper } = await setup(expression as MetricExpression); + const [valueMatch] = + wrapper.html().match('0.5') ?? []; + expect(valueMatch).toBeTruthy(); + }); +}); diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx index 653b9e1d5c308e..1487557bde3a01 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx @@ -3,10 +3,11 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useState, useMemo } from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon, EuiSpacer } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon, EuiSpacer, EuiText } from '@elastic/eui'; import { IFieldType } from 'src/plugins/data/public'; +import { pctToDecimal, decimalToPct } from '../../../../common/utils/corrected_percent_convert'; import { WhenExpression, OfExpression, @@ -76,6 +77,8 @@ export const ExpressionRow: React.FC = (props) => { threshold = [], } = expression; + const isMetricPct = useMemo(() => metric && metric.endsWith('.pct'), [metric]); + const updateAggType = useCallback( (at: string) => { setAlertParams(expressionId, { @@ -102,14 +105,22 @@ export const ExpressionRow: React.FC = (props) => { ); const updateThreshold = useCallback( - (t) => { + (enteredThreshold) => { + const t = isMetricPct + ? enteredThreshold.map((v: number) => pctToDecimal(v)) + : enteredThreshold; if (t.join() !== expression.threshold.join()) { setAlertParams(expressionId, { ...expression, threshold: t }); } }, - [expressionId, expression, setAlertParams] + [expressionId, expression, isMetricPct, setAlertParams] ); + const displayedThreshold = useMemo(() => { + if (isMetricPct) return threshold.map((v) => decimalToPct(v)); + return threshold; + }, [threshold, isMetricPct]); + return ( <> @@ -149,13 +160,22 @@ export const ExpressionRow: React.FC = (props) => { + {isMetricPct && ( +
+ % +
+ )} {canDelete && ( 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 fa705798baf7a2..3a52bb6b6ce710 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 @@ -387,6 +387,36 @@ describe('The metric threshold alert type', () => { // expect(getState(instanceID).alertState).toBe(AlertStates.OK); // }); // }); + + describe('querying a metric with a percentage metric', () => { + const instanceID = '*'; + const execute = () => + executor({ + services, + params: { + sourceId: 'default', + criteria: [ + { + ...baseCriterion, + metric: 'test.metric.pct', + comparator: Comparator.GT, + threshold: [0.75], + }, + ], + }, + }); + 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('*'); + expect(action.reason).toContain('current value is 100%'); + 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()); + }); + }); }); const createMockStaticConfiguration = (sources: any) => ({ 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 b2a8f0281b9e2d..9265e8089e9154 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 @@ -14,6 +14,7 @@ import { buildNoDataAlertReason, stateToAlertMessage, } from '../common/messages'; +import { createFormatter } from '../../../../common/formatters'; import { AlertStates } from './types'; import { evaluateAlert } from './lib/evaluate_alert'; @@ -59,7 +60,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => let reason; if (nextState === AlertStates.ALERT) { reason = alertResults - .map((result) => buildFiredAlertReason(result[group] as any)) + .map((result) => buildFiredAlertReason(formatAlertResult(result[group]) as any)) .join('\n'); } if (alertOnNoData) { @@ -83,8 +84,14 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => alertState: stateToAlertMessage[nextState], reason, timestamp, - value: mapToConditionsLookup(alertResults, (result) => result[group].currentValue), - threshold: mapToConditionsLookup(criteria, (c) => c.threshold), + value: mapToConditionsLookup( + alertResults, + (result) => formatAlertResult(result[group]).currentValue + ), + threshold: mapToConditionsLookup( + alertResults, + (result) => formatAlertResult(result[group]).threshold + ), metric: mapToConditionsLookup(criteria, (c) => c.metric), }); } @@ -113,3 +120,18 @@ const mapToConditionsLookup = ( (result: Record, value, i) => ({ ...result, [`condition${i}`]: value }), {} ); + +const formatAlertResult = (alertResult: { + metric: string; + currentValue: number; + threshold: number[]; +}) => { + const { metric, currentValue, threshold } = alertResult; + if (!metric.endsWith('.pct')) return alertResult; + const formatter = createFormatter('percent'); + return { + ...alertResult, + currentValue: formatter(currentValue), + threshold: Array.isArray(threshold) ? threshold.map((v: number) => formatter(v)) : threshold, + }; +};