Skip to content

Commit

Permalink
[RAC][Metrics] review readability of alert reason messages (#123806)
Browse files Browse the repository at this point in the history
* adapt inventory reason message to new requirements (interval and proper group name is missing)

* recovered message (waiting for feedback on final format)

* remove % (it appears twice)

* add duration in the reason message

* format group

* add percentage to the inventory threshold & timeUnit to the metric threshold

* bring back old format for recovered messages, since they are currently disabled and input is needed

* remove unused comparatorToI18n

* temp fix typescript errors

* use TimeUnitChar instead of TimeUnit

* format no data message

* fix failing tests

* fix failing tests

* fix CI errors

* more CI tests

* more CI fixes

* use proper formatter for threshold

* i18n checks

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
mgiota and kibanamachine authored Jan 31, 2022
1 parent 41c7521 commit b6ff8a6
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 77 deletions.
6 changes: 3 additions & 3 deletions x-pack/plugins/infra/common/alerting/metrics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { Unit } from '@elastic/datemath';
import * as rt from 'io-ts';
import { SnapshotCustomMetricInput } from '../../http_api';
import { ANOMALY_THRESHOLD } from '../../infra_ml';
import { InventoryItemType, SnapshotMetricType } from '../../inventory_models/types';
import { TimeUnitChar } from '../../../../observability/common/utils/formatters/duration';

export const METRIC_THRESHOLD_ALERT_TYPE_ID = 'metrics.alert.threshold';
export const METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID = 'metrics.alert.inventory.threshold';
Expand Down Expand Up @@ -69,7 +69,7 @@ export interface MetricAnomalyParams {
export interface InventoryMetricConditions {
metric: SnapshotMetricType;
timeSize: number;
timeUnit: Unit;
timeUnit: TimeUnitChar;
sourceId?: string;
threshold: number[];
comparator: Comparator;
Expand All @@ -89,7 +89,7 @@ export interface InventoryMetricThresholdParams {

interface BaseMetricExpressionParams {
timeSize: number;
timeUnit: Unit;
timeUnit: TimeUnitChar;
sourceId?: string;
threshold: number[];
comparator: Comparator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { Unit } from '@elastic/datemath';
import {
EuiButtonEmpty,
EuiButtonIcon,
Expand Down Expand Up @@ -64,6 +63,7 @@ import { convertKueryToElasticSearchQuery } from '../../../utils/kuery';
import { ExpressionChart } from './expression_chart';
import { MetricExpression } from './metric';
import { NodeTypeExpression } from './node_type';
import { TimeUnitChar } from '../../../../../observability/common/utils/formatters/duration';

const FILTER_TYPING_DEBOUNCE_MS = 500;

Expand Down Expand Up @@ -113,7 +113,7 @@ export const Expressions: React.FC<Props> = (props) => {
toastWarning: notifications.toasts.addWarning,
});
const [timeSize, setTimeSize] = useState<number | undefined>(1);
const [timeUnit, setTimeUnit] = useState<Unit>('m');
const [timeUnit, setTimeUnit] = useState<TimeUnitChar>('m');

const derivedIndexPattern = useMemo(
() => createDerivedIndexPattern(),
Expand Down Expand Up @@ -196,7 +196,7 @@ export const Expressions: React.FC<Props> = (props) => {
...c,
timeUnit: tu,
}));
setTimeUnit(tu as Unit);
setTimeUnit(tu as TimeUnitChar);
setRuleParams('criteria', criteria as Criteria);
},
[ruleParams.criteria, setRuleParams]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { Unit } from '@elastic/datemath';
import {
EuiAccordion,
EuiButtonEmpty,
Expand Down Expand Up @@ -39,7 +38,7 @@ import { convertKueryToElasticSearchQuery } from '../../../utils/kuery';
import { AlertContextMeta, AlertParams, MetricExpression } from '../types';
import { ExpressionChart } from './expression_chart';
import { ExpressionRow } from './expression_row';

import { TimeUnitChar } from '../../../../../observability/common/utils/formatters/duration';
const FILTER_TYPING_DEBOUNCE_MS = 500;

type Props = Omit<
Expand All @@ -66,7 +65,7 @@ export const Expressions: React.FC<Props> = (props) => {
});

const [timeSize, setTimeSize] = useState<number | undefined>(1);
const [timeUnit, setTimeUnit] = useState<Unit | undefined>('m');
const [timeUnit, setTimeUnit] = useState<TimeUnitChar | undefined>('m');
const derivedIndexPattern = useMemo(
() => createDerivedIndexPattern(),
[createDerivedIndexPattern]
Expand Down Expand Up @@ -168,7 +167,7 @@ export const Expressions: React.FC<Props> = (props) => {
...c,
timeUnit: tu,
})) || [];
setTimeUnit(tu as Unit);
setTimeUnit(tu as TimeUnitChar);
setRuleParams('criteria', criteria as AlertParams['criteria']);
},
[ruleParams.criteria, setRuleParams]
Expand Down
60 changes: 17 additions & 43 deletions x-pack/plugins/infra/server/lib/alerting/common/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@

import { i18n } from '@kbn/i18n';
import { AlertStates, Comparator } from '../../../../common/alerting/metrics';
import {
formatDurationFromTimeUnitChar,
TimeUnitChar,
} from '../../../../../observability/common/utils/formatters/duration';
import { UNGROUPED_FACTORY_KEY } from './utils';

export const DOCUMENT_COUNT_I18N = i18n.translate(
'xpack.infra.metrics.alerting.threshold.documentCount',
Expand Down Expand Up @@ -36,43 +41,6 @@ export const stateToAlertMessage = {
const toNumber = (value: number | string) =>
typeof value === 'string' ? parseFloat(value) : value;

const comparatorToI18n = (comparator: Comparator, threshold: number[], currentValue: number) => {
const gtText = i18n.translate('xpack.infra.metrics.alerting.threshold.gtComparator', {
defaultMessage: 'greater than',
});
const ltText = i18n.translate('xpack.infra.metrics.alerting.threshold.ltComparator', {
defaultMessage: 'less than',
});
const eqText = i18n.translate('xpack.infra.metrics.alerting.threshold.eqComparator', {
defaultMessage: 'equal to',
});

switch (comparator) {
case Comparator.BETWEEN:
return i18n.translate('xpack.infra.metrics.alerting.threshold.betweenComparator', {
defaultMessage: 'between',
});
case Comparator.OUTSIDE_RANGE:
return i18n.translate('xpack.infra.metrics.alerting.threshold.outsideRangeComparator', {
defaultMessage: 'not between',
});
case Comparator.GT:
return gtText;
case Comparator.LT:
return ltText;
case Comparator.GT_OR_EQ:
case Comparator.LT_OR_EQ: {
if (currentValue === threshold[0]) {
return eqText;
} else if (currentValue < threshold[0]) {
return ltText;
} else {
return gtText;
}
}
}
};

const recoveredComparatorToI18n = (
comparator: Comparator,
threshold: number[],
Expand Down Expand Up @@ -108,25 +76,31 @@ const thresholdToI18n = ([a, b]: Array<number | string>) => {
});
};

const formatGroup = (group: string) => (group === UNGROUPED_FACTORY_KEY ? 'all hosts' : group);

export const buildFiredAlertReason: (alertResult: {
group: string;
metric: string;
comparator: Comparator;
threshold: Array<number | string>;
currentValue: number | string;
}) => string = ({ group, metric, comparator, threshold, currentValue }) =>
timeSize: number;
timeUnit: TimeUnitChar;
}) => string = ({ group, metric, comparator, threshold, currentValue, timeSize, timeUnit }) =>
i18n.translate('xpack.infra.metrics.alerting.threshold.firedAlertReason', {
defaultMessage:
'{metric} is {comparator} a threshold of {threshold} (current value is {currentValue}) for {group}',
'{metric} is {currentValue} in the last {duration} for {group}. Alert when {comparator} {threshold}.',
values: {
group,
group: formatGroup(group),
metric,
comparator: comparatorToI18n(comparator, threshold.map(toNumber), toNumber(currentValue)),
comparator,
threshold: thresholdToI18n(threshold),
currentValue,
duration: formatDurationFromTimeUnitChar(timeSize, timeUnit),
},
});

// Once recovered reason messages are re-enabled, checkout this issue https://github.com/elastic/kibana/issues/121272 regarding latest reason format
export const buildRecoveredAlertReason: (alertResult: {
group: string;
metric: string;
Expand Down Expand Up @@ -157,11 +131,11 @@ export const buildNoDataAlertReason: (alertResult: {
timeUnit: string;
}) => string = ({ group, metric, timeSize, timeUnit }) =>
i18n.translate('xpack.infra.metrics.alerting.threshold.noDataAlertReason', {
defaultMessage: '{metric} has reported no data over the past {interval} for {group}',
defaultMessage: '{metric} reported no data in the last {interval} for {group}',
values: {
metric,
interval: `${timeSize}${timeUnit}`,
group,
group: formatGroup(group),
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,32 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
}
});

const formatThreshold = (metric: SnapshotMetricType, value: number) => {
const metricFormatter = get(METRIC_FORMATTERS, metric, METRIC_FORMATTERS.count);
const formatter = createFormatter(metricFormatter.formatter, metricFormatter.template);

const threshold = Array.isArray(value)
? value.map((v: number) => {
if (metricFormatter.formatter === 'percent') {
v = Number(v) / 100;
}
return formatter(v);
})
: value;
return threshold;
};

const buildReasonWithVerboseMetricName = (
group: string,
resultItem: any,
buildReason: (r: any) => string,
useWarningThreshold?: boolean
) => {
if (!resultItem) return '';

const thresholdToFormat = useWarningThreshold
? resultItem.warningThreshold!
: resultItem.threshold;
const resultWithVerboseMetricName = {
...resultItem,
group,
Expand All @@ -232,7 +251,7 @@ const buildReasonWithVerboseMetricName = (
? getCustomMetricLabel(resultItem.customMetric)
: resultItem.metric),
currentValue: formatMetric(resultItem.metric, resultItem.currentValue),
threshold: useWarningThreshold ? resultItem.warningThreshold! : resultItem.threshold,
threshold: formatThreshold(resultItem.metric, thresholdToFormat),
comparator: useWarningThreshold ? resultItem.warningComparator! : resultItem.comparator,
};
return buildReason(resultWithVerboseMetricName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { Unit } from '@elastic/datemath';
import { schema, Type } from '@kbn/config-schema';
import { i18n } from '@kbn/i18n';
import { PluginSetupContract } from '../../../../../alerting/server';
Expand Down Expand Up @@ -39,11 +38,12 @@ import {
FIRED_ACTIONS_ID,
WARNING_ACTIONS,
} from './inventory_metric_threshold_executor';
import { TimeUnitChar } from '../../../../../observability/common/utils/formatters/duration';

const condition = schema.object({
threshold: schema.arrayOf(schema.number()),
comparator: oneOfLiterals(Object.values(Comparator)) as Type<Comparator>,
timeUnit: schema.string() as Type<Unit>,
timeUnit: schema.string() as Type<TimeUnitChar>,
timeSize: schema.number(),
metric: oneOfLiterals(Object.keys(SnapshotMetricTypeKeys)) as Type<SnapshotMetricType>,
warningThreshold: schema.maybe(schema.arrayOf(schema.number())),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,10 @@ describe('The metric threshold alert type', () => {
await execute(Comparator.GT, [0.75]);
const { action } = mostRecentAction(instanceID);
expect(action.group).toBe('*');
expect(action.reason).toContain('current value is 1');
expect(action.reason).toContain('threshold of 0.75');
expect(action.reason).toContain('is 1');
expect(action.reason).toContain('Alert when > 0.75');
expect(action.reason).toContain('test.metric.1');
expect(action.reason).toContain('in the last 1 min');
});
});

Expand Down Expand Up @@ -343,10 +344,14 @@ describe('The metric threshold alert type', () => {
expect(reasons.length).toBe(2);
expect(reasons[0]).toContain('test.metric.1');
expect(reasons[1]).toContain('test.metric.2');
expect(reasons[0]).toContain('current value is 1');
expect(reasons[1]).toContain('current value is 3');
expect(reasons[0]).toContain('threshold of 1');
expect(reasons[1]).toContain('threshold of 3');
expect(reasons[0]).toContain('is 1');
expect(reasons[1]).toContain('is 3');
expect(reasons[0]).toContain('Alert when >= 1');
expect(reasons[1]).toContain('Alert when >= 3');
expect(reasons[0]).toContain('in the last 1 min');
expect(reasons[1]).toContain('in the last 1 min');
expect(reasons[0]).toContain('for all hosts');
expect(reasons[1]).toContain('for all hosts');
});
});
describe('querying with the count aggregator', () => {
Expand Down Expand Up @@ -714,8 +719,8 @@ describe('The metric threshold alert type', () => {
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.reason).toContain('is 100%');
expect(action.reason).toContain('Alert when > 75%');
expect(action.threshold.condition0[0]).toBe('75%');
expect(action.value.condition0).toBe('100%');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
} from '../common/messages';
import { UNGROUPED_FACTORY_KEY } from '../common/utils';
import { EvaluatedRuleParams, evaluateRule } from './lib/evaluate_rule';
import { TimeUnitChar } from '../../../../../observability/common/utils/formatters/duration';

export type MetricThresholdRuleParams = Record<string, any>;
export type MetricThresholdRuleTypeState = RuleTypeState & {
Expand Down Expand Up @@ -147,7 +148,6 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
const groups = [...new Set([...prevGroups, ...resultGroups])];

const hasGroups = !isEqual(groups, [UNGROUPED_FACTORY_KEY]);

for (const group of groups) {
// AND logic; all criteria must be across the threshold
const shouldAlertFire = alertResults.every((result) =>
Expand Down Expand Up @@ -286,6 +286,8 @@ const formatAlertResult = <AlertResult>(
comparator: Comparator;
warningThreshold?: number[];
warningComparator?: Comparator;
timeSize: number;
timeUnit: TimeUnitChar;
} & AlertResult,
useWarningThreshold?: boolean
) => {
Expand All @@ -305,6 +307,7 @@ const formatAlertResult = <AlertResult>(
const formatter = createFormatter('percent');
const thresholdToFormat = useWarningThreshold ? warningThreshold! : threshold;
const comparatorToFormat = useWarningThreshold ? warningComparator! : comparator;

return {
...alertResult,
currentValue:
Expand Down
6 changes: 0 additions & 6 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -14025,22 +14025,16 @@
"xpack.infra.metrics.alerting.threshold.aboveRecovery": "より大",
"xpack.infra.metrics.alerting.threshold.alertState": "アラート",
"xpack.infra.metrics.alerting.threshold.belowRecovery": "より小",
"xpack.infra.metrics.alerting.threshold.betweenComparator": "の間",
"xpack.infra.metrics.alerting.threshold.betweenRecovery": "の間",
"xpack.infra.metrics.alerting.threshold.defaultActionMessage": "\\{\\{alertName\\}\\} - \\{\\{context.group\\}\\}は状態\\{\\{context.alertState\\}\\}です\n\n理由:\n\\{\\{context.reason\\}\\}\n",
"xpack.infra.metrics.alerting.threshold.documentCount": "ドキュメントカウント",
"xpack.infra.metrics.alerting.threshold.eqComparator": "等しい",
"xpack.infra.metrics.alerting.threshold.errorAlertReason": "{metric}のデータのクエリを試行しているときに、Elasticsearchが失敗しました",
"xpack.infra.metrics.alerting.threshold.errorState": "エラー",
"xpack.infra.metrics.alerting.threshold.fired": "アラート",
"xpack.infra.metrics.alerting.threshold.firedAlertReason": "{metric}は{group}の{comparator} {threshold}のしきい値です(現在の値は{currentValue})",
"xpack.infra.metrics.alerting.threshold.gtComparator": "より大きい",
"xpack.infra.metrics.alerting.threshold.ltComparator": "より小さい",
"xpack.infra.metrics.alerting.threshold.noDataAlertReason": "{metric}は過去{interval}に{group}のデータを報告していません",
"xpack.infra.metrics.alerting.threshold.noDataFormattedValue": "[データなし]",
"xpack.infra.metrics.alerting.threshold.noDataState": "データなし",
"xpack.infra.metrics.alerting.threshold.okState": "OK [回復済み]",
"xpack.infra.metrics.alerting.threshold.outsideRangeComparator": "の間にない",
"xpack.infra.metrics.alerting.threshold.queryErrorAlertReason": "アラートは正しくない形式のKQLクエリを使用しています:{filterQueryText}",
"xpack.infra.metrics.alerting.threshold.recoveredAlertReason": "{metric}は{group}の{comparator} {threshold}のしきい値です(現在の値は{currentValue})",
"xpack.infra.metrics.alerting.threshold.thresholdRange": "{a}と{b}",
Expand Down
6 changes: 0 additions & 6 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -14224,22 +14224,16 @@
"xpack.infra.metrics.alerting.threshold.aboveRecovery": "高于",
"xpack.infra.metrics.alerting.threshold.alertState": "告警",
"xpack.infra.metrics.alerting.threshold.belowRecovery": "低于",
"xpack.infra.metrics.alerting.threshold.betweenComparator": "介于",
"xpack.infra.metrics.alerting.threshold.betweenRecovery": "介于",
"xpack.infra.metrics.alerting.threshold.defaultActionMessage": "\\{\\{alertName\\}\\} - \\{\\{context.group\\}\\} 处于 \\{\\{context.alertState\\}\\} 状态\n\n原因:\n\\{\\{context.reason\\}\\}\n",
"xpack.infra.metrics.alerting.threshold.documentCount": "文档计数",
"xpack.infra.metrics.alerting.threshold.eqComparator": "等于",
"xpack.infra.metrics.alerting.threshold.errorAlertReason": "Elasticsearch 尝试查询 {metric} 的数据时出现故障",
"xpack.infra.metrics.alerting.threshold.errorState": "错误",
"xpack.infra.metrics.alerting.threshold.fired": "告警",
"xpack.infra.metrics.alerting.threshold.firedAlertReason": "对于 {group},{metric} {comparator}阈值 {threshold}(当前值为 {currentValue})",
"xpack.infra.metrics.alerting.threshold.gtComparator": "大于",
"xpack.infra.metrics.alerting.threshold.ltComparator": "小于",
"xpack.infra.metrics.alerting.threshold.noDataAlertReason": "对于 {group},{metric} 在过去 {interval}中未报告数据",
"xpack.infra.metrics.alerting.threshold.noDataFormattedValue": "[无数据]",
"xpack.infra.metrics.alerting.threshold.noDataState": "无数据",
"xpack.infra.metrics.alerting.threshold.okState": "正常 [已恢复]",
"xpack.infra.metrics.alerting.threshold.outsideRangeComparator": "不介于",
"xpack.infra.metrics.alerting.threshold.queryErrorAlertReason": "告警使用格式错误的 KQL 查询:{filterQueryText}",
"xpack.infra.metrics.alerting.threshold.recoveredAlertReason": "对于 {group},{metric} 现在{comparator}阈值 {threshold}(当前值为 {currentValue})",
"xpack.infra.metrics.alerting.threshold.thresholdRange": "{a} 和 {b}",
Expand Down

0 comments on commit b6ff8a6

Please sign in to comment.