From b6ff8a685930d2fa112b031577b9721741a1050b Mon Sep 17 00:00:00 2001 From: mgiota Date: Mon, 31 Jan 2022 22:17:44 +0100 Subject: [PATCH] [RAC][Metrics] review readability of alert reason messages (#123806) * 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> --- .../infra/common/alerting/metrics/types.ts | 6 +- .../inventory/components/expression.tsx | 6 +- .../components/expression.tsx | 7 +-- .../server/lib/alerting/common/messages.ts | 60 ++++++------------- .../inventory_metric_threshold_executor.ts | 21 ++++++- ...er_inventory_metric_threshold_rule_type.ts | 4 +- .../metric_threshold_executor.test.ts | 21 ++++--- .../metric_threshold_executor.ts | 5 +- .../translations/translations/ja-JP.json | 6 -- .../translations/translations/zh-CN.json | 6 -- 10 files changed, 65 insertions(+), 77 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/metrics/types.ts b/x-pack/plugins/infra/common/alerting/metrics/types.ts index 0216f63b8f85d..082f585270b7a 100644 --- a/x-pack/plugins/infra/common/alerting/metrics/types.ts +++ b/x-pack/plugins/infra/common/alerting/metrics/types.ts @@ -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'; @@ -69,7 +69,7 @@ export interface MetricAnomalyParams { export interface InventoryMetricConditions { metric: SnapshotMetricType; timeSize: number; - timeUnit: Unit; + timeUnit: TimeUnitChar; sourceId?: string; threshold: number[]; comparator: Comparator; @@ -89,7 +89,7 @@ export interface InventoryMetricThresholdParams { interface BaseMetricExpressionParams { timeSize: number; - timeUnit: Unit; + timeUnit: TimeUnitChar; sourceId?: string; threshold: number[]; comparator: Comparator; diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx index f7d52a1aa95f0..babd79a298547 100644 --- a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx @@ -5,7 +5,6 @@ * 2.0. */ -import { Unit } from '@elastic/datemath'; import { EuiButtonEmpty, EuiButtonIcon, @@ -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; @@ -113,7 +113,7 @@ export const Expressions: React.FC = (props) => { toastWarning: notifications.toasts.addWarning, }); const [timeSize, setTimeSize] = useState(1); - const [timeUnit, setTimeUnit] = useState('m'); + const [timeUnit, setTimeUnit] = useState('m'); const derivedIndexPattern = useMemo( () => createDerivedIndexPattern(), @@ -196,7 +196,7 @@ export const Expressions: React.FC = (props) => { ...c, timeUnit: tu, })); - setTimeUnit(tu as Unit); + setTimeUnit(tu as TimeUnitChar); setRuleParams('criteria', criteria as Criteria); }, [ruleParams.criteria, setRuleParams] diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx index e0039c4590069..6fed2cd655958 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx @@ -5,7 +5,6 @@ * 2.0. */ -import { Unit } from '@elastic/datemath'; import { EuiAccordion, EuiButtonEmpty, @@ -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< @@ -66,7 +65,7 @@ export const Expressions: React.FC = (props) => { }); const [timeSize, setTimeSize] = useState(1); - const [timeUnit, setTimeUnit] = useState('m'); + const [timeUnit, setTimeUnit] = useState('m'); const derivedIndexPattern = useMemo( () => createDerivedIndexPattern(), [createDerivedIndexPattern] @@ -168,7 +167,7 @@ export const Expressions: React.FC = (props) => { ...c, timeUnit: tu, })) || []; - setTimeUnit(tu as Unit); + setTimeUnit(tu as TimeUnitChar); setRuleParams('criteria', criteria as AlertParams['criteria']); }, [ruleParams.criteria, setRuleParams] diff --git a/x-pack/plugins/infra/server/lib/alerting/common/messages.ts b/x-pack/plugins/infra/server/lib/alerting/common/messages.ts index d92670a4eb5f6..068861b2bac42 100644 --- a/x-pack/plugins/infra/server/lib/alerting/common/messages.ts +++ b/x-pack/plugins/infra/server/lib/alerting/common/messages.ts @@ -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', @@ -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[], @@ -108,25 +76,31 @@ const thresholdToI18n = ([a, b]: Array) => { }); }; +const formatGroup = (group: string) => (group === UNGROUPED_FACTORY_KEY ? 'all hosts' : group); + export const buildFiredAlertReason: (alertResult: { group: string; metric: string; comparator: Comparator; threshold: Array; 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; @@ -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), }, }); diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts index 497bb0cc960a7..9301f17f4d99c 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts @@ -216,6 +216,21 @@ 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, @@ -223,6 +238,10 @@ const buildReasonWithVerboseMetricName = ( useWarningThreshold?: boolean ) => { if (!resultItem) return ''; + + const thresholdToFormat = useWarningThreshold + ? resultItem.warningThreshold! + : resultItem.threshold; const resultWithVerboseMetricName = { ...resultItem, group, @@ -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); diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_rule_type.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_rule_type.ts index ac68a0df706fb..81d7758886644 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_rule_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_rule_type.ts @@ -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'; @@ -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, - timeUnit: schema.string() as Type, + timeUnit: schema.string() as Type, timeSize: schema.number(), metric: oneOfLiterals(Object.keys(SnapshotMetricTypeKeys)) as Type, warningThreshold: schema.maybe(schema.arrayOf(schema.number())), 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 c007154320db4..b3c4de9658eda 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 @@ -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'); }); }); @@ -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', () => { @@ -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%'); }); 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 8f6c359b150c1..f16b8a8135a37 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 @@ -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; export type MetricThresholdRuleTypeState = RuleTypeState & { @@ -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) => @@ -286,6 +286,8 @@ const formatAlertResult = ( comparator: Comparator; warningThreshold?: number[]; warningComparator?: Comparator; + timeSize: number; + timeUnit: TimeUnitChar; } & AlertResult, useWarningThreshold?: boolean ) => { @@ -305,6 +307,7 @@ const formatAlertResult = ( const formatter = createFormatter('percent'); const thresholdToFormat = useWarningThreshold ? warningThreshold! : threshold; const comparatorToFormat = useWarningThreshold ? warningComparator! : comparator; + return { ...alertResult, currentValue: diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 51881fe5f685a..4b3b85515bc4e 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -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}", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 818a3a28eea68..c68849e290d84 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -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}",