[Custom threshold] Fix threshold change when unit is percent (pct) (#171857)

Partially implements #171121

## Summary

This PR fixes the threshold change when the field unit is percent (pct).



816158b9-eb5c-4dd0-ab72-0c3d485150b1
This commit is contained in:
Maryam Saeidi 2023-11-24 09:36:36 +01:00 committed by GitHub
parent 97b1851f00
commit 553bcde9af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 140 additions and 6 deletions

View file

@ -22,6 +22,7 @@ import { IErrorObject } from '@kbn/triggers-actions-ui-plugin/public';
import { FormattedMessage } from '@kbn/i18n-react';
import { DataViewBase } from '@kbn/es-query';
import { i18n } from '@kbn/i18n';
import { adjustThresholdBasedOnFormat } from '../../helpers/adjust_threshold_based_on_format';
import {
Aggregators,
CustomThresholdExpressionMetric,
@ -71,7 +72,12 @@ export function CustomEquationEditor({
const currentVars = previous?.map((m) => m.name) ?? [];
const name = first(xor(VAR_NAMES, currentVars))!;
const nextMetrics = [...(previous || []), { ...NEW_METRIC, name }];
debouncedOnChange({ ...expression, metrics: nextMetrics, equation });
debouncedOnChange({
...expression,
metrics: nextMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
});
return nextMetrics;
});
}, [debouncedOnChange, equation, expression]);
@ -81,7 +87,12 @@ export function CustomEquationEditor({
setCustomMetrics((previous) => {
const nextMetrics = previous?.filter((row) => row.name !== name) ?? [NEW_METRIC];
const finalMetrics = (nextMetrics.length && nextMetrics) || [NEW_METRIC];
debouncedOnChange({ ...expression, metrics: finalMetrics, equation });
debouncedOnChange({
...expression,
metrics: finalMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
});
return finalMetrics;
});
},
@ -92,7 +103,12 @@ export function CustomEquationEditor({
(metric: CustomThresholdExpressionMetric) => {
setCustomMetrics((previous) => {
const nextMetrics = previous?.map((m) => (m.name === metric.name ? metric : m));
debouncedOnChange({ ...expression, metrics: nextMetrics, equation });
debouncedOnChange({
...expression,
metrics: nextMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
});
return nextMetrics;
});
},

View file

@ -120,7 +120,7 @@ export function MetricRowWithAgg({
fullWidth
label={
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiFlexItem grow={false} css={{ paddingTop: 2, paddingBottom: 2 }}>
{i18n.translate(
'xpack.observability.customThreshold.rule.alertFlyout.customEquationEditor.aggregationLabel',
{ defaultMessage: 'Aggregation {name}', values: { name } }

View file

@ -92,7 +92,7 @@ describe('Expression', () => {
},
],
comparator: Comparator.GT,
threshold: [1000],
threshold: [100],
timeSize: 1,
timeUnit: 'm',
},

View file

@ -59,7 +59,7 @@ export const defaultExpression: MetricExpression = {
aggType: Aggregators.COUNT,
},
],
threshold: [1000],
threshold: [100],
timeSize: 1,
timeUnit: 'm',
};

View file

@ -0,0 +1,93 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import {
Aggregators,
CustomThresholdExpressionMetric,
} from '../../../../common/custom_threshold_rule/types';
import { adjustThresholdBasedOnFormat } from './adjust_threshold_based_on_format';
describe('adjustThresholdBasedOnFormat', () => {
test('previous: nonPercent, next: percent -> threshold / 100', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const threshold: number[] = [100];
const expectedThreshold: number[] = [1];
expect(adjustThresholdBasedOnFormat(previous, next, threshold)).toEqual(expectedThreshold);
});
test('previous: percent, next: nonPercent -> threshold * 100', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
];
const threshold: number[] = [1];
const expectedThreshold: number[] = [100];
expect(adjustThresholdBasedOnFormat(previous, next, threshold)).toEqual(expectedThreshold);
});
test('previous: percent, next: percent -> no threshold change', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.total.norm.pct',
aggType: Aggregators.AVERAGE,
},
];
const threshold: number[] = [1];
expect(adjustThresholdBasedOnFormat(previous, next, threshold)).toEqual(threshold);
});
test('previous: nonPercent, next: nonPercent -> threshold * 100', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'host.disk.read.bytes',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
];
const threshold: number[] = [100];
expect(adjustThresholdBasedOnFormat(previous, next, threshold)).toEqual(threshold);
});
});

View file

@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { CustomThresholdExpressionMetric } from '../../../../common/custom_threshold_rule/types';
import { decimalToPct, pctToDecimal } from './corrected_percent_convert';
export const adjustThresholdBasedOnFormat = (
previous: CustomThresholdExpressionMetric[],
next: CustomThresholdExpressionMetric[],
threshold: number[]
) => {
const isPreviousPercent = Boolean(previous.length === 1 && previous[0].field?.endsWith('.pct'));
const isPercent = Boolean(next.length === 1 && next[0].field?.endsWith('.pct'));
return isPercent === isPreviousPercent
? threshold
: isPercent
? threshold.map((v: number) => pctToDecimal(v))
: isPreviousPercent
? threshold.map((v: number) => decimalToPct(v))
: threshold;
};