Skip to content

Commit

Permalink
Rename customMetric to metrics in the custom threshold rule criteria (#…
Browse files Browse the repository at this point in the history
…165719)

Closes #163819

## Summary

This PR renames customMetric to metrics in the custom threshold rule
criteria to have a cleaner API for this rule, considering that custom
aggregation is now the default option.

<img
src="https://github.com/elastic/kibana/assets/12370520/8953e7ad-aa2b-4bf6-b568-ff269ab84ca5"
width=300/>


### 🧪 How to test
- Create a custom threshold rule and ensure the preview is working fine
- Check the related saved object and ensure the data is saved as shown
above
- Make sure the related alerts are generated as expected
  • Loading branch information
maryam-saeidi authored Sep 7, 2023
1 parent b6d6f60 commit 0c2d483
Show file tree
Hide file tree
Showing 27 changed files with 66 additions and 74 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/observability/common/threshold_rule/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export type CustomMetricAggTypes = Exclude<
Aggregators.CUSTOM | Aggregators.RATE | Aggregators.P95 | Aggregators.P99
>;

export interface MetricExpressionCustomMetric {
export interface CustomThresholdExpressionMetric {
name: string;
aggType: CustomMetricAggTypes;
field?: string;
Expand All @@ -243,7 +243,7 @@ export interface MetricExpressionCustomMetric {

export interface CustomMetricExpressionParams extends BaseMetricExpressionParams {
aggType: Aggregators.CUSTOM;
customMetrics: MetricExpressionCustomMetric[];
metrics: CustomThresholdExpressionMetric[];
equation?: string;
label?: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ CustomEquationEditorWithEquationErrors.args = {
expression: {
...BASE_ARGS.expression,
equation: 'Math.round(A / B)',
customMetrics: [
metrics: [
{ name: 'A', aggType: Aggregators.AVERAGE, field: 'system.cpu.user.pct' },
{ name: 'B', aggType: Aggregators.MAX, field: 'system.cpu.cores' },
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { OMITTED_AGGREGATIONS_FOR_CUSTOM_METRICS } from '../../../../../common/t
import {
Aggregators,
CustomMetricAggTypes,
MetricExpressionCustomMetric,
CustomThresholdExpressionMetric,
} from '../../../../../common/threshold_rule/types';

import { MetricExpression } from '../../types';
Expand Down Expand Up @@ -58,7 +58,7 @@ export function CustomEquationEditor({
dataView,
}: CustomEquationEditorProps) {
const [customMetrics, setCustomMetrics] = useState<CustomMetrics>(
expression?.customMetrics ?? [NEW_METRIC]
expression?.metrics ?? [NEW_METRIC]
);
const [customEqPopoverOpen, setCustomEqPopoverOpen] = useState(false);
const [equation, setEquation] = useState<string | undefined>(expression?.equation || undefined);
Expand All @@ -69,7 +69,7 @@ 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, customMetrics: nextMetrics, equation });
debouncedOnChange({ ...expression, metrics: nextMetrics, equation });
return nextMetrics;
});
}, [debouncedOnChange, equation, expression]);
Expand All @@ -79,18 +79,18 @@ export function CustomEquationEditor({
setCustomMetrics((previous) => {
const nextMetrics = previous?.filter((row) => row.name !== name) ?? [NEW_METRIC];
const finalMetrics = (nextMetrics.length && nextMetrics) || [NEW_METRIC];
debouncedOnChange({ ...expression, customMetrics: finalMetrics, equation });
debouncedOnChange({ ...expression, metrics: finalMetrics, equation });
return finalMetrics;
});
},
[equation, expression, debouncedOnChange]
);

const handleChange = useCallback(
(metric: MetricExpressionCustomMetric) => {
(metric: CustomThresholdExpressionMetric) => {
setCustomMetrics((previous) => {
const nextMetrics = previous?.map((m) => (m.name === metric.name ? metric : m));
debouncedOnChange({ ...expression, customMetrics: nextMetrics, equation });
debouncedOnChange({ ...expression, metrics: nextMetrics, equation });
return nextMetrics;
});
},
Expand All @@ -100,7 +100,7 @@ export function CustomEquationEditor({
const handleEquationChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
setEquation(e.target.value);
debouncedOnChange({ ...expression, customMetrics, equation: e.target.value });
debouncedOnChange({ ...expression, metrics: customMetrics, equation: e.target.value });
},
[debouncedOnChange, expression, customMetrics]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export function MetricRowWithAgg({
[name, aggType, onChange]
);

const isAggInvalid = get(errors, ['customMetrics', name, 'aggType']) != null;
const isFieldInvalid = get(errors, ['customMetrics', name, 'field']) != null || !field;
const isAggInvalid = get(errors, ['metrics', name, 'aggType']) != null;
const isFieldInvalid = get(errors, ['metrics', name, 'field']) != null || !field;

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
* 2.0.
*/
import { AggregationType, IErrorObject } from '@kbn/triggers-actions-ui-plugin/public';
import { MetricExpressionCustomMetric } from '../../../../../common/threshold_rule/types';
import { CustomThresholdExpressionMetric } from '../../../../../common/threshold_rule/types';
import { MetricExpression } from '../../types';

export type CustomMetrics = MetricExpression['customMetrics'];
export type CustomMetrics = MetricExpression['metrics'];

export interface AggregationTypes {
[x: string]: AggregationType;
Expand All @@ -27,7 +27,7 @@ export interface MetricRowBaseProps {
onDelete: (name: string) => void;
disableDelete: boolean;
disableAdd: boolean;
onChange: (metric: MetricExpressionCustomMetric) => void;
onChange: (metric: CustomThresholdExpressionMetric) => void;
aggregationTypes: AggregationTypes;
errors: IErrorObject;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ export function validateMetricThreshold({
threshold1: string[];
};
metric: string[];
customMetricsError?: string;
customMetrics: Record<string, { aggType?: string; field?: string; filter?: string }>;
metricsError?: string;
metrics: Record<string, { aggType?: string; field?: string; filter?: string }>;
equation?: string;
};
} & { filterQuery?: string[]; searchConfiguration?: string[] } = {};
Expand Down Expand Up @@ -100,7 +100,7 @@ export function validateMetricThreshold({
threshold1: [],
},
metric: [],
customMetrics: {},
metrics: {},
};
if (!c.aggType) {
errors[id].aggField.push(
Expand Down Expand Up @@ -179,27 +179,27 @@ export function validateMetricThreshold({
}

if (isCustomMetricExpressionParams(c)) {
if (!c.customMetrics || (c.customMetrics && c.customMetrics.length < 1)) {
errors[id].customMetricsError = i18n.translate(
'xpack.observability.threshold.rule.alertFlyout.error.customMetricsError',
if (!c.metrics || (c.metrics && c.metrics.length < 1)) {
errors[id].metricsError = i18n.translate(
'xpack.observability.threshold.rule.alertFlyout.error.metricsError',
{
defaultMessage: 'You must define at least 1 custom metric',
}
);
} else {
c.customMetrics.forEach((metric) => {
c.metrics.forEach((metric) => {
const customMetricErrors: { aggType?: string; field?: string; filter?: string } = {};
if (!metric.aggType) {
customMetricErrors.aggType = i18n.translate(
'xpack.observability.threshold.rule.alertFlyout.error.customMetrics.aggTypeRequired',
'xpack.observability.threshold.rule.alertFlyout.error.metrics.aggTypeRequired',
{
defaultMessage: 'Aggregation is required',
}
);
}
if (metric.aggType !== 'count' && !metric.field) {
customMetricErrors.field = i18n.translate(
'xpack.observability.threshold.rule.alertFlyout.error.customMetrics.fieldRequired',
'xpack.observability.threshold.rule.alertFlyout.error.metrics.fieldRequired',
{
defaultMessage: 'Field is required',
}
Expand All @@ -213,7 +213,7 @@ export function validateMetricThreshold({
}
}
if (!isEmpty(customMetricErrors)) {
errors[id].customMetrics[metric.name] = customMetricErrors;
errors[id].metrics[metric.name] = customMetricErrors;
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import DateMath from '@kbn/datemath';
import { DataViewBase } from '@kbn/es-query';
import { useMemo } from 'react';
import { MetricExplorerCustomMetricAggregations } from '../../../../common/threshold_rule/metrics_explorer';
import { MetricExpressionCustomMetric } from '../../../../common/threshold_rule/types';
import { CustomThresholdExpressionMetric } from '../../../../common/threshold_rule/types';
import { MetricExpression, TimeRange } from '../types';
import { useMetricsExplorerData } from './use_metrics_explorer_data';

Expand Down Expand Up @@ -43,8 +43,7 @@ export const useMetricsExplorerChartData = (
? {
aggregation: 'custom',
custom_metrics:
expression?.customMetrics?.map(mapMetricThresholdMetricToMetricsExplorerMetric) ??
[],
expression?.metrics?.map(mapMetricThresholdMetricToMetricsExplorerMetric) ?? [],
equation: expression.equation,
}
: { field: expression.metric, aggregation: expression.aggType },
Expand All @@ -57,7 +56,7 @@ export const useMetricsExplorerChartData = (
expression.equation,
expression.metric,
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(expression.customMetrics),
JSON.stringify(expression.metrics),
filterQuery,
groupBy,
]
Expand All @@ -78,7 +77,9 @@ export const useMetricsExplorerChartData = (
return useMetricsExplorerData(options, derivedIndexPattern, timestamps);
};

const mapMetricThresholdMetricToMetricsExplorerMetric = (metric: MetricExpressionCustomMetric) => {
const mapMetricThresholdMetricToMetricsExplorerMetric = (
metric: CustomThresholdExpressionMetric
) => {
if (metric.aggType === 'count') {
return {
name: metric.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export default function Expressions(props: Props) {
(newDataView: DataView) => {
const ruleCriteria = (ruleParams.criteria ? ruleParams.criteria.slice() : []).map(
(criterion) => {
criterion.customMetrics?.forEach((metric) => {
criterion.metrics?.forEach((metric) => {
metric.field = undefined;
});
return criterion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ export interface AlertContextMeta {

export type MetricExpression = Omit<
MetricExpressionParams,
'metric' | 'timeSize' | 'timeUnit' | 'metrics' | 'equation' | 'customMetrics'
'metric' | 'timeSize' | 'timeUnit' | 'metrics' | 'equation'
> & {
metric?: NonCountMetricExpressionParams['metric'];
customMetrics?: CustomMetricExpressionParams['customMetrics'];
metrics?: CustomMetricExpressionParams['metrics'];
label?: CustomMetricExpressionParams['label'];
equation?: CustomMetricExpressionParams['equation'];
timeSize?: MetricExpressionParams['timeSize'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@

import { fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query';
import { isEmpty } from 'lodash';
import { MetricExpressionCustomMetric } from '../../../../../common/threshold_rule/types';
import { CustomThresholdExpressionMetric } from '../../../../../common/threshold_rule/types';
import { MetricsExplorerCustomMetric } from './metrics_explorer';

const isMetricExpressionCustomMetric = (
subject: MetricsExplorerCustomMetric | MetricExpressionCustomMetric
): subject is MetricExpressionCustomMetric => {
return (subject as MetricExpressionCustomMetric).aggType != null;
subject: MetricsExplorerCustomMetric | CustomThresholdExpressionMetric
): subject is CustomThresholdExpressionMetric => {
return 'aggType' in subject;
};

export const createCustomMetricsAggregations = (
id: string,
customMetrics: Array<MetricsExplorerCustomMetric | MetricExpressionCustomMetric>,
customMetrics: Array<MetricsExplorerCustomMetric | CustomThresholdExpressionMetric>,
equation?: string
) => {
const bucketsPath: { [id: string]: string } = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export const getElasticsearchMetricQuery = (
: isCustom(metricParams)
? createCustomMetricsAggregations(
'aggregatedValue',
metricParams.customMetrics,
metricParams.metrics,
metricParams.equation
)
: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function thresholdRuleType(
...baseCriterion,
metric: schema.string(),
aggType: oneOfLiterals(METRIC_EXPLORER_AGGREGATIONS),
customMetrics: schema.never(),
metrics: schema.never(),
equation: schema.never(),
label: schema.never(),
});
Expand All @@ -84,7 +84,7 @@ export function thresholdRuleType(
...baseCriterion,
aggType: schema.literal('count'),
metric: schema.never(),
customMetrics: schema.never(),
metrics: schema.never(),
equation: schema.never(),
label: schema.never(),
});
Expand All @@ -93,7 +93,7 @@ export function thresholdRuleType(
...baseCriterion,
aggType: schema.literal('custom'),
metric: schema.never(),
customMetrics: schema.arrayOf(
metrics: schema.arrayOf(
schema.oneOf([
schema.object({
name: schema.string(),
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -27673,9 +27673,6 @@
"xpack.observability.threshold.rule.alertFlyout.customEquationEditor.labelLabel": "Étiquette (facultatif)",
"xpack.observability.threshold.rule.alertFlyout.docCountNoDataDisabledHelpText": "[Ce paramètre n’est pas applicable à l’agrégateur du nombre de documents.]",
"xpack.observability.threshold.rule.alertFlyout.error.aggregationRequired": "L'agrégation est requise.",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.aggTypeRequired": "L'agrégation est requise",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.fieldRequired": "Le champ est obligatoire",
"xpack.observability.threshold.rule.alertFlyout.error.customMetricsError": "Vous devez définir au moins 1 indicateur personnalisé",
"xpack.observability.threshold.rule.alertFlyout.error.equation.invalidCharacters": "Le champ d'équation prend en charge uniquement les caractères suivants : A-Z, +, -, /, *, (, ), ?, !, &, :, |, >, <, =",
"xpack.observability.threshold.rule.alertFlyout.error.invalidFilterQuery": "La requête de filtre n'est pas valide.",
"xpack.observability.threshold.rule.alertFlyout.error.metricRequired": "L'indicateur est requis.",
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -27673,9 +27673,6 @@
"xpack.observability.threshold.rule.alertFlyout.customEquationEditor.labelLabel": "ラベル(任意)",
"xpack.observability.threshold.rule.alertFlyout.docCountNoDataDisabledHelpText": "[この設定は、ドキュメントカウントアグリゲーターには適用されません。]",
"xpack.observability.threshold.rule.alertFlyout.error.aggregationRequired": "集約が必要です。",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.aggTypeRequired": "集約が必要です",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.fieldRequired": "フィールドが必要です",
"xpack.observability.threshold.rule.alertFlyout.error.customMetricsError": "1つ以上のカスタムメトリックを定義する必要があります",
"xpack.observability.threshold.rule.alertFlyout.error.equation.invalidCharacters": "等式フィールドでは次の文字のみを使用できます:A-Z、+、-、/、*、(、)、?、!、&、:、|、>、<、=",
"xpack.observability.threshold.rule.alertFlyout.error.invalidFilterQuery": "フィルタークエリは無効です。",
"xpack.observability.threshold.rule.alertFlyout.error.metricRequired": "メトリックが必要です。",
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -27671,9 +27671,6 @@
"xpack.observability.threshold.rule.alertFlyout.customEquationEditor.labelLabel": "标签(可选)",
"xpack.observability.threshold.rule.alertFlyout.docCountNoDataDisabledHelpText": "[此设置不适用于文档计数聚合器。]",
"xpack.observability.threshold.rule.alertFlyout.error.aggregationRequired": "“聚合”必填。",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.aggTypeRequired": "“聚合”必填",
"xpack.observability.threshold.rule.alertFlyout.error.customMetrics.fieldRequired": "“字段”必填",
"xpack.observability.threshold.rule.alertFlyout.error.customMetricsError": "必须至少定义 1 个定制指标",
"xpack.observability.threshold.rule.alertFlyout.error.equation.invalidCharacters": "方程字段仅支持以下字符:A-Z、+、-、/、*、(、)、?、!、&、:、|、>、<、=",
"xpack.observability.threshold.rule.alertFlyout.error.invalidFilterQuery": "筛选查询无效。",
"xpack.observability.threshold.rule.alertFlyout.error.metricRequired": "“指标”必填。",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [
metrics: [
{ name: 'A', field: 'system.cpu.user.pct', aggType: Aggregators.AVERAGE },
],
},
Expand Down Expand Up @@ -169,7 +169,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
metrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
},
],
alertOnNoData: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [
metrics: [
{ name: 'A', field: 'system.cpu.user.pct', aggType: Aggregators.AVERAGE },
],
},
Expand Down Expand Up @@ -163,7 +163,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
metrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
},
],
alertOnNoData: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [7500000],
timeSize: 5,
timeUnit: 'm',
customMetrics: [
metrics: [
{ name: 'A', field: 'span.self_time.sum.us', aggType: Aggregators.AVERAGE },
],
},
Expand Down Expand Up @@ -189,7 +189,7 @@ export default function ({ getService }: FtrProviderContext) {
threshold: [7500000],
timeSize: 5,
timeUnit: 'm',
customMetrics: [{ name: 'A', field: 'span.self_time.sum.us', aggType: 'avg' }],
metrics: [{ name: 'A', field: 'span.self_time.sum.us', aggType: 'avg' }],
},
],
alertOnNoData: true,
Expand Down
Loading

0 comments on commit 0c2d483

Please sign in to comment.