Skip to content

Commit

Permalink
[Custom threshold] Use percent unit if all metrics are percent (#179841)
Browse files Browse the repository at this point in the history 
Closes #171121

## Summary

Now, for multiple aggregations, the unit will be changed to percent if
all metrics are percent.


https://github.com/elastic/kibana/assets/12370520/37cc6bd0-b75d-4dba-8529-b40f9fb41ba2
  • Loading branch information
@maryam-saeidi
maryam-saeidi authored Apr 4, 2024
1 parent cc840e2 commit 1e0ebaa
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 104 deletions.
8 changes: 4 additions & 4 deletions .../public/components/custom_threshold/components/custom_equation/custom_equation_editor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +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 { convertToApiThreshold } from '../../helpers/threshold_unit';
import {
Aggregators,
CustomThresholdExpressionMetric,
Expand Down Expand Up @@ -81,7 +81,7 @@ export function CustomEquationEditor({
...expression,
metrics: nextMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
threshold: convertToApiThreshold(previous, nextMetrics, expression.threshold),
});
return nextMetrics;
});
Expand All @@ -96,7 +96,7 @@ export function CustomEquationEditor({
...expression,
metrics: finalMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
threshold: convertToApiThreshold(previous, nextMetrics, expression.threshold),
});
return finalMetrics;
});
Expand All @@ -112,7 +112,7 @@ export function CustomEquationEditor({
...expression,
metrics: nextMetrics,
equation,
threshold: adjustThresholdBasedOnFormat(previous, nextMetrics, expression.threshold),
threshold: convertToApiThreshold(previous, nextMetrics, expression.threshold),
});
return nextMetrics;
});
Expand Down
6 changes: 2 additions & 4 deletions ...y_solution/observability/public/components/custom_threshold/components/expression_row.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { MetricExpression } from '../types';
import { CustomEquationEditor } from './custom_equation';
import { CUSTOM_EQUATION, LABEL_HELP_MESSAGE, LABEL_LABEL } from '../i18n_strings';
import { decimalToPct, pctToDecimal } from '../helpers/corrected_percent_convert';
import { isPercent } from '../helpers/threshold_unit';

// Create a new object with COMPARATORS.NOT_BETWEEN removed as we use OUTSIDE_RANGE
const updatedBuiltInComparators = { ...builtInComparators };
Expand Down Expand Up @@ -84,10 +85,7 @@ export const ExpressionRow: React.FC<ExpressionRowProps> = (props) => {

const { metrics, comparator = Comparator.GT, threshold = [] } = expression;

const isMetricPct = useMemo(
() => Boolean(metrics.length === 1 && metrics[0].field?.endsWith('.pct')),
[metrics]
);
const isMetricPct = useMemo(() => isPercent(metrics), [metrics]);
const [label, setLabel] = useState<string | undefined>(expression?.label || undefined);

const updateComparator = useCallback(
Expand Down
93 changes: 0 additions & 93 deletions ...ility/public/components/custom_threshold/helpers/adjust_threshold_based_on_format.test.ts
View file

This file was deleted.

182 changes: 182 additions & 0 deletions ..._solution/observability/public/components/custom_threshold/helpers/threshold_unit.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/*
* 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 { convertToApiThreshold } from './threshold_unit';

describe('convertToApiThreshold', () => {
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(convertToApiThreshold(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(convertToApiThreshold(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(convertToApiThreshold(previous, next, threshold)).toEqual(threshold);
});

test('previous: nonPercent, next: nonPercent -> no threshold change', () => {
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(convertToApiThreshold(previous, next, threshold)).toEqual(threshold);
});

test('multiple metrics (previous: one percent, next: one percent) -> no threshold change', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'host.disk.read.bytes',
aggType: Aggregators.AVERAGE,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const threshold: number[] = [100];

expect(convertToApiThreshold(previous, next, threshold)).toEqual(threshold);
});

test('multiple metrics (previous: ALL percent, next: some percent) -> threshold * 100', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.user.pct',
aggType: Aggregators.AVERAGE,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const threshold: number[] = [1];
const expectedThreshold: number[] = [100];

expect(convertToApiThreshold(previous, next, threshold)).toEqual(expectedThreshold);
});

test('multiple metrics (previous: some percent, next: ALL percent) -> threshold / 100', () => {
const previous: CustomThresholdExpressionMetric[] = [
{
name: 'A',
aggType: Aggregators.COUNT,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const next: CustomThresholdExpressionMetric[] = [
{
name: 'A',
field: 'system.cpu.user.pct',
aggType: Aggregators.AVERAGE,
},
{
name: 'B',
field: 'system.cpu.system.pct',
aggType: Aggregators.AVERAGE,
},
];
const threshold: number[] = [100];
const expectedThreshold: number[] = [1];

expect(convertToApiThreshold(previous, next, threshold)).toEqual(expectedThreshold);
});
});
10 changes: 7 additions & 3 deletions ...lpers/adjust_threshold_based_on_format.ts → ...ustom_threshold/helpers/threshold_unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
import { CustomThresholdExpressionMetric } from '../../../../common/custom_threshold_rule/types';
import { decimalToPct, pctToDecimal } from './corrected_percent_convert';

export const adjustThresholdBasedOnFormat = (
export const convertToApiThreshold = (
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'));
const isPreviousPercent = Boolean(previous.every((metric) => metric.field?.endsWith('.pct')));
const isPercent = Boolean(next.every((metric) => metric.field?.endsWith('.pct')));

return isPercent === isPreviousPercent
? threshold
: isPercent
Expand All @@ -23,3 +24,6 @@ export const adjustThresholdBasedOnFormat = (
? threshold.map((v: number) => decimalToPct(v))
: threshold;
};

export const isPercent = (metrics: CustomThresholdExpressionMetric[]) =>
Boolean(metrics.every((metric) => metric.field?.endsWith('.pct')));

0 comments on commit 1e0ebaa

Please sign in to comment.