From daf3d62b4e7a85ac161edb0856b0255b4dfe7b75 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Thu, 20 Apr 2023 16:25:41 +0200 Subject: [PATCH 1/5] Add threshold component to metric threshold alert details page --- .../components/alert_details_app_section.tsx | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx index 06602c9638865..62d586d2ac47d 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx @@ -5,17 +5,19 @@ * 2.0. */ +import { i18n } from '@kbn/i18n'; import React, { useMemo } from 'react'; import moment from 'moment'; import { EuiFlexGroup, EuiFlexItem, EuiPanel, useEuiTheme } from '@elastic/eui'; import { TopAlert } from '@kbn/observability-plugin/public'; -import { ALERT_END, ALERT_START } from '@kbn/rule-data-utils'; +import { ALERT_END, ALERT_START, ALERT_EVALUATION_VALUES } from '@kbn/rule-data-utils'; import { Rule } from '@kbn/alerting-plugin/common'; import { AlertAnnotation, getPaddedAlertTimeRange, AlertActiveTimeRangeAnnotation, } from '@kbn/observability-alert-details'; +import { Threshold } from '../../common/components/threshold'; import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source'; import { generateUniqueKey } from '../lib/generate_unique_key'; import { MetricsExplorerChartType } from '../../../pages/metrics/metrics_explorer/hooks/use_metrics_explorer_options'; @@ -42,7 +44,7 @@ interface AppSectionProps { } export function AlertDetailsAppSection({ alert, rule }: AppSectionProps) { - const { uiSettings } = useKibanaContextForPlugin().services; + const { uiSettings, charts } = useKibanaContextForPlugin().services; const { source, createDerivedIndexPattern } = useSourceContext(); const { euiTheme } = useEuiTheme(); @@ -50,6 +52,10 @@ export function AlertDetailsAppSection({ alert, rule }: AppSectionProps) { () => createDerivedIndexPattern(), [createDerivedIndexPattern] ); + const chartProps = { + theme: charts.theme.useChartsTheme(), + baseTheme: charts.theme.useChartsBaseTheme(), + }; const timeRange = getPaddedAlertTimeRange(alert.fields[ALERT_START]!, alert.fields[ALERT_END]); const alertEnd = alert.fields[ALERT_END] ? moment(alert.fields[ALERT_END]).valueOf() : undefined; const annotations = [ @@ -71,19 +77,39 @@ export function AlertDetailsAppSection({ alert, rule }: AppSectionProps) { return !!rule.params.criteria ? ( - {rule.params.criteria.map((criterion) => ( + {rule.params.criteria.map((criterion, index) => ( - + + + `${Number(d.toFixed(2))} %`} + title={i18n.translate( + 'xpack.infra.metrics.alertDetailsAppSection.thresholdTitle', + { + defaultMessage: 'Threshold breached', + } + )} + comparator={criterion.comparator} + /> + + + + + ))} From 4eea41f7329db6e4de55b162af289f5f4c536782 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Fri, 21 Apr 2023 12:24:57 +0200 Subject: [PATCH 2/5] Add title and subitle for each criterion --- .../alert_details_app_section.test.tsx.snap | 1 + .../alert_details_app_section.test.tsx | 13 ++++- .../components/alert_details_app_section.tsx | 57 ++++++++++++++++--- .../components/expression_chart.tsx | 54 ++++++++++-------- .../mocks/metric_threshold_rule.ts | 19 +++++-- 5 files changed, 104 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/__snapshots__/alert_details_app_section.test.tsx.snap b/x-pack/plugins/infra/public/alerting/metric_threshold/components/__snapshots__/alert_details_app_section.test.tsx.snap index 9994945cd3290..5ee10d2d3381e 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/__snapshots__/alert_details_app_section.test.tsx.snap +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/__snapshots__/alert_details_app_section.test.tsx.snap @@ -34,6 +34,7 @@ Array [ "groupBy": Array [ "host.hostname", ], + "hideTitle": true, "source": Object { "id": "default", }, diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx index 20f134633e04b..2a5ae84ed7d1b 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx @@ -5,6 +5,7 @@ * 2.0. */ +import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import React from 'react'; import { coreMock as mockCoreMock } from '@kbn/core/public/mocks'; import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; @@ -17,6 +18,8 @@ import { import { AlertDetailsAppSection } from './alert_details_app_section'; import { ExpressionChart } from './expression_chart'; +const mockedChartStartContract = chartPluginMock.createStartContract(); + jest.mock('@kbn/observability-alert-details', () => ({ AlertAnnotation: () => {}, AlertActiveTimeRangeAnnotation: () => {}, @@ -32,7 +35,10 @@ jest.mock('./expression_chart', () => ({ jest.mock('../../../hooks/use_kibana', () => ({ useKibanaContextForPlugin: () => ({ - services: mockCoreMock.createStart(), + services: { + ...mockCoreMock.createStart(), + charts: mockedChartStartContract, + }, }), })); @@ -46,6 +52,7 @@ jest.mock('../../../containers/metrics_source/source', () => ({ describe('AlertDetailsAppSection', () => { const queryClient = new QueryClient(); + const mockedSetAlertSummaryFields = jest.fn(); const renderComponent = () => { return render( @@ -53,16 +60,18 @@ describe('AlertDetailsAppSection', () => { ); }; - it('should render rule data', async () => { + it('should render rule and alert data', async () => { const result = renderComponent(); expect((await result.findByTestId('metricThresholdAppSection')).children.length).toBe(3); + expect(result.getByTestId('threshold-2000-2500')).toBeTruthy(); }); it('should render annotations', async () => { diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx index 62d586d2ac47d..8972fdf44ae35 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx @@ -6,10 +6,19 @@ */ import { i18n } from '@kbn/i18n'; -import React, { useMemo } from 'react'; +import { FormattedMessage } from '@kbn/i18n-react'; +import React, { useEffect, useMemo } from 'react'; import moment from 'moment'; -import { EuiFlexGroup, EuiFlexItem, EuiPanel, useEuiTheme } from '@elastic/eui'; -import { TopAlert } from '@kbn/observability-plugin/public'; +import { + EuiFlexGroup, + EuiFlexItem, + EuiPanel, + EuiSpacer, + EuiText, + EuiTitle, + useEuiTheme, +} from '@elastic/eui'; +import { AlertSummaryField, TopAlert } from '@kbn/observability-plugin/public'; import { ALERT_END, ALERT_START, ALERT_EVALUATION_VALUES } from '@kbn/rule-data-utils'; import { Rule } from '@kbn/alerting-plugin/common'; import { @@ -17,6 +26,7 @@ import { getPaddedAlertTimeRange, AlertActiveTimeRangeAnnotation, } from '@kbn/observability-alert-details'; +import { TIME_LABELS } from '../../common/criterion_preview_chart/criterion_preview_chart'; import { Threshold } from '../../common/components/threshold'; import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source'; import { generateUniqueKey } from '../lib/generate_unique_key'; @@ -41,9 +51,10 @@ const ALERT_TIME_RANGE_ANNOTATION_ID = 'alert_time_range_annotation'; interface AppSectionProps { rule: MetricThresholdRule; alert: MetricThresholdAlert; + setAlertSummaryFields: React.Dispatch>; } -export function AlertDetailsAppSection({ alert, rule }: AppSectionProps) { +export function AlertDetailsAppSection({ alert, rule, setAlertSummaryFields }: AppSectionProps) { const { uiSettings, charts } = useKibanaContextForPlugin().services; const { source, createDerivedIndexPattern } = useSourceContext(); const { euiTheme } = useEuiTheme(); @@ -74,14 +85,41 @@ export function AlertDetailsAppSection({ alert, rule }: AppSectionProps) { key={ALERT_TIME_RANGE_ANNOTATION_ID} />, ]; + useEffect(() => { + setAlertSummaryFields([ + { + label: i18n.translate('xpack.infra.metrics.alertDetailsAppSection.summaryField.rule', { + defaultMessage: 'Rule', + }), + value: rule.name, + }, + ]); + }, [alert, rule, setAlertSummaryFields]); return !!rule.params.criteria ? ( {rule.params.criteria.map((criterion, index) => ( + +

+ {criterion.aggType.toUpperCase()}{' '} + {'metric' in criterion ? criterion.metric : undefined} +

+ + + + + - + diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx index 8dd7762feb6b9..3a2ace2189292 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx @@ -48,23 +48,25 @@ import { CUSTOM_EQUATION } from '../i18n_strings'; interface Props { expression: MetricExpression; derivedIndexPattern: DataViewBase; - source?: MetricsSourceConfiguration; + annotations?: Array>; + chartType?: MetricsExplorerChartType; filterQuery?: string; groupBy?: string | string[]; - chartType?: MetricsExplorerChartType; + hideTitle?: boolean; + source?: MetricsSourceConfiguration; timeRange?: TimeRange; - annotations?: Array>; } export const ExpressionChart: React.FC = ({ expression, derivedIndexPattern, - source, + annotations, + chartType = MetricsExplorerChartType.bar, filterQuery, groupBy, - chartType = MetricsExplorerChartType.bar, + hideTitle = false, + source, timeRange, - annotations, }) => { const { uiSettings } = useKibanaContextForPlugin().services; @@ -187,25 +189,27 @@ export const ExpressionChart: React.FC = ({ -
- {series.id !== 'ALL' ? ( - - - - ) : ( - - - - )} -
+ {!hideTitle && ( +
+ {series.id !== 'ALL' ? ( + + + + ) : ( + + + + )} +
+ )} ); }; diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/mocks/metric_threshold_rule.ts b/x-pack/plugins/infra/public/alerting/metric_threshold/mocks/metric_threshold_rule.ts index 3f579a56ce7a3..812a8864cffd7 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/mocks/metric_threshold_rule.ts +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/mocks/metric_threshold_rule.ts @@ -130,18 +130,29 @@ export const buildMetricThresholdAlert = ( 'kibana.alert.rule.parameters': { criteria: [ { - aggType: 'avg', - comparator: '>', - threshold: [0.1], - timeSize: 1, + aggType: Aggregators.AVERAGE, + comparator: Comparator.GT, + threshold: [2000], + timeSize: 15, timeUnit: 'm', metric: 'system.cpu.user.pct', }, + { + aggType: Aggregators.MAX, + comparator: Comparator.GT, + threshold: [4], + timeSize: 15, + timeUnit: 'm', + metric: 'system.cpu.user.pct', + warningComparator: Comparator.GT, + warningThreshold: [2.2], + }, ], sourceId: 'default', alertOnNoData: true, alertOnGroupDisappear: true, }, + 'kibana.alert.evaluation.values': [2500, 5], 'kibana.alert.rule.category': 'Metric threshold', 'kibana.alert.rule.consumer': 'alerts', 'kibana.alert.rule.execution.uuid': '62dd07ef-ead9-4b1f-a415-7c83d03925f7', From cebda3fcff5938dac9a8d9879525138fdaae09d9 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Fri, 21 Apr 2023 17:25:10 +0200 Subject: [PATCH 3/5] Add metric value formatter --- .../metrics/metric_value_formatter.test.ts | 27 +++++++++++++++++++ .../metrics/metric_value_formatter.ts | 24 +++++++++++++++++ .../components/alert_details_app_section.tsx | 7 +++-- 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.test.ts create mode 100644 x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts diff --git a/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.test.ts b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.test.ts new file mode 100644 index 0000000000000..b6413b37b0380 --- /dev/null +++ b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.test.ts @@ -0,0 +1,27 @@ +/* + * 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 { metricValueFormatter } from './metric_value_formatter'; + +describe('metricValueFormatter', () => { + const testData = [ + { value: null, metric: undefined, result: '[NO DATA]' }, + { value: null, metric: 'system.cpu.user.pct', result: '[NO DATA]' }, + { value: 50, metric: undefined, result: '50' }, + { value: 0.7, metric: 'system.cpu.user.pct', result: '70%' }, + { value: 0.7012345, metric: 'system.cpu.user.pct', result: '70.1%' }, + { value: 208, metric: 'system.cpu.user.ticks', result: '208' }, + { value: 0.8, metric: 'system.cpu.user.ticks', result: '0.8' }, + ]; + + it.each(testData)( + 'metricValueFormatter($value, $metric) = $result', + ({ value, metric, result }) => { + expect(metricValueFormatter(value, metric)).toBe(result); + } + ); +}); diff --git a/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts new file mode 100644 index 0000000000000..27e43c4586d9f --- /dev/null +++ b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts @@ -0,0 +1,24 @@ +/* + * 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 { i18n } from '@kbn/i18n'; +import { createFormatter } from '../../formatters'; + +export const metricValueFormatter = (value: number | null, metric: string = '') => { + const noDataValue = i18n.translate('xpack.infra.metrics.alerting.noDataFormattedValue', { + defaultMessage: '[NO DATA]', + }); + + let formatter; + if (metric.endsWith('.pct')) { + formatter = createFormatter('percent'); + } else { + formatter = createFormatter('highPrecision'); + } + + return value !== null && value !== undefined ? formatter(value) : noDataValue; +}; diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx index 8972fdf44ae35..c77c0d46f693d 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx @@ -26,6 +26,7 @@ import { getPaddedAlertTimeRange, AlertActiveTimeRangeAnnotation, } from '@kbn/observability-alert-details'; +import { metricValueFormatter } from '../../../../common/alerting/metrics/metric_value_formatter'; import { TIME_LABELS } from '../../common/criterion_preview_chart/criterion_preview_chart'; import { Threshold } from '../../common/components/threshold'; import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source'; @@ -119,13 +120,15 @@ export function AlertDetailsAppSection({ alert, rule, setAlertSummaryFields }: A - + `${Number(d.toFixed(2))} %`} + valueFormatter={(d) => + metricValueFormatter(d, 'metric' in criterion ? criterion.metric : undefined) + } title={i18n.translate( 'xpack.infra.metrics.alertDetailsAppSection.thresholdTitle', { From 427cd096fd0ade6c0dd08aefb187d77050188068 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Mon, 24 Apr 2023 12:11:36 +0200 Subject: [PATCH 4/5] Add rule link to the alert details page of metric threshold --- .../alert_details_app_section.test.tsx | 25 ++++++++++++++++++- .../components/alert_details_app_section.tsx | 19 +++++++++++--- .../pages/alert_details/alert_details.tsx | 1 + 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx index 2a5ae84ed7d1b..d73aec96da4d1 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.test.tsx @@ -5,8 +5,9 @@ * 2.0. */ -import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import React from 'react'; +import { EuiLink } from '@elastic/eui'; +import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import { coreMock as mockCoreMock } from '@kbn/core/public/mocks'; import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; import { render } from '@testing-library/react'; @@ -53,6 +54,7 @@ jest.mock('../../../containers/metrics_source/source', () => ({ describe('AlertDetailsAppSection', () => { const queryClient = new QueryClient(); const mockedSetAlertSummaryFields = jest.fn(); + const ruleLink = 'ruleLink'; const renderComponent = () => { return render( @@ -60,6 +62,7 @@ describe('AlertDetailsAppSection', () => { @@ -67,6 +70,10 @@ describe('AlertDetailsAppSection', () => { ); }; + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should render rule and alert data', async () => { const result = renderComponent(); @@ -74,6 +81,22 @@ describe('AlertDetailsAppSection', () => { expect(result.getByTestId('threshold-2000-2500')).toBeTruthy(); }); + it('should render rule link', async () => { + renderComponent(); + + expect(mockedSetAlertSummaryFields).toBeCalledTimes(1); + expect(mockedSetAlertSummaryFields).toBeCalledWith([ + { + label: 'Rule', + value: ( + + Monitoring hosts + + ), + }, + ]); + }); + it('should render annotations', async () => { const mockedExpressionChart = jest.fn(() =>
); (ExpressionChart as jest.Mock).mockImplementation(mockedExpressionChart); diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx index c77c0d46f693d..466d032b5c01f 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_details_app_section.tsx @@ -12,6 +12,7 @@ import moment from 'moment'; import { EuiFlexGroup, EuiFlexItem, + EuiLink, EuiPanel, EuiSpacer, EuiText, @@ -50,12 +51,18 @@ const ALERT_START_ANNOTATION_ID = 'alert_start_annotation'; const ALERT_TIME_RANGE_ANNOTATION_ID = 'alert_time_range_annotation'; interface AppSectionProps { - rule: MetricThresholdRule; alert: MetricThresholdAlert; + rule: MetricThresholdRule; + ruleLink: string; setAlertSummaryFields: React.Dispatch>; } -export function AlertDetailsAppSection({ alert, rule, setAlertSummaryFields }: AppSectionProps) { +export function AlertDetailsAppSection({ + alert, + rule, + ruleLink, + setAlertSummaryFields, +}: AppSectionProps) { const { uiSettings, charts } = useKibanaContextForPlugin().services; const { source, createDerivedIndexPattern } = useSourceContext(); const { euiTheme } = useEuiTheme(); @@ -92,10 +99,14 @@ export function AlertDetailsAppSection({ alert, rule, setAlertSummaryFields }: A label: i18n.translate('xpack.infra.metrics.alertDetailsAppSection.summaryField.rule', { defaultMessage: 'Rule', }), - value: rule.name, + value: ( + + {rule.name} + + ), }, ]); - }, [alert, rule, setAlertSummaryFields]); + }, [alert, rule, ruleLink, setAlertSummaryFields]); return !!rule.params.criteria ? ( diff --git a/x-pack/plugins/observability/public/pages/alert_details/alert_details.tsx b/x-pack/plugins/observability/public/pages/alert_details/alert_details.tsx index d994669fbc6b0..9572c287257de 100644 --- a/x-pack/plugins/observability/public/pages/alert_details/alert_details.tsx +++ b/x-pack/plugins/observability/public/pages/alert_details/alert_details.tsx @@ -130,6 +130,7 @@ export function AlertDetails() { rule={rule} timeZone={timeZone} setAlertSummaryFields={setSummaryFields} + ruleLink={http.basePath.prepend(paths.observability.ruleDetails(rule.id))} /> )} From 8782a80dde7906fb3f4a8683d54c28259910c937 Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Tue, 25 Apr 2023 10:00:03 +0200 Subject: [PATCH 5/5] Refactort metric_value_formatter based on comments --- .../common/alerting/metrics/metric_value_formatter.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts index 27e43c4586d9f..2049a3667d0e5 100644 --- a/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts +++ b/x-pack/plugins/infra/common/alerting/metrics/metric_value_formatter.ts @@ -13,12 +13,9 @@ export const metricValueFormatter = (value: number | null, metric: string = '') defaultMessage: '[NO DATA]', }); - let formatter; - if (metric.endsWith('.pct')) { - formatter = createFormatter('percent'); - } else { - formatter = createFormatter('highPrecision'); - } + const formatter = metric.endsWith('.pct') + ? createFormatter('percent') + : createFormatter('highPrecision'); - return value !== null && value !== undefined ? formatter(value) : noDataValue; + return value == null ? noDataValue : formatter(value); };