From ea12307c67460569590d9af16ac9ccf8dde8bf01 Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Fri, 3 Apr 2020 12:40:16 -0500 Subject: [PATCH 01/25] [Logs UI Alerting] Bootstrap with skeleton of alert typedef and frontend type --- .../components/alerting/logs/alert_flyout.tsx | 47 ++++++++++++++++ .../alerting/logs/log_threshold_alert_type.ts | 29 ++++++++++ .../log_threshold/log_threshold_executor.ts | 56 +++++++++++++++++++ .../register_log_threshold_alert_type.ts | 48 ++++++++++++++++ .../lib/alerting/log_threshold/types.ts | 26 +++++++++ .../lib/alerting/register_alert_types.ts | 3 +- 6 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts create mode 100644 x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts create mode 100644 x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts create mode 100644 x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts diff --git a/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx new file mode 100644 index 0000000000000..37a92e03ef397 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx @@ -0,0 +1,47 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useContext } from 'react'; +import { AlertsContextProvider, AlertAdd } from '../../../../../triggers_actions_ui/public'; +import { TriggerActionsContext } from '../../../utils/triggers_actions_context'; +import { useKibana } from '../../../../../../../src/plugins/kibana_react/public'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { LOG_THRESHOLD_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; + +interface Props { + visible?: boolean; + setVisible: React.Dispatch>; +} + +export const AlertFlyout = (props: Props) => { + const { triggersActionsUI } = useContext(TriggerActionsContext); + const { services } = useKibana(); + + return ( + <> + {triggersActionsUI && ( + + + + )} + + ); +}; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts new file mode 100644 index 0000000000000..8354000ca85a8 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts @@ -0,0 +1,29 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { i18n } from '@kbn/i18n'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { AlertTypeModel } from '../../../../../triggers_actions_ui/public/types'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { LOG_THRESHOLD_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; + +export function getAlertType(): AlertTypeModel { + return { + id: LOG_THRESHOLD_ALERT_TYPE_ID, + name: i18n.translate('xpack.infra.logs.alertFlyout.alertName', { + defaultMessage: 'Log threshold', + }), + iconClass: 'bell', + alertParamsExpression: () => null, // React component defining the expression editor + validate: () => ({ errors: {} }), // Function to validate expression and return any errors + defaultActionMessage: i18n.translate( + 'xpack.infra.logs.alerting.threshold.defaultActionMessage', + { + defaultMessage: + 'This should be a default message for a logs alert, including example use of context variables', + } + ), + }; +} diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts new file mode 100644 index 0000000000000..15149dbb329ce --- /dev/null +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { i18n } from '@kbn/i18n'; +import { AlertExecutorOptions } from '../../../../../alerting/server'; +import { AlertStates, Comparator, LogThresholdAlertParams } from './types'; + +const comparatorMap = { + [Comparator.GT]: (a: number, b: number) => a > b, + [Comparator.GT_OR_EQ]: (a: number, b: number) => a >= b, +}; + +export const createLogThresholdExecutor = (alertUUID: string) => + async function({ services, params }: AlertExecutorOptions) { + const alertInstance = services.alertInstanceFactory(alertUUID); + const { threshold, comparator, timeUnit, timeSize } = params as LogThresholdAlertParams; + + const interval = `${timeSize}${timeUnit}`; + + const logCount = 1; // Replace this with a function to count matching logs over the `interval` + const comparisonFunction = comparatorMap[comparator]; + + const shouldAlertFire = comparisonFunction(logCount, threshold); + const isNoData = false; // Set this to true if there is no log data over the specified interval + const isError = false; // Set this to true if Elasticsearch throws an error when fetching logs + + if (shouldAlertFire) { + const sampleOutput = 'This is an example of data to send to an action context'; + + alertInstance.scheduleActions(FIRED_ACTIONS.id, { + sample: sampleOutput, + }); + } + // Future use: ability to fetch and display current alert state + alertInstance.replaceState({ + alertState: isError + ? AlertStates.ERROR + : isNoData + ? AlertStates.NO_DATA + : shouldAlertFire + ? AlertStates.ALERT + : AlertStates.OK, + }); + }; + +// When the Alerting plugin implements support for multiple action groups, add additional +// action groups here to send different messages, e.g. a recovery notification +export const FIRED_ACTIONS = { + id: 'logs.threshold.fired', + name: i18n.translate('xpack.infra.logs.alerting.threshold.fired', { + defaultMessage: 'Fired', + }), +}; diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts new file mode 100644 index 0000000000000..16514a1d30d47 --- /dev/null +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import uuid from 'uuid'; +import { i18n } from '@kbn/i18n'; +import { schema } from '@kbn/config-schema'; +import { PluginSetupContract } from '../../../../../alerting/server'; +import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_executor'; +import { LOG_THRESHOLD_ALERT_TYPE_ID } from './types'; + +const sampleActionVariableDescription = i18n.translate( + 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', + { + defaultMessage: + 'Action variables are whatever values you want to make available to messages that this alert sends. This one would replace {{context.sample}} in an action message.', + } +); + +export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupContract) { + if (!alertingPlugin) { + throw new Error( + 'Cannot register log threshold alert type. Both the actions and alerting plugins need to be enabled.' + ); + } + + const alertUUID = uuid.v4(); + + alertingPlugin.registerType({ + id: LOG_THRESHOLD_ALERT_TYPE_ID, + name: 'Log threshold', + validate: { + params: schema.object({ + threshold: schema.number(), + comparator: schema.oneOf([schema.literal('>'), schema.literal('>=')]), + timeUnit: schema.string(), + timeSize: schema.number(), + }), + }, + defaultActionGroupId: FIRED_ACTIONS.id, + actionGroups: [FIRED_ACTIONS], + executor: createLogThresholdExecutor(alertUUID), + actionVariables: { + context: [{ name: 'sample', description: sampleActionVariableDescription }], + }, + }); +} diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts new file mode 100644 index 0000000000000..70be73e9f317b --- /dev/null +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export const LOG_THRESHOLD_ALERT_TYPE_ID = 'logs.alert.threshold'; + +export enum Comparator { + GT = '>', + GT_OR_EQ = '>=', +} + +export enum AlertStates { + OK, + ALERT, + NO_DATA, + ERROR, +} + +export interface LogThresholdAlertParams { + threshold: number; + comparator: Comparator; + timeUnit: 's' | 'm' | 'h' | 'd'; + timeSize: number; +} diff --git a/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts b/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts index 6ec6f31256b78..7c74a5d74e802 100644 --- a/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts @@ -6,10 +6,11 @@ import { PluginSetupContract } from '../../../../alerting/server'; import { registerMetricThresholdAlertType } from './metric_threshold/register_metric_threshold_alert_type'; +import { registerLogThresholdAlertType } from './log_threshold/register_log_threshold_alert_type'; const registerAlertTypes = (alertingPlugin: PluginSetupContract) => { if (alertingPlugin) { - const registerFns = [registerMetricThresholdAlertType]; + const registerFns = [registerMetricThresholdAlertType, registerLogThresholdAlertType]; registerFns.forEach(fn => { fn(alertingPlugin); From 8a68de37efc04d30194ba13cd52767ce0cbc7b16 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Tue, 7 Apr 2020 15:31:04 +0100 Subject: [PATCH 02/25] Amend schema and add alert dropdown menu --- .../alerting/logs/alert_dropdown.tsx | 62 +++++++++++++++++++ .../infra/public/pages/logs/page_content.tsx | 23 ++++--- .../register_log_threshold_alert_type.ts | 43 ++++++++----- 3 files changed, 107 insertions(+), 21 deletions(-) create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx diff --git a/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx b/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx new file mode 100644 index 0000000000000..ba5fb09e9de2d --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx @@ -0,0 +1,62 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useState, useCallback, useMemo } from 'react'; +import { EuiPopover, EuiButtonEmpty, EuiContextMenuItem, EuiContextMenuPanel } from '@elastic/eui'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { AlertFlyout } from './alert_flyout'; +import { useLinkProps } from '../../../hooks/use_link_props'; + +export const AlertDropdown = () => { + const [popoverOpen, setPopoverOpen] = useState(false); + const [flyoutVisible, setFlyoutVisible] = useState(false); + const manageAlertsLinkProps = useLinkProps({ + app: 'kibana', + hash: 'management/kibana/triggersActions/alerts', + }); + + const closePopover = useCallback(() => { + setPopoverOpen(false); + }, [setPopoverOpen]); + + const openPopover = useCallback(() => { + setPopoverOpen(true); + }, [setPopoverOpen]); + + const menuItems = useMemo(() => { + return [ + setFlyoutVisible(true)}> + + , + + + , + ]; + }, [manageAlertsLinkProps]); + + return ( + <> + + + + } + isOpen={popoverOpen} + closePopover={closePopover} + > + + + + + ); +}; diff --git a/x-pack/plugins/infra/public/pages/logs/page_content.tsx b/x-pack/plugins/infra/public/pages/logs/page_content.tsx index ed6f06deeef64..dc210406275d8 100644 --- a/x-pack/plugins/infra/public/pages/logs/page_content.tsx +++ b/x-pack/plugins/infra/public/pages/logs/page_content.tsx @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import { Route, Switch } from 'react-router-dom'; @@ -21,6 +22,7 @@ import { LogEntryCategoriesPage } from './log_entry_categories'; import { LogEntryRatePage } from './log_entry_rate'; import { LogsSettingsPage } from './settings'; import { StreamPage } from './stream'; +import { AlertDropdown } from '../../components/alerting/logs/alert_dropdown'; export const LogsPageContent: React.FunctionComponent = () => { const uiCapabilities = useKibana().services.application?.capabilities; @@ -65,13 +67,20 @@ export const LogsPageContent: React.FunctionComponent = () => { readOnlyBadge={!uiCapabilities?.logs?.save} /> - + + + + + + + + diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts index 16514a1d30d47..2a97dd54f8a3c 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -10,13 +10,31 @@ import { PluginSetupContract } from '../../../../../alerting/server'; import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_executor'; import { LOG_THRESHOLD_ALERT_TYPE_ID } from './types'; -const sampleActionVariableDescription = i18n.translate( - 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', - { - defaultMessage: - 'Action variables are whatever values you want to make available to messages that this alert sends. This one would replace {{context.sample}} in an action message.', - } -); +// const sampleActionVariableDescription = i18n.translate( +// 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', +// { +// defaultMessage: +// 'Action variables are whatever values you want to make available to messages that this alert sends. This one would replace in an action message.', +// } +// ); + +const criteriaSchema = schema.object({ + threshold: schema.oneOf([schema.number(), schema.string()]), + comparator: schema.oneOf([ + schema.literal('>'), + schema.literal('<'), + schema.literal('>='), + schema.literal('<='), + schema.literal('equals'), + schema.literal('does not equal'), + schema.literal('matches'), + schema.literal('does not match'), + ]), + timeUnit: schema.string(), + timeSize: schema.number(), + field: schema.string(), + aggType: schema.oneOf([schema.literal('documentCount')]), +}); export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupContract) { if (!alertingPlugin) { @@ -32,17 +50,14 @@ export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupC name: 'Log threshold', validate: { params: schema.object({ - threshold: schema.number(), - comparator: schema.oneOf([schema.literal('>'), schema.literal('>=')]), - timeUnit: schema.string(), - timeSize: schema.number(), + criteria: schema.arrayOf(criteriaSchema), }), }, defaultActionGroupId: FIRED_ACTIONS.id, actionGroups: [FIRED_ACTIONS], executor: createLogThresholdExecutor(alertUUID), - actionVariables: { - context: [{ name: 'sample', description: sampleActionVariableDescription }], - }, + // actionVariables: { + // context: [{ name: 'sample', description: sampleActionVariableDescription }], + // }, }); } From a2b4fae804375d948b6fdbd73d95d355166c6014 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Tue, 7 Apr 2020 15:41:02 +0100 Subject: [PATCH 03/25] Register public side of AlertType --- x-pack/plugins/infra/public/plugin.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/infra/public/plugin.ts b/x-pack/plugins/infra/public/plugin.ts index 3b6647b9bfbbe..40366b2a54f24 100644 --- a/x-pack/plugins/infra/public/plugin.ts +++ b/x-pack/plugins/infra/public/plugin.ts @@ -21,7 +21,8 @@ import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/p import { DataEnhancedSetup, DataEnhancedStart } from '../../data_enhanced/public'; import { TriggersAndActionsUIPublicPluginSetup } from '../../../plugins/triggers_actions_ui/public'; -import { getAlertType } from './components/alerting/metrics/metric_threshold_alert_type'; +import { getAlertType as getMetricsAlertType } from './components/alerting/metrics/metric_threshold_alert_type'; +import { getAlertType as getLogsAlertType } from './components/alerting/logs/log_threshold_alert_type'; export type ClientSetup = void; export type ClientStart = void; @@ -52,7 +53,8 @@ export class Plugin setup(core: CoreSetup, pluginsSetup: ClientPluginsSetup) { registerFeatures(pluginsSetup.home); - pluginsSetup.triggers_actions_ui.alertTypeRegistry.register(getAlertType()); + pluginsSetup.triggers_actions_ui.alertTypeRegistry.register(getMetricsAlertType()); + pluginsSetup.triggers_actions_ui.alertTypeRegistry.register(getLogsAlertType()); core.application.register({ id: 'logs', From 4af847086c499f107714add59f7e9bd7ab86ce39 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Wed, 8 Apr 2020 16:22:15 +0100 Subject: [PATCH 04/25] Document count portion of expression editor --- .../components/alerting/logs/alert_flyout.tsx | 4 +- .../logs/expression_editor/document_count.tsx | 98 +++++++++++++++++ .../logs/expression_editor/editor.tsx | 101 ++++++++++++++++++ .../alerting/logs/expression_editor/index.tsx | 7 ++ .../alerting/logs/log_threshold_alert_type.ts | 7 +- .../register_log_threshold_alert_type.ts | 41 ++++--- .../lib/alerting/log_threshold/types.ts | 14 ++- 7 files changed, 249 insertions(+), 23 deletions(-) create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/expression_editor/index.tsx diff --git a/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx index 37a92e03ef397..099cc3d1e6dd9 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx @@ -9,7 +9,7 @@ import { AlertsContextProvider, AlertAdd } from '../../../../../triggers_actions import { TriggerActionsContext } from '../../../utils/triggers_actions_context'; import { useKibana } from '../../../../../../../src/plugins/kibana_react/public'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { LOG_THRESHOLD_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; +import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; interface Props { visible?: boolean; @@ -36,7 +36,7 @@ export const AlertFlyout = (props: Props) => { diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx new file mode 100644 index 0000000000000..233e1fe2bbca9 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useState } from 'react'; +import { + EuiPopoverTitle, + EuiFlexItem, + EuiFlexGroup, + EuiPopover, + EuiSelect, + EuiFieldNumber, + EuiExpression, +} from '@elastic/eui'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; + +const getComparatorOptions = () => { + return [ + { value: Comparator.LT, text: Comparator.LT }, + { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, + { value: Comparator.GT, text: Comparator.GT }, + { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, + ]; +}; + +export const DocumentCount: React.FC = ({ comparator, value, updateCount }) => { + const [isComparatorPopoverOpen, setComparatorPopoverOpenState] = useState(false); + const [isValuePopoverOpen, setIsValuePopoverOpen] = useState(false); + + return ( + + + setComparatorPopoverOpenState(true)} + /> + } + isOpen={isComparatorPopoverOpen} + closePopover={() => setComparatorPopoverOpenState(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ WHEN + updateCount({ comparator: e.target.value })} + options={getComparatorOptions()} + /> +
+ + + + + setIsValuePopoverOpen(true)} + /> + } + isOpen={isValuePopoverOpen} + closePopover={() => setIsValuePopoverOpen(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ LOG ENTRIES + { + const number = parseInt(e.target.value, 10); + updateCount({ value: number ? number : undefined }); + }} + /> +
+ + + + ); +}; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx new file mode 100644 index 0000000000000..7184f199bded1 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -0,0 +1,101 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useCallback, useMemo, useEffect, useState } from 'react'; +import { + EuiFlexGroup, + EuiFlexItem, + EuiButtonIcon, + EuiSpacer, + EuiText, + EuiFormRow, + EuiButtonEmpty, +} from '@elastic/eui'; +import { IFieldType } from 'src/plugins/data/public'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { i18n } from '@kbn/i18n'; +import { euiStyled } from '../../../../../../observability/public'; +import { + ForLastExpression, + // eslint-disable-next-line @kbn/eslint/no-restricted-paths +} from '../../../../../../triggers_actions_ui/public/common'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { useSource } from '../../../../containers/source'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Comparator, TimeUnit } from '../../../../../server/lib/alerting/log_threshold/types'; +import { DocumentCount } from './document_count'; + +export interface LogsDocumentCountExpression { + count?: { + value?: number; + comparator?: Comparator; + }; + timeSize?: number; + timeUnit?: TimeUnit; + criteria?: ExpressionCriteria[]; +} + +export interface ExpressionCriteria { + field?: string; + comparator?: Comparator; + value?: string | number; +} + +interface Props { + errors: IErrorObject[]; + alertParams: LogsDocumentCountExpression; + setAlertParams(key: string, value: any): void; + setAlertProperty(key: string, value: any): void; +} + +export const ExpressionEditor: React.FC = props => { + const { setAlertParams, alertParams, errors } = props; + const { source, createDerivedIndexPattern } = useSource({ sourceId: 'default' }); + const [timeSize, setTimeSize] = useState(1); + const [timeUnit, setTimeUnit] = useState('m'); + const derivedIndexPattern = useMemo(() => createDerivedIndexPattern('logs'), [ + createDerivedIndexPattern, + ]); + + const defaultExpression = useMemo(() => { + return { + count: { + value: 75, + comparator: Comparator.GT, + }, + criteria: [{ field: null, comparator: Comparator.EQ, value: null }], + timeSize: 5, + timeUnit: 'm', + }; + }, []); + + // Set the default expression + useEffect(() => { + for (const [key, value] of Object.entries(defaultExpression)) { + setAlertParams(key, value); + } + }, []); // eslint-disable-line react-hooks/exhaustive-deps + + const updateCount = useCallback( + countParams => { + const nextCountParams = { ...alertParams.count, ...countParams }; + setAlertParams('count', nextCountParams); + }, + [alertParams.count, setAlertParams] + ); + + return ( + <> + + + ); +}; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/index.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/index.tsx new file mode 100644 index 0000000000000..8b0fd5eb721b3 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/index.tsx @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export * from './editor'; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts index 8354000ca85a8..35d41185e4f24 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts @@ -7,16 +7,17 @@ import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { AlertTypeModel } from '../../../../../triggers_actions_ui/public/types'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { LOG_THRESHOLD_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; +import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; +import { ExpressionEditor } from './expression_editor'; export function getAlertType(): AlertTypeModel { return { - id: LOG_THRESHOLD_ALERT_TYPE_ID, + id: LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, name: i18n.translate('xpack.infra.logs.alertFlyout.alertName', { defaultMessage: 'Log threshold', }), iconClass: 'bell', - alertParamsExpression: () => null, // React component defining the expression editor + alertParamsExpression: ExpressionEditor, validate: () => ({ errors: {} }), // Function to validate expression and return any errors defaultActionMessage: i18n.translate( 'xpack.infra.logs.alerting.threshold.defaultActionMessage', diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts index 2a97dd54f8a3c..dc2175c1118e0 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; import { PluginSetupContract } from '../../../../../alerting/server'; import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_executor'; -import { LOG_THRESHOLD_ALERT_TYPE_ID } from './types'; +import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, Comparator } from './types'; // const sampleActionVariableDescription = i18n.translate( // 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', @@ -18,22 +18,30 @@ import { LOG_THRESHOLD_ALERT_TYPE_ID } from './types'; // } // ); -const criteriaSchema = schema.object({ - threshold: schema.oneOf([schema.number(), schema.string()]), +const countSchema = schema.object({ + value: schema.number(), comparator: schema.oneOf([ - schema.literal('>'), - schema.literal('<'), - schema.literal('>='), - schema.literal('<='), - schema.literal('equals'), - schema.literal('does not equal'), - schema.literal('matches'), - schema.literal('does not match'), + schema.literal(Comparator.GT), + schema.literal(Comparator.LT), + schema.literal(Comparator.GT_OR_EQ), + schema.literal(Comparator.LT_OR_EQ), + schema.literal(Comparator.EQ), ]), - timeUnit: schema.string(), - timeSize: schema.number(), +}); + +const criteriaSchema = schema.object({ field: schema.string(), - aggType: schema.oneOf([schema.literal('documentCount')]), + comparator: schema.oneOf([ + schema.literal(Comparator.GT), + schema.literal(Comparator.LT), + schema.literal(Comparator.GT_OR_EQ), + schema.literal(Comparator.LT_OR_EQ), + schema.literal(Comparator.EQ), + schema.literal(Comparator.NOT_EQ), + schema.literal(Comparator.MATCH), + schema.literal(Comparator.NOT_MATCH), + ]), + value: schema.oneOf([schema.number(), schema.string()]), }); export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupContract) { @@ -46,11 +54,14 @@ export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupC const alertUUID = uuid.v4(); alertingPlugin.registerType({ - id: LOG_THRESHOLD_ALERT_TYPE_ID, + id: LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, name: 'Log threshold', validate: { params: schema.object({ + count: countSchema, criteria: schema.arrayOf(criteriaSchema), + timeUnit: schema.string(), + timeSize: schema.number(), }), }, defaultActionGroupId: FIRED_ACTIONS.id, diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts index 70be73e9f317b..e4c2a08acc65e 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts @@ -4,11 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -export const LOG_THRESHOLD_ALERT_TYPE_ID = 'logs.alert.threshold'; +export const LOG_DOCUMENT_COUNT_ALERT_TYPE_ID = 'logs.alert.document.count'; export enum Comparator { - GT = '>', - GT_OR_EQ = '>=', + GT = 'more than', + GT_OR_EQ = 'more than or equals', + LT = 'less than', + LT_OR_EQ = 'less than or equals', + EQ = 'equals', + NOT_EQ = 'does not equal', + MATCH = 'matches', + NOT_MATCH = 'does not match', } export enum AlertStates { @@ -24,3 +30,5 @@ export interface LogThresholdAlertParams { timeUnit: 's' | 'm' | 'h' | 'd'; timeSize: number; } + +export type TimeUnit = 's' | 'm' | 'h' | 'd'; From 295c223df61a7f72574b5eb62309ce62f8df7b29 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Wed, 8 Apr 2020 16:50:01 +0100 Subject: [PATCH 05/25] Start of criteria --- .../alerting/logs/expression_editor/editor.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 7184f199bded1..cba01313c48a5 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -29,6 +29,7 @@ import { useSource } from '../../../../containers/source'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Comparator, TimeUnit } from '../../../../../server/lib/alerting/log_threshold/types'; import { DocumentCount } from './document_count'; +import { Criteria } from './criteria'; export interface LogsDocumentCountExpression { count?: { @@ -68,13 +69,13 @@ export const ExpressionEditor: React.FC = props => { value: 75, comparator: Comparator.GT, }, - criteria: [{ field: null, comparator: Comparator.EQ, value: null }], + criteria: [{ field: undefined, comparator: Comparator.EQ, value: undefined }], timeSize: 5, timeUnit: 'm', }; }, []); - // Set the default expression + // Set the default expression (disables exhaustive-deps due to only wanting to run this once on mount) useEffect(() => { for (const [key, value] of Object.entries(defaultExpression)) { setAlertParams(key, value); @@ -89,6 +90,10 @@ export const ExpressionEditor: React.FC = props => { [alertParams.count, setAlertParams] ); + const updateCriteria = useCallback(() => { + // Update things here + }, []); + return ( <> = props => { value={alertParams.count?.value} updateCount={updateCount} /> + + ); }; From 2bd61dcc80b77a5297e7fc987010193c6efaabd9 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 9 Apr 2020 14:12:38 +0100 Subject: [PATCH 06/25] (Most of) criteria selection --- .../logs/expression_editor/criteria.tsx | 40 +++++ .../logs/expression_editor/criterion.tsx | 140 ++++++++++++++++++ .../logs/expression_editor/editor.tsx | 36 ++++- .../lib/alerting/log_threshold/types.ts | 2 + 4 files changed, 212 insertions(+), 6 deletions(-) create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx new file mode 100644 index 0000000000000..c016989cbeb1e --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useState } from 'react'; +import { + EuiPopoverTitle, + EuiFlexItem, + EuiFlexGroup, + EuiPopover, + EuiSelect, + EuiFieldNumber, + EuiExpression, +} from '@elastic/eui'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; +import { Criterion } from './criterion'; + +export const Criteria: React.FC = ({ fields, criteria, updateCriteria }) => { + if (!criteria) return null; + return ( + + + {criteria.map((criterion, idx) => { + return ( + + ); + })} + + + ); +}; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx new file mode 100644 index 0000000000000..a9b9e60bb4e34 --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -0,0 +1,140 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useState, useMemo } from 'react'; +import { + EuiPopoverTitle, + EuiFlexItem, + EuiFlexGroup, + EuiPopover, + EuiSelect, + EuiFieldNumber, + EuiExpression, + EuiFieldText, +} from '@elastic/eui'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; + +export const Criterion: React.FC = ({ idx, fields, criterion, updateCriteria }) => { + const [isFieldPopoverOpen, setIsFieldPopoverOpen] = useState(false); + const [isComparatorPopoverOpen, setIsComparatorPopoverOpen] = useState(false); + + const fieldOptions = useMemo(() => { + return fields.map(field => { + return { value: field.name, text: field.name }; + }); + }, [fields]); + + const fieldInfo = useMemo(() => { + return fields.find(field => { + return field.name === criterion.field; + }); + }, [fields, criterion]); + + const compatibleComparatorOptions = useMemo(() => { + if (fieldInfo.type === 'number') { + return [ + { value: Comparator.GT, text: Comparator.GT }, + { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, + { value: Comparator.LT, text: Comparator.LT }, + { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, + { value: Comparator.EQ, text: Comparator.EQ }, + { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + ]; + } else if (fieldInfo.aggregatable) { + return [ + { value: Comparator.EQ, text: Comparator.EQ }, + { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + ]; + } else { + return [ + { value: Comparator.MATCH, text: Comparator.MATCH }, + { value: Comparator.NOT_MATCH, text: Comparator.NOT_MATCH }, + { value: Comparator.MATCH_PHRASE, text: Comparator.MATCH_PHRASE }, + { value: Comparator.NOT_MATCH_PHRASE, text: Comparator.NOT_MATCH_PHRASE }, + ]; + } + }, [fieldInfo]); + + return ( + + + setIsFieldPopoverOpen(true)} + /> + } + isOpen={isFieldPopoverOpen} + closePopover={() => setIsFieldPopoverOpen(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ Field + updateCriteria(idx, { field: e.target.value })} + options={fieldOptions} + /> +
+ + + + setIsComparatorPopoverOpen(true)} + /> + } + isOpen={isComparatorPopoverOpen} + closePopover={() => setIsComparatorPopoverOpen(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ Comparison : Value + updateCriteria(idx, { comparator: e.target.value })} + options={compatibleComparatorOptions} + /> + {fieldInfo.type === 'number' ? ( + { + const number = parseInt(e.target.value, 10); + updateCriteria(idx, { value: number ? number : undefined }); + }} + /> + ) : ( + updateCriteria(idx, { value: e.target.value })} + /> + )} +
+ + + + ); +}; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index cba01313c48a5..4072bb1114483 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -17,6 +17,7 @@ import { import { IFieldType } from 'src/plugins/data/public'; import { FormattedMessage } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; +import { uniq } from 'lodash'; import { euiStyled } from '../../../../../../observability/public'; import { ForLastExpression, @@ -63,19 +64,29 @@ export const ExpressionEditor: React.FC = props => { createDerivedIndexPattern, ]); + const supportedFields = useMemo(() => { + if (derivedIndexPattern?.fields) { + return derivedIndexPattern.fields.filter(field => { + return (field.type === 'string' || field.type === 'number') && field.searchable; + }); + } else { + return []; + } + }, [derivedIndexPattern]); + const defaultExpression = useMemo(() => { return { count: { value: 75, comparator: Comparator.GT, }, - criteria: [{ field: undefined, comparator: Comparator.EQ, value: undefined }], + criteria: [{ field: 'log.level', comparator: Comparator.EQ, value: 'error' }], timeSize: 5, timeUnit: 'm', }; }, []); - // Set the default expression (disables exhaustive-deps due to only wanting to run this once on mount) + // Set the default expression (disables exhaustive-deps as we only want to run this once on mount) useEffect(() => { for (const [key, value] of Object.entries(defaultExpression)) { setAlertParams(key, value); @@ -90,9 +101,18 @@ export const ExpressionEditor: React.FC = props => { [alertParams.count, setAlertParams] ); - const updateCriteria = useCallback(() => { - // Update things here - }, []); + const updateCriteria = useCallback( + (idx, criterionParams) => { + const nextCriteria = alertParams.criteria?.map((criterion, index) => { + return idx === index ? { ...criterion, ...criterionParams } : criterion; + }); + setAlertParams('criteria', nextCriteria ? nextCriteria : []); + }, + [alertParams, setAlertParams] + ); + + // Wait until field info has loaded + if (supportedFields.length === 0) return null; return ( <> @@ -102,7 +122,11 @@ export const ExpressionEditor: React.FC = props => { updateCount={updateCount} /> - + ); }; diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts index e4c2a08acc65e..275b9320c92fc 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts @@ -15,6 +15,8 @@ export enum Comparator { NOT_EQ = 'does not equal', MATCH = 'matches', NOT_MATCH = 'does not match', + MATCH_PHRASE = 'matches phrase', + NOT_MATCH_PHRASE = 'does not match phrase', } export enum AlertStates { From e906ff69276c72a90ce00323198c804e1146ff55 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 9 Apr 2020 14:27:37 +0100 Subject: [PATCH 07/25] Add "for the last" functionality --- .../logs/expression_editor/criteria.tsx | 13 ++------ .../logs/expression_editor/editor.tsx | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx index c016989cbeb1e..1c50a66853360 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -4,18 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useState } from 'react'; -import { - EuiPopoverTitle, - EuiFlexItem, - EuiFlexGroup, - EuiPopover, - EuiSelect, - EuiFieldNumber, - EuiExpression, -} from '@elastic/eui'; +import React from 'react'; +import { EuiFlexItem, EuiFlexGroup } from '@elastic/eui'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; import { Criterion } from './criterion'; export const Criteria: React.FC = ({ fields, criteria, updateCriteria }) => { diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 4072bb1114483..c8db886729dfa 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -111,6 +111,29 @@ export const ExpressionEditor: React.FC = props => { [alertParams, setAlertParams] ); + const updateTimeSize = useCallback( + (ts: number | undefined) => { + setTimeSize(ts || undefined); + setAlertParams('timeSize', ts); + }, + [setTimeSize, setAlertParams] + ); + + const updateTimeUnit = useCallback( + (tu: string) => { + setTimeUnit(tu as TimeUnit); + setAlertParams('timeUnit', tu); + }, + [setAlertParams] + ); + + const emptyError = useMemo(() => { + return { + timeSizeUnit: [], + timeWindowSize: [], + }; + }, []); + // Wait until field info has loaded if (supportedFields.length === 0) return null; @@ -127,6 +150,14 @@ export const ExpressionEditor: React.FC = props => { criteria={alertParams.criteria} updateCriteria={updateCriteria} /> + + ); }; From c18a7e1e84fb7a406e240b302461ba6e517df395 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 9 Apr 2020 14:37:54 +0100 Subject: [PATCH 08/25] Add ability to add more criteria --- .../logs/expression_editor/editor.tsx | 48 ++++++++++++++----- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index c8db886729dfa..34b4468a9104d 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -55,6 +55,18 @@ interface Props { setAlertProperty(key: string, value: any): void; } +const DEFAULT_CRITERIA = { field: 'log.level', comparator: Comparator.EQ, value: 'error' }; + +const DEFAULT_EXPRESSION = { + count: { + value: 75, + comparator: Comparator.GT, + }, + criteria: [DEFAULT_CRITERIA], + timeSize: 5, + timeUnit: 'm', +}; + export const ExpressionEditor: React.FC = props => { const { setAlertParams, alertParams, errors } = props; const { source, createDerivedIndexPattern } = useSource({ sourceId: 'default' }); @@ -74,21 +86,9 @@ export const ExpressionEditor: React.FC = props => { } }, [derivedIndexPattern]); - const defaultExpression = useMemo(() => { - return { - count: { - value: 75, - comparator: Comparator.GT, - }, - criteria: [{ field: 'log.level', comparator: Comparator.EQ, value: 'error' }], - timeSize: 5, - timeUnit: 'm', - }; - }, []); - // Set the default expression (disables exhaustive-deps as we only want to run this once on mount) useEffect(() => { - for (const [key, value] of Object.entries(defaultExpression)) { + for (const [key, value] of Object.entries(DEFAULT_EXPRESSION)) { setAlertParams(key, value); } }, []); // eslint-disable-line react-hooks/exhaustive-deps @@ -127,6 +127,13 @@ export const ExpressionEditor: React.FC = props => { [setAlertParams] ); + const addCriteria = useCallback(() => { + const nextCriteria = alertParams?.criteria + ? [...alertParams.criteria, DEFAULT_CRITERIA] + : [DEFAULT_CRITERIA]; + setAlertParams('criteria', nextCriteria); + }, [alertParams, setAlertParams]); + const emptyError = useMemo(() => { return { timeSizeUnit: [], @@ -158,6 +165,21 @@ export const ExpressionEditor: React.FC = props => { onChangeWindowSize={updateTimeSize} onChangeWindowUnit={updateTimeUnit} /> + +
+ + + +
); }; From 7f6e9f76879b331c33e471e1f6fcd0310a96e748 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Wed, 15 Apr 2020 11:28:34 +0100 Subject: [PATCH 09/25] Add ability to remove criterion --- .../logs/expression_editor/criteria.tsx | 6 ++-- .../logs/expression_editor/criterion.tsx | 31 ++++++++++++++++--- .../logs/expression_editor/editor.tsx | 21 ++++++++++--- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx index 1c50a66853360..a05d7a175e6ac 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -9,7 +9,7 @@ import { EuiFlexItem, EuiFlexGroup } from '@elastic/eui'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Criterion } from './criterion'; -export const Criteria: React.FC = ({ fields, criteria, updateCriteria }) => { +export const Criteria: React.FC = ({ fields, criteria, updateCriterion, removeCriterion }) => { if (!criteria) return null; return ( @@ -21,7 +21,9 @@ export const Criteria: React.FC = ({ fields, criteria, updateCriteria }) => { idx={idx} fields={fields} criterion={criterion} - updateCriteria={updateCriteria} + updateCriterion={updateCriterion} + removeCriterion={removeCriterion} + canDelete={criteria.length > 1} /> ); })} diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index a9b9e60bb4e34..1093dc729426f 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -14,11 +14,20 @@ import { EuiFieldNumber, EuiExpression, EuiFieldText, + EuiButtonIcon, } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; -export const Criterion: React.FC = ({ idx, fields, criterion, updateCriteria }) => { +export const Criterion: React.FC = ({ + idx, + fields, + criterion, + updateCriterion, + removeCriterion, + canDelete, +}) => { const [isFieldPopoverOpen, setIsFieldPopoverOpen] = useState(false); const [isComparatorPopoverOpen, setIsComparatorPopoverOpen] = useState(false); @@ -84,7 +93,7 @@ export const Criterion: React.FC = ({ idx, fields, criterion, updateCriteria }) updateCriteria(idx, { field: e.target.value })} + onChange={e => updateCriterion(idx, { field: e.target.value })} options={fieldOptions} /> @@ -113,7 +122,7 @@ export const Criterion: React.FC = ({ idx, fields, criterion, updateCriteria }) updateCriteria(idx, { comparator: e.target.value })} + onChange={e => updateCriterion(idx, { comparator: e.target.value })} options={compatibleComparatorOptions} /> {fieldInfo.type === 'number' ? ( @@ -122,19 +131,31 @@ export const Criterion: React.FC = ({ idx, fields, criterion, updateCriteria }) value={criterion.value} onChange={e => { const number = parseInt(e.target.value, 10); - updateCriteria(idx, { value: number ? number : undefined }); + updateCriterion(idx, { value: number ? number : undefined }); }} /> ) : ( updateCriteria(idx, { value: e.target.value })} + onChange={e => updateCriterion(idx, { value: e.target.value })} /> )} + {canDelete && ( + + removeCriterion(idx)} + /> + + )} ); }; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 34b4468a9104d..672a93c428822 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -101,7 +101,7 @@ export const ExpressionEditor: React.FC = props => { [alertParams.count, setAlertParams] ); - const updateCriteria = useCallback( + const updateCriterion = useCallback( (idx, criterionParams) => { const nextCriteria = alertParams.criteria?.map((criterion, index) => { return idx === index ? { ...criterion, ...criterionParams } : criterion; @@ -127,13 +127,23 @@ export const ExpressionEditor: React.FC = props => { [setAlertParams] ); - const addCriteria = useCallback(() => { + const addCriterion = useCallback(() => { const nextCriteria = alertParams?.criteria ? [...alertParams.criteria, DEFAULT_CRITERIA] : [DEFAULT_CRITERIA]; setAlertParams('criteria', nextCriteria); }, [alertParams, setAlertParams]); + const removeCriterion = useCallback( + idx => { + const nextCriteria = alertParams?.criteria.filter((criterion, index) => { + return index !== idx; + }); + setAlertParams('criteria', nextCriteria); + }, + [alertParams, setAlertParams] + ); + const emptyError = useMemo(() => { return { timeSizeUnit: [], @@ -155,7 +165,8 @@ export const ExpressionEditor: React.FC = props => { = props => { iconSide={'left'} flush={'left'} iconType={'plusInCircleFilled'} - onClick={addCriteria} + onClick={addCriterion} > From cc14109e1e7050ad2a164b3ce19cde582a15dfe0 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Wed, 15 Apr 2020 16:54:28 +0100 Subject: [PATCH 10/25] Add bulk of UI validation logic --- .../logs/expression_editor/criteria.tsx | 9 +- .../logs/expression_editor/criterion.tsx | 64 +++++++----- .../logs/expression_editor/document_count.tsx | 22 +++-- .../logs/expression_editor/editor.tsx | 8 +- .../alerting/logs/log_threshold_alert_type.ts | 3 +- .../components/alerting/logs/validation.ts | 98 +++++++++++++++++++ 6 files changed, 164 insertions(+), 40 deletions(-) create mode 100644 x-pack/plugins/infra/public/components/alerting/logs/validation.ts diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx index a05d7a175e6ac..3954c8f0e3d83 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -9,7 +9,13 @@ import { EuiFlexItem, EuiFlexGroup } from '@elastic/eui'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Criterion } from './criterion'; -export const Criteria: React.FC = ({ fields, criteria, updateCriterion, removeCriterion }) => { +export const Criteria: React.FC = ({ + fields, + criteria, + updateCriterion, + removeCriterion, + errors, +}) => { if (!criteria) return null; return ( @@ -24,6 +30,7 @@ export const Criteria: React.FC = ({ fields, criteria, updateCriterion, removeCr updateCriterion={updateCriterion} removeCriterion={removeCriterion} canDelete={criteria.length > 1} + errors={errors[idx]} /> ); })} diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index 1093dc729426f..c88c073149e67 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -15,6 +15,7 @@ import { EuiExpression, EuiFieldText, EuiButtonIcon, + EuiFormRow, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths @@ -27,6 +28,7 @@ export const Criterion: React.FC = ({ updateCriterion, removeCriterion, canDelete, + errors, }) => { const [isFieldPopoverOpen, setIsFieldPopoverOpen] = useState(false); const [isComparatorPopoverOpen, setIsComparatorPopoverOpen] = useState(false); @@ -79,6 +81,7 @@ export const Criterion: React.FC = ({ uppercase={true} value={criterion.field} isActive={isFieldPopoverOpen} + color={errors.field.length === 0 ? 'secondary' : 'danger'} onClick={() => setIsFieldPopoverOpen(true)} /> } @@ -90,12 +93,14 @@ export const Criterion: React.FC = ({ >
Field - updateCriterion(idx, { field: e.target.value })} - options={fieldOptions} - /> + 0} error={errors.field}> + updateCriterion(idx, { field: e.target.value })} + options={fieldOptions} + /> +
@@ -108,6 +113,9 @@ export const Criterion: React.FC = ({ uppercase={true} value={criterion.value} isActive={isComparatorPopoverOpen} + color={ + errors.comparator.length === 0 && errors.value.length === 0 ? 'secondary' : 'danger' + } onClick={() => setIsComparatorPopoverOpen(true)} /> } @@ -119,28 +127,32 @@ export const Criterion: React.FC = ({ >
Comparison : Value - updateCriterion(idx, { comparator: e.target.value })} - options={compatibleComparatorOptions} - /> - {fieldInfo.type === 'number' ? ( - { - const number = parseInt(e.target.value, 10); - updateCriterion(idx, { value: number ? number : undefined }); - }} - /> - ) : ( - 0} error={errors.comparator}> + updateCriterion(idx, { value: e.target.value })} + value={criterion.comparator} + onChange={e => updateCriterion(idx, { comparator: e.target.value })} + options={compatibleComparatorOptions} /> - )} + + 0} error={errors.value}> + {fieldInfo.type === 'number' ? ( + { + const number = parseInt(e.target.value, 10); + updateCriterion(idx, { value: number ? number : undefined }); + }} + /> + ) : ( + updateCriterion(idx, { value: e.target.value })} + /> + )} +
diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx index 233e1fe2bbca9..23aa9ba9e3ad4 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -13,6 +13,7 @@ import { EuiSelect, EuiFieldNumber, EuiExpression, + EuiFormRow, } from '@elastic/eui'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; @@ -26,7 +27,7 @@ const getComparatorOptions = () => { ]; }; -export const DocumentCount: React.FC = ({ comparator, value, updateCount }) => { +export const DocumentCount: React.FC = ({ comparator, value, updateCount, errors }) => { const [isComparatorPopoverOpen, setComparatorPopoverOpenState] = useState(false); const [isValuePopoverOpen, setIsValuePopoverOpen] = useState(false); @@ -72,6 +73,7 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount }) => { value={'log entries'} isActive={isValuePopoverOpen} onClick={() => setIsValuePopoverOpen(true)} + color={errors.value.length === 0 ? 'secondary' : 'danger'} /> } isOpen={isValuePopoverOpen} @@ -82,14 +84,16 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount }) => { >
LOG ENTRIES - { - const number = parseInt(e.target.value, 10); - updateCount({ value: number ? number : undefined }); - }} - /> + 0} error={errors.value}> + { + const number = parseInt(e.target.value, 10); + updateCount({ value: number ? number : undefined }); + }} + /> +
diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 672a93c428822..4efb91e7aec58 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -49,7 +49,7 @@ export interface ExpressionCriteria { } interface Props { - errors: IErrorObject[]; + errors: IErrorObject; alertParams: LogsDocumentCountExpression; setAlertParams(key: string, value: any): void; setAlertProperty(key: string, value: any): void; @@ -69,7 +69,7 @@ const DEFAULT_EXPRESSION = { export const ExpressionEditor: React.FC = props => { const { setAlertParams, alertParams, errors } = props; - const { source, createDerivedIndexPattern } = useSource({ sourceId: 'default' }); + const { createDerivedIndexPattern } = useSource({ sourceId: 'default' }); const [timeSize, setTimeSize] = useState(1); const [timeUnit, setTimeUnit] = useState('m'); const derivedIndexPattern = useMemo(() => createDerivedIndexPattern('logs'), [ @@ -160,6 +160,7 @@ export const ExpressionEditor: React.FC = props => { comparator={alertParams.count?.comparator} value={alertParams.count?.value} updateCount={updateCount} + errors={errors.count} /> = props => { criteria={alertParams.criteria} updateCriterion={updateCriterion} removeCriterion={removeCriterion} + errors={errors.criteria} />
diff --git a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts index 35d41185e4f24..fcd9643825722 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts @@ -9,6 +9,7 @@ import { AlertTypeModel } from '../../../../../triggers_actions_ui/public/types' // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; import { ExpressionEditor } from './expression_editor'; +import { validateExpression } from './validation'; export function getAlertType(): AlertTypeModel { return { @@ -18,7 +19,7 @@ export function getAlertType(): AlertTypeModel { }), iconClass: 'bell', alertParamsExpression: ExpressionEditor, - validate: () => ({ errors: {} }), // Function to validate expression and return any errors + validate: validateExpression, defaultActionMessage: i18n.translate( 'xpack.infra.logs.alerting.threshold.defaultActionMessage', { diff --git a/x-pack/plugins/infra/public/components/alerting/logs/validation.ts b/x-pack/plugins/infra/public/components/alerting/logs/validation.ts new file mode 100644 index 0000000000000..c9a108b1ce68d --- /dev/null +++ b/x-pack/plugins/infra/public/components/alerting/logs/validation.ts @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { i18n } from '@kbn/i18n'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { ValidationResult } from '../../../../../triggers_actions_ui/public/types'; + +export function validateExpression({ count, criteria, timeSize, timeUnit }: any): ValidationResult { + const validationResult = { errors: {} }; + + // NOTE: In the case of components provided by the Alerting framework the error property names + // must match what they expect. + const errors: { + count: { + value: string[]; + }; + criteria: { + [id: string]: { + field: string[]; + comparator: string[]; + value: string[]; + }; + }; + timeWindowSize: string[]; + timeSizeUnit: string[]; + } = { + count: { + value: [], + }, + criteria: {}, + timeSizeUnit: [], + timeWindowSize: [], + }; + + validationResult.errors = errors; + + if (!count && !criteria && !timeSize && !timeUnit) return validationResult; + + // Document count validation + if (!count?.value) { + errors.count.value.push( + i18n.translate('xpack.infra.logs.alertFlyout.error.documentCountRequired', { + defaultMessage: 'Document count is Required.', + }) + ); + } + + // Time validation + if (!timeSize) { + errors.timeWindowSize.push( + i18n.translate('xpack.infra.logs.alertFlyout.error.timeSizeRequired', { + defaultMessage: 'Time size is Required.', + }) + ); + } + + if (criteria && criteria.length > 0) { + // Criteria validation + criteria.forEach((criterion, idx: number) => { + const id = idx.toString(); + + errors.criteria[id] = { + field: [], + comparator: [], + value: [], + }; + + if (!criterion.field) { + errors.criteria[id].field.push( + i18n.translate('xpack.infra.logs.alertFlyout.error.criterionFieldRequired', { + defaultMessage: 'Field is required.', + }) + ); + } + + if (!criterion.comparator) { + errors.criteria[id].comparator.push( + i18n.translate('xpack.infra.logs.alertFlyout.error.criterionComparatorRequired', { + defaultMessage: 'Comparator is required.', + }) + ); + } + + if (!criterion.value) { + errors.criteria[id].value.push( + i18n.translate('xpack.infra.logs.alertFlyout.error.criterionValueRequired', { + defaultMessage: 'Value is required.', + }) + ); + } + }); + } + + return validationResult; +} From 1a373468b286f544802c6447ed4969b31f6fc16b Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 16 Apr 2020 11:51:41 +0100 Subject: [PATCH 11/25] Ensure compatibility between field and comparator / value --- .../logs/expression_editor/criterion.tsx | 84 +++++++++++++------ 1 file changed, 57 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index c88c073149e67..9d268eed35b45 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useState, useMemo } from 'react'; +import React, { useState, useMemo, useCallback } from 'react'; import { EuiPopoverTitle, EuiFlexItem, @@ -21,6 +21,37 @@ import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; +const getCompatibleComparatorsForField = fieldInfo => { + if (fieldInfo.type === 'number') { + return [ + { value: Comparator.GT, text: Comparator.GT }, + { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, + { value: Comparator.LT, text: Comparator.LT }, + { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, + { value: Comparator.EQ, text: Comparator.EQ }, + { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + ]; + } else if (fieldInfo.aggregatable) { + return [ + { value: Comparator.EQ, text: Comparator.EQ }, + { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + ]; + } else { + return [ + { value: Comparator.MATCH, text: Comparator.MATCH }, + { value: Comparator.NOT_MATCH, text: Comparator.NOT_MATCH }, + { value: Comparator.MATCH_PHRASE, text: Comparator.MATCH_PHRASE }, + { value: Comparator.NOT_MATCH_PHRASE, text: Comparator.NOT_MATCH_PHRASE }, + ]; + } +}; + +const getFieldInfo = (fields, fieldName) => { + return fields.find(field => { + return field.name === fieldName; + }); +}; + export const Criterion: React.FC = ({ idx, fields, @@ -40,36 +71,35 @@ export const Criterion: React.FC = ({ }, [fields]); const fieldInfo = useMemo(() => { - return fields.find(field => { - return field.name === criterion.field; - }); + return getFieldInfo(fields, criterion.field); }, [fields, criterion]); const compatibleComparatorOptions = useMemo(() => { - if (fieldInfo.type === 'number') { - return [ - { value: Comparator.GT, text: Comparator.GT }, - { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, - { value: Comparator.LT, text: Comparator.LT }, - { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, - { value: Comparator.EQ, text: Comparator.EQ }, - { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, - ]; - } else if (fieldInfo.aggregatable) { - return [ - { value: Comparator.EQ, text: Comparator.EQ }, - { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, - ]; - } else { - return [ - { value: Comparator.MATCH, text: Comparator.MATCH }, - { value: Comparator.NOT_MATCH, text: Comparator.NOT_MATCH }, - { value: Comparator.MATCH_PHRASE, text: Comparator.MATCH_PHRASE }, - { value: Comparator.NOT_MATCH_PHRASE, text: Comparator.NOT_MATCH_PHRASE }, - ]; - } + return getCompatibleComparatorsForField(fieldInfo); }, [fieldInfo]); + const handleFieldChange = useCallback( + e => { + const fieldName = e.target.value; + const nextFieldInfo = getFieldInfo(fields, fieldName); + // If the field information we're dealing with has changed, reset the comparator and value. + if ( + fieldInfo.type !== nextFieldInfo.type || + fieldInfo.aggregatable !== nextFieldInfo.aggregatable + ) { + const compatibleComparators = getCompatibleComparatorsForField(nextFieldInfo); + updateCriterion(idx, { + field: fieldName, + comparator: compatibleComparators[0].value, + value: undefined, + }); + } else { + updateCriterion(idx, { field: fieldName }); + } + }, + [fieldInfo.aggregatable, fieldInfo.type, fields, idx, updateCriterion] + ); + return ( @@ -97,7 +127,7 @@ export const Criterion: React.FC = ({ updateCriterion(idx, { field: e.target.value })} + onChange={handleFieldChange} options={fieldOptions} /> From ba482ec7572f52aa1f4e5cd6da4859a8c176c675 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 16 Apr 2020 12:35:01 +0100 Subject: [PATCH 12/25] Change getSourceConfiguration dependencies so we can use it outside of route handler contexts (like the alert executor) --- .../infra/server/graphql/sources/resolvers.ts | 5 ++++- .../infra/server/lib/domains/fields_domain.ts | 2 +- .../log_entries_domain/log_entries_domain.ts | 6 +++--- .../plugins/infra/server/lib/source_status.ts | 12 +++++------ .../infra/server/lib/sources/sources.ts | 20 ++++++++++++------- .../server/routes/inventory_metadata/index.ts | 2 +- .../infra/server/routes/log_entries/item.ts | 5 +++-- .../infra/server/routes/metadata/index.ts | 2 +- .../infra/server/routes/node_details/index.ts | 5 ++++- .../infra/server/routes/snapshot/index.ts | 5 ++++- 10 files changed, 40 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/infra/server/graphql/sources/resolvers.ts b/x-pack/plugins/infra/server/graphql/sources/resolvers.ts index 1fe1431392a38..f880eca933241 100644 --- a/x-pack/plugins/infra/server/graphql/sources/resolvers.ts +++ b/x-pack/plugins/infra/server/graphql/sources/resolvers.ts @@ -93,7 +93,10 @@ export const createSourcesResolvers = ( } => ({ Query: { async source(root, args, { req }) { - const requestedSourceConfiguration = await libs.sources.getSourceConfiguration(req, args.id); + const requestedSourceConfiguration = await libs.sources.getSourceConfiguration( + req.core.savedObjects.client, + args.id + ); return requestedSourceConfiguration; }, diff --git a/x-pack/plugins/infra/server/lib/domains/fields_domain.ts b/x-pack/plugins/infra/server/lib/domains/fields_domain.ts index d2e151ca2c3f5..b6837e5b769a6 100644 --- a/x-pack/plugins/infra/server/lib/domains/fields_domain.ts +++ b/x-pack/plugins/infra/server/lib/domains/fields_domain.ts @@ -21,7 +21,7 @@ export class InfraFieldsDomain { indexType: InfraIndexType ): Promise { const { configuration } = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const includeMetricIndices = [InfraIndexType.ANY, InfraIndexType.METRICS].includes(indexType); diff --git a/x-pack/plugins/infra/server/lib/domains/log_entries_domain/log_entries_domain.ts b/x-pack/plugins/infra/server/lib/domains/log_entries_domain/log_entries_domain.ts index 528b9a69327fa..7f23278b44849 100644 --- a/x-pack/plugins/infra/server/lib/domains/log_entries_domain/log_entries_domain.ts +++ b/x-pack/plugins/infra/server/lib/domains/log_entries_domain/log_entries_domain.ts @@ -113,7 +113,7 @@ export class InfraLogEntriesDomain { params: LogEntriesParams ): Promise { const { configuration } = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); @@ -179,7 +179,7 @@ export class InfraLogEntriesDomain { filterQuery?: LogEntryQuery ): Promise { const { configuration } = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const dateRangeBuckets = await this.adapter.getContainedLogSummaryBuckets( @@ -203,7 +203,7 @@ export class InfraLogEntriesDomain { filterQuery?: LogEntryQuery ): Promise { const { configuration } = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const messageFormattingRules = compileFormattingRules( diff --git a/x-pack/plugins/infra/server/lib/source_status.ts b/x-pack/plugins/infra/server/lib/source_status.ts index 1f0845b6b223f..9bb953845e5a1 100644 --- a/x-pack/plugins/infra/server/lib/source_status.ts +++ b/x-pack/plugins/infra/server/lib/source_status.ts @@ -18,7 +18,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const indexNames = await this.adapter.getIndexNames( @@ -32,7 +32,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const indexNames = await this.adapter.getIndexNames( @@ -46,7 +46,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const hasAlias = await this.adapter.hasAlias( @@ -60,7 +60,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const hasAlias = await this.adapter.hasAlias( @@ -74,7 +74,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const hasIndices = await this.adapter.hasIndices( @@ -88,7 +88,7 @@ export class InfraSourceStatus { sourceId: string ): Promise { const sourceConfiguration = await this.libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const hasIndices = await this.adapter.hasIndices( diff --git a/x-pack/plugins/infra/server/lib/sources/sources.ts b/x-pack/plugins/infra/server/lib/sources/sources.ts index c7ff6c9638204..265cc68dcc916 100644 --- a/x-pack/plugins/infra/server/lib/sources/sources.ts +++ b/x-pack/plugins/infra/server/lib/sources/sources.ts @@ -9,7 +9,7 @@ import { failure } from 'io-ts/lib/PathReporter'; import { identity, constant } from 'fp-ts/lib/function'; import { pipe } from 'fp-ts/lib/pipeable'; import { map, fold } from 'fp-ts/lib/Either'; -import { RequestHandlerContext } from 'src/core/server'; +import { RequestHandlerContext, SavedObjectsClientContract } from 'src/core/server'; import { defaultSourceConfiguration } from './defaults'; import { NotFoundError } from './errors'; import { infraSourceConfigurationSavedObjectType } from './saved_object_mappings'; @@ -35,7 +35,10 @@ export class InfraSources { this.libs = libs; } - public async getSourceConfiguration(requestContext: RequestHandlerContext, sourceId: string) { + public async getSourceConfiguration( + savedObjectsClient: SavedObjectsClientContract, + sourceId: string + ) { const staticDefaultSourceConfiguration = await this.getStaticDefaultSourceConfiguration(); const savedSourceConfiguration = await this.getInternalSourceConfiguration(sourceId) @@ -51,7 +54,7 @@ export class InfraSources { })) .catch(err => err instanceof NotFoundError - ? this.getSavedSourceConfiguration(requestContext, sourceId).then(result => ({ + ? this.getSavedSourceConfiguration(savedObjectsClient, sourceId).then(result => ({ ...result, configuration: mergeSourceConfiguration( staticDefaultSourceConfiguration, @@ -61,7 +64,7 @@ export class InfraSources { : Promise.reject(err) ) .catch(err => - requestContext.core.savedObjects.client.errors.isNotFoundError(err) + savedObjectsClient.errors.isNotFoundError(err) ? Promise.resolve({ id: sourceId, version: undefined, @@ -132,7 +135,10 @@ export class InfraSources { ) { const staticDefaultSourceConfiguration = await this.getStaticDefaultSourceConfiguration(); - const { configuration, version } = await this.getSourceConfiguration(requestContext, sourceId); + const { configuration, version } = await this.getSourceConfiguration( + requestContext.core.savedObjects.client, + sourceId + ); const updatedSourceConfigurationAttributes = mergeSourceConfiguration( configuration, @@ -195,10 +201,10 @@ export class InfraSources { } private async getSavedSourceConfiguration( - requestContext: RequestHandlerContext, + savedObjectsClient: SavedObjectsClientContract, sourceId: string ) { - const savedObject = await requestContext.core.savedObjects.client.get( + const savedObject = await savedObjectsClient.get( infraSourceConfigurationSavedObjectType, sourceId ); diff --git a/x-pack/plugins/infra/server/routes/inventory_metadata/index.ts b/x-pack/plugins/infra/server/routes/inventory_metadata/index.ts index 7e9b7ada28c8e..687e368736a41 100644 --- a/x-pack/plugins/infra/server/routes/inventory_metadata/index.ts +++ b/x-pack/plugins/infra/server/routes/inventory_metadata/index.ts @@ -39,7 +39,7 @@ export const initInventoryMetaRoute = (libs: InfraBackendLibs) => { ); const { configuration } = await libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const awsMetadata = await getCloudMetadata( diff --git a/x-pack/plugins/infra/server/routes/log_entries/item.ts b/x-pack/plugins/infra/server/routes/log_entries/item.ts index 3a6bdaf3804e3..85dba8f598a89 100644 --- a/x-pack/plugins/infra/server/routes/log_entries/item.ts +++ b/x-pack/plugins/infra/server/routes/log_entries/item.ts @@ -37,8 +37,9 @@ export const initLogEntriesItemRoute = ({ framework, sources, logEntries }: Infr ); const { id, sourceId } = payload; - const sourceConfiguration = (await sources.getSourceConfiguration(requestContext, sourceId)) - .configuration; + const sourceConfiguration = ( + await sources.getSourceConfiguration(requestContext.core.savedObjects.client, sourceId) + ).configuration; const logEntry = await logEntries.getLogItem(requestContext, id, sourceConfiguration); diff --git a/x-pack/plugins/infra/server/routes/metadata/index.ts b/x-pack/plugins/infra/server/routes/metadata/index.ts index c45f191b1130d..fe142aa93dcda 100644 --- a/x-pack/plugins/infra/server/routes/metadata/index.ts +++ b/x-pack/plugins/infra/server/routes/metadata/index.ts @@ -44,7 +44,7 @@ export const initMetadataRoute = (libs: InfraBackendLibs) => { ); const { configuration } = await libs.sources.getSourceConfiguration( - requestContext, + requestContext.core.savedObjects.client, sourceId ); const metricsMetadata = await getMetricMetadata( diff --git a/x-pack/plugins/infra/server/routes/node_details/index.ts b/x-pack/plugins/infra/server/routes/node_details/index.ts index 36906f6f4125b..a457ccac2416c 100644 --- a/x-pack/plugins/infra/server/routes/node_details/index.ts +++ b/x-pack/plugins/infra/server/routes/node_details/index.ts @@ -37,7 +37,10 @@ export const initNodeDetailsRoute = (libs: InfraBackendLibs) => { NodeDetailsRequestRT.decode(request.body), fold(throwErrors(Boom.badRequest), identity) ); - const source = await libs.sources.getSourceConfiguration(requestContext, sourceId); + const source = await libs.sources.getSourceConfiguration( + requestContext.core.savedObjects.client, + sourceId + ); UsageCollector.countNode(nodeType); diff --git a/x-pack/plugins/infra/server/routes/snapshot/index.ts b/x-pack/plugins/infra/server/routes/snapshot/index.ts index e45b9884967d0..d1dc03893a0d9 100644 --- a/x-pack/plugins/infra/server/routes/snapshot/index.ts +++ b/x-pack/plugins/infra/server/routes/snapshot/index.ts @@ -42,7 +42,10 @@ export const initSnapshotRoute = (libs: InfraBackendLibs) => { SnapshotRequestRT.decode(request.body), fold(throwErrors(Boom.badRequest), identity) ); - const source = await libs.sources.getSourceConfiguration(requestContext, sourceId); + const source = await libs.sources.getSourceConfiguration( + requestContext.core.savedObjects.client, + sourceId + ); UsageCollector.countNode(nodeType); const options = { filterQuery: parseFilterQuery(filterQuery), From 12e2270002537eb4df2822857ab29cdaf4abbb13 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 16 Apr 2020 12:55:23 +0100 Subject: [PATCH 13/25] Pull index pattern into executor through sources library --- .../log_threshold/log_threshold_executor.ts | 16 ++++++++-------- .../register_log_threshold_alert_type.ts | 8 ++++++-- .../register_metric_threshold_alert_type.ts | 6 +++++- .../server/lib/alerting/register_alert_types.ts | 5 +++-- x-pack/plugins/infra/server/plugin.ts | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index 15149dbb329ce..b5573101def08 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -7,22 +7,22 @@ import { i18n } from '@kbn/i18n'; import { AlertExecutorOptions } from '../../../../../alerting/server'; import { AlertStates, Comparator, LogThresholdAlertParams } from './types'; +import { InfraBackendLibs } from '../../infra_types'; const comparatorMap = { [Comparator.GT]: (a: number, b: number) => a > b, [Comparator.GT_OR_EQ]: (a: number, b: number) => a >= b, }; -export const createLogThresholdExecutor = (alertUUID: string) => +export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackendLibs) => async function({ services, params }: AlertExecutorOptions) { - const alertInstance = services.alertInstanceFactory(alertUUID); - const { threshold, comparator, timeUnit, timeSize } = params as LogThresholdAlertParams; - + const { alertInstanceFactory, savedObjectsClient } = services; + const { sources } = libs; + const alertInstance = alertInstanceFactory(alertUUID); + const { timeSize, timeUnit, count, criteria } = params as LogThresholdAlertParams; const interval = `${timeSize}${timeUnit}`; - - const logCount = 1; // Replace this with a function to count matching logs over the `interval` - const comparisonFunction = comparatorMap[comparator]; - + const sourceConfiguration = await sources.getSourceConfiguration(savedObjectsClient, 'default'); + const indexPattern = sourceConfiguration.configuration.logAlias; const shouldAlertFire = comparisonFunction(logCount, threshold); const isNoData = false; // Set this to true if there is no log data over the specified interval const isError = false; // Set this to true if Elasticsearch throws an error when fetching logs diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts index dc2175c1118e0..31f649edac1aa 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -9,6 +9,7 @@ import { schema } from '@kbn/config-schema'; import { PluginSetupContract } from '../../../../../alerting/server'; import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_executor'; import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, Comparator } from './types'; +import { InfraBackendLibs } from '../../infra_types'; // const sampleActionVariableDescription = i18n.translate( // 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', @@ -44,7 +45,10 @@ const criteriaSchema = schema.object({ value: schema.oneOf([schema.number(), schema.string()]), }); -export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupContract) { +export async function registerLogThresholdAlertType( + alertingPlugin: PluginSetupContract, + libs: InfraBackendLibs +) { if (!alertingPlugin) { throw new Error( 'Cannot register log threshold alert type. Both the actions and alerting plugins need to be enabled.' @@ -66,7 +70,7 @@ export async function registerLogThresholdAlertType(alertingPlugin: PluginSetupC }, defaultActionGroupId: FIRED_ACTIONS.id, actionGroups: [FIRED_ACTIONS], - executor: createLogThresholdExecutor(alertUUID), + executor: createLogThresholdExecutor(alertUUID, libs), // actionVariables: { // context: [{ name: 'sample', description: sampleActionVariableDescription }], // }, diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts index 8808219cabaa7..18450776eb54c 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts @@ -9,8 +9,12 @@ import { schema } from '@kbn/config-schema'; import { PluginSetupContract } from '../../../../../alerting/server'; import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor'; import { METRIC_THRESHOLD_ALERT_TYPE_ID } from './types'; +import { InfraBackendLibs } from '../../infra_types'; -export async function registerMetricThresholdAlertType(alertingPlugin: PluginSetupContract) { +export async function registerMetricThresholdAlertType( + alertingPlugin: PluginSetupContract, + libs: InfraBackendLibs +) { if (!alertingPlugin) { throw new Error( 'Cannot register metric threshold alert type. Both the actions and alerting plugins need to be enabled.' diff --git a/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts b/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts index 7c74a5d74e802..9760873ff7478 100644 --- a/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/register_alert_types.ts @@ -7,13 +7,14 @@ import { PluginSetupContract } from '../../../../alerting/server'; import { registerMetricThresholdAlertType } from './metric_threshold/register_metric_threshold_alert_type'; import { registerLogThresholdAlertType } from './log_threshold/register_log_threshold_alert_type'; +import { InfraBackendLibs } from '../infra_types'; -const registerAlertTypes = (alertingPlugin: PluginSetupContract) => { +const registerAlertTypes = (alertingPlugin: PluginSetupContract, libs: InfraBackendLibs) => { if (alertingPlugin) { const registerFns = [registerMetricThresholdAlertType, registerLogThresholdAlertType]; registerFns.forEach(fn => { - fn(alertingPlugin); + fn(alertingPlugin, libs); }); } }; diff --git a/x-pack/plugins/infra/server/plugin.ts b/x-pack/plugins/infra/server/plugin.ts index 64fc496f3597e..be499d4254650 100644 --- a/x-pack/plugins/infra/server/plugin.ts +++ b/x-pack/plugins/infra/server/plugin.ts @@ -147,7 +147,7 @@ export class InfraServerPlugin { ]); initInfraServer(this.libs); - registerAlertTypes(plugins.alerting); + registerAlertTypes(plugins.alerting, this.libs); // Telemetry UsageCollector.registerUsageCollector(plugins.usageCollection); From 603ccb8b5888ce09abebdb787411cc7feda28797 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Fri, 17 Apr 2020 16:24:27 +0100 Subject: [PATCH 14/25] Executor and ActionVariable logic --- .../alerting/logs/log_threshold_alert_type.ts | 3 +- .../log_threshold/log_threshold_executor.ts | 203 ++++++++++++++++-- .../register_log_threshold_alert_type.ts | 29 ++- 3 files changed, 201 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts index fcd9643825722..1b5c269b2d1a6 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts @@ -23,8 +23,7 @@ export function getAlertType(): AlertTypeModel { defaultActionMessage: i18n.translate( 'xpack.infra.logs.alerting.threshold.defaultActionMessage', { - defaultMessage: - 'This should be a default message for a logs alert, including example use of context variables', + defaultMessage: `\\{\\{context.matchingDocuments\\}\\} log entries have matched the following conditions: \\{\\{context.conditions\\}\\}`, } ), }; diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index b5573101def08..3763b72a4a9d3 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -8,44 +8,203 @@ import { i18n } from '@kbn/i18n'; import { AlertExecutorOptions } from '../../../../../alerting/server'; import { AlertStates, Comparator, LogThresholdAlertParams } from './types'; import { InfraBackendLibs } from '../../infra_types'; +import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds'; -const comparatorMap = { +const checkValueAgainstComparatorMap = { [Comparator.GT]: (a: number, b: number) => a > b, [Comparator.GT_OR_EQ]: (a: number, b: number) => a >= b, + [Comparator.LT]: (a: number, b: number) => a < b, + [Comparator.LT_OR_EQ]: (a: number, b: number) => a <= b, }; export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackendLibs) => async function({ services, params }: AlertExecutorOptions) { - const { alertInstanceFactory, savedObjectsClient } = services; - const { sources } = libs; - const alertInstance = alertInstanceFactory(alertUUID); const { timeSize, timeUnit, count, criteria } = params as LogThresholdAlertParams; - const interval = `${timeSize}${timeUnit}`; + const { alertInstanceFactory, savedObjectsClient, callCluster } = services; + const { sources } = libs; + const sourceConfiguration = await sources.getSourceConfiguration(savedObjectsClient, 'default'); const indexPattern = sourceConfiguration.configuration.logAlias; - const shouldAlertFire = comparisonFunction(logCount, threshold); - const isNoData = false; // Set this to true if there is no log data over the specified interval - const isError = false; // Set this to true if Elasticsearch throws an error when fetching logs - if (shouldAlertFire) { - const sampleOutput = 'This is an example of data to send to an action context'; + const alertInstance = alertInstanceFactory(alertUUID); + + try { + const query = getESQuery(params, sourceConfiguration.configuration); + const result = await getResults(query, indexPattern, callCluster); + + if (checkValueAgainstComparatorMap[count.comparator](result.count, count.value)) { + alertInstance.scheduleActions(FIRED_ACTIONS.id, { + matchingDocuments: result.count, + conditions: createConditionsMessage(criteria), + }); - alertInstance.scheduleActions(FIRED_ACTIONS.id, { - sample: sampleOutput, + alertInstance.replaceState({ + alertState: AlertStates.ALERT, + }); + } else { + alertInstance.replaceState({ + alertState: AlertStates.OK, + }); + } + } catch (e) { + alertInstance.replaceState({ + alertState: AlertStates.ERROR, }); } - // Future use: ability to fetch and display current alert state - alertInstance.replaceState({ - alertState: isError - ? AlertStates.ERROR - : isNoData - ? AlertStates.NO_DATA - : shouldAlertFire - ? AlertStates.ALERT - : AlertStates.OK, - }); }; +const getESQuery = (params, sourceConfiguration) => { + const { timeSize, timeUnit, count, criteria } = params as LogThresholdAlertParams; + const interval = `${timeSize}${timeUnit}`; + const intervalAsSeconds = getIntervalInSeconds(interval); + const to = Date.now(); + const from = to - intervalAsSeconds * 1000; + + const rangeFilters = [ + { + range: { + [sourceConfiguration.fields.timestamp]: { + gte: from, + lte: to, + format: 'epoch_millis', + }, + }, + }, + ]; + + const positiveComparators = getPositiveComparators(); + const negativeComparators = getNegativeComparators(); + const positiveCriteria = criteria.filter(criterion => + positiveComparators.includes(criterion.comparator) + ); + const negativeCriteria = criteria.filter(criterion => + negativeComparators.includes(criterion.comparator) + ); + // Positive assertions (things that "must" match) + const mustFilters = buildFiltersForCriteria(positiveCriteria); + // Negative assertions (things that "must not" match) + const mustNotFilters = buildFiltersForCriteria(negativeCriteria); + + const query = { + query: { + bool: { + filter: [...rangeFilters], + ...(mustFilters.length > 0 && { must: mustFilters }), + ...(mustNotFilters.length > 0 && { must_not: mustNotFilters }), + }, + }, + }; + + return query; +}; + +const buildFiltersForCriteria = criteria => { + let filters = []; + criteria.forEach(criterion => { + const criterionQuery = buildCriterionQuery(criterion); + filters = [...filters, criterionQuery]; + }); + return filters; +}; + +const buildCriterionQuery = criterion => { + const { field, value, comparator } = criterion; + + const queryType = getQueryMappingForComparator(comparator); + + switch (queryType) { + case 'term': + return { + term: { + [field]: { + value, + }, + }, + }; + break; + case 'match': { + return { + match: { + [field]: value, + }, + }; + } + case 'match_phrase': { + return { + match_phrase: { + [field]: value, + }, + }; + } + case 'range': { + const comparatorToRangePropertyMapping = { + [Comparator.LT]: 'lt', + [Comparator.LT_OR_EQ]: 'lte', + [Comparator.GT]: 'gt', + [Comparator.GT_OR_EQ]: 'gte', + }; + + const rangeProperty = comparatorToRangePropertyMapping[comparator]; + + return { + range: { + [field]: { + [rangeProperty]: value, + }, + }, + }; + } + } +}; + +const getPositiveComparators = () => { + return [ + Comparator.GT, + Comparator.GT_OR_EQ, + Comparator.LT, + Comparator.LT_OR_EQ, + Comparator.EQ, + Comparator.MATCH, + Comparator.MATCH_PHRASE, + ]; +}; + +const getNegativeComparators = () => { + return [Comparator.NOT_EQ, Comparator.NOT_MATCH, Comparator.NOT_MATCH_PHRASE]; +}; + +const queryMappings = { + [Comparator.GT]: 'range', + [Comparator.GT_OR_EQ]: 'range', + [Comparator.LT]: 'range', + [Comparator.LT_OR_EQ]: 'range', + [Comparator.EQ]: 'term', + [Comparator.MATCH]: 'match', + [Comparator.MATCH_PHRASE]: 'match_phrase', + [Comparator.NOT_EQ]: 'term', + [Comparator.NOT_MATCH]: 'match', + [Comparator.NOT_MATCH_PHRASE]: 'match_phrase', +}; + +const getQueryMappingForComparator = comparator => { + return queryMappings[comparator]; +}; + +const getResults = async (query, index, callCluster) => { + return await callCluster('count', { + body: query, + index, + }); +}; + +const createConditionsMessage = criteria => { + const parts = criteria.map((criterion, index) => { + const { field, comparator, value } = criterion; + return `${index === 0 ? '' : 'and'} ${field} ${comparator} ${value}`; + }); + return parts.join(' '); +}; + // When the Alerting plugin implements support for multiple action groups, add additional // action groups here to send different messages, e.g. a recovery notification export const FIRED_ACTIONS = { diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts index 31f649edac1aa..a6ae3c33a7315 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -11,13 +11,19 @@ import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_execu import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, Comparator } from './types'; import { InfraBackendLibs } from '../../infra_types'; -// const sampleActionVariableDescription = i18n.translate( -// 'xpack.infra.logs.alerting.threshold.sampleActionVariableDescription', -// { -// defaultMessage: -// 'Action variables are whatever values you want to make available to messages that this alert sends. This one would replace in an action message.', -// } -// ); +const documentCountActionVariableDescription = i18n.translate( + 'xpack.infra.logs.alerting.threshold.documentCountActionVariableDescription', + { + defaultMessage: 'The number of log entries that matched the conditions provided', + } +); + +const conditionsActionVariableDescription = i18n.translate( + 'xpack.infra.logs.alerting.threshold.conditionsActionVariableDescription', + { + defaultMessage: 'The conditions that log entries needed to fulfill', + } +); const countSchema = schema.object({ value: schema.number(), @@ -71,8 +77,11 @@ export async function registerLogThresholdAlertType( defaultActionGroupId: FIRED_ACTIONS.id, actionGroups: [FIRED_ACTIONS], executor: createLogThresholdExecutor(alertUUID, libs), - // actionVariables: { - // context: [{ name: 'sample', description: sampleActionVariableDescription }], - // }, + actionVariables: { + context: [ + { name: 'matchingDocuments', description: documentCountActionVariableDescription }, + { name: 'conditions', description: conditionsActionVariableDescription }, + ], + }, }); } From 023edda379df3520e676562c24a0792ec433ad15 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 20 Apr 2020 13:05:46 +0100 Subject: [PATCH 15/25] Move types to /common --- .../log_threshold => common/alerting/logs}/types.ts | 0 .../infra/public/components/alerting/logs/alert_flyout.tsx | 3 +-- .../alerting/logs/expression_editor/criterion.tsx | 3 +-- .../alerting/logs/expression_editor/document_count.tsx | 3 +-- .../components/alerting/logs/expression_editor/editor.tsx | 2 +- .../components/alerting/logs/log_threshold_alert_type.ts | 3 +-- .../lib/alerting/log_threshold/log_threshold_executor.ts | 6 +++++- .../log_threshold/register_log_threshold_alert_type.ts | 5 ++++- 8 files changed, 14 insertions(+), 11 deletions(-) rename x-pack/plugins/infra/{server/lib/alerting/log_threshold => common/alerting/logs}/types.ts (100%) diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts b/x-pack/plugins/infra/common/alerting/logs/types.ts similarity index 100% rename from x-pack/plugins/infra/server/lib/alerting/log_threshold/types.ts rename to x-pack/plugins/infra/common/alerting/logs/types.ts diff --git a/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx index 099cc3d1e6dd9..b18c2e5b8d69c 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/alert_flyout.tsx @@ -8,8 +8,7 @@ import React, { useContext } from 'react'; import { AlertsContextProvider, AlertAdd } from '../../../../../triggers_actions_ui/public'; import { TriggerActionsContext } from '../../../utils/triggers_actions_context'; import { useKibana } from '../../../../../../../src/plugins/kibana_react/public'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; +import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../common/alerting/logs/types'; interface Props { visible?: boolean; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index 9d268eed35b45..1a37b79a8770f 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -18,8 +18,7 @@ import { EuiFormRow, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; +import { Comparator } from '../../../../../common/alerting/logs/types'; const getCompatibleComparatorsForField = fieldInfo => { if (fieldInfo.type === 'number') { diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx index 23aa9ba9e3ad4..4309f18fcc682 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -15,8 +15,7 @@ import { EuiExpression, EuiFormRow, } from '@elastic/eui'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Comparator } from '../../../../../server/lib/alerting/log_threshold/types'; +import { Comparator } from '../../../../../common/alerting/logs/types'; const getComparatorOptions = () => { return [ diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 4efb91e7aec58..168e9d35e2149 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -28,7 +28,7 @@ import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { useSource } from '../../../../containers/source'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Comparator, TimeUnit } from '../../../../../server/lib/alerting/log_threshold/types'; +import { Comparator, TimeUnit } from '../../../../../common/alerting/logs/types'; import { DocumentCount } from './document_count'; import { Criteria } from './criteria'; diff --git a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts index 1b5c269b2d1a6..18126ec583696 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts @@ -6,8 +6,7 @@ import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { AlertTypeModel } from '../../../../../triggers_actions_ui/public/types'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../server/lib/alerting/log_threshold/types'; +import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../common/alerting/logs/types'; import { ExpressionEditor } from './expression_editor'; import { validateExpression } from './validation'; diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index 3763b72a4a9d3..fed7bd69ed550 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -6,7 +6,11 @@ import { i18n } from '@kbn/i18n'; import { AlertExecutorOptions } from '../../../../../alerting/server'; -import { AlertStates, Comparator, LogThresholdAlertParams } from './types'; +import { + AlertStates, + Comparator, + LogThresholdAlertParams, +} from '../../../../common/alerting/logs/types'; import { InfraBackendLibs } from '../../infra_types'; import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds'; diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts index a6ae3c33a7315..04207a4233dfd 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_alert_type.ts @@ -8,7 +8,10 @@ import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; import { PluginSetupContract } from '../../../../../alerting/server'; import { createLogThresholdExecutor, FIRED_ACTIONS } from './log_threshold_executor'; -import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, Comparator } from './types'; +import { + LOG_DOCUMENT_COUNT_ALERT_TYPE_ID, + Comparator, +} from '../../../../common/alerting/logs/types'; import { InfraBackendLibs } from '../../infra_types'; const documentCountActionVariableDescription = i18n.translate( From b44a04bbdc8494c4d8533032739efdd42b796a33 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 20 Apr 2020 14:10:07 +0100 Subject: [PATCH 16/25] Full typing for executor function --- .../infra/common/alerting/logs/types.ts | 15 ++++- .../log_threshold/log_threshold_executor.ts | 61 ++++++++++++++----- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/logs/types.ts b/x-pack/plugins/infra/common/alerting/logs/types.ts index 275b9320c92fc..472ef04bbd8ff 100644 --- a/x-pack/plugins/infra/common/alerting/logs/types.ts +++ b/x-pack/plugins/infra/common/alerting/logs/types.ts @@ -26,9 +26,20 @@ export enum AlertStates { ERROR, } -export interface LogThresholdAlertParams { - threshold: number; +export interface DocumentCount { comparator: Comparator; + value: number; +} + +export interface Criterion { + field: string; + comparator: Comparator; + value: string | number; +} + +export interface LogDocumentCountAlertParams { + count: DocumentCount; + criteria: Criterion[]; timeUnit: 's' | 'm' | 'h' | 'd'; timeSize: number; } diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index fed7bd69ed550..c642779bb1f5e 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -5,16 +5,20 @@ */ import { i18n } from '@kbn/i18n'; -import { AlertExecutorOptions } from '../../../../../alerting/server'; +import { AlertExecutorOptions, AlertServices } from '../../../../../alerting/server'; import { AlertStates, Comparator, - LogThresholdAlertParams, + LogDocumentCountAlertParams, + Criterion, } from '../../../../common/alerting/logs/types'; import { InfraBackendLibs } from '../../infra_types'; import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds'; +import { InfraSource } from '../../../../common/http_api/source_api'; -const checkValueAgainstComparatorMap = { +const checkValueAgainstComparatorMap: { + [key: string]: (a: number, b: number) => boolean; +} = { [Comparator.GT]: (a: number, b: number) => a > b, [Comparator.GT_OR_EQ]: (a: number, b: number) => a >= b, [Comparator.LT]: (a: number, b: number) => a < b, @@ -23,7 +27,7 @@ const checkValueAgainstComparatorMap = { export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackendLibs) => async function({ services, params }: AlertExecutorOptions) { - const { timeSize, timeUnit, count, criteria } = params as LogThresholdAlertParams; + const { count, criteria } = params as LogDocumentCountAlertParams; const { alertInstanceFactory, savedObjectsClient, callCluster } = services; const { sources } = libs; @@ -33,7 +37,10 @@ export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackend const alertInstance = alertInstanceFactory(alertUUID); try { - const query = getESQuery(params, sourceConfiguration.configuration); + const query = getESQuery( + params as LogDocumentCountAlertParams, + sourceConfiguration.configuration + ); const result = await getResults(query, indexPattern, callCluster); if (checkValueAgainstComparatorMap[count.comparator](result.count, count.value)) { @@ -57,8 +64,11 @@ export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackend } }; -const getESQuery = (params, sourceConfiguration) => { - const { timeSize, timeUnit, count, criteria } = params as LogThresholdAlertParams; +const getESQuery = ( + params: LogDocumentCountAlertParams, + sourceConfiguration: InfraSource['configuration'] +): object => { + const { timeSize, timeUnit, criteria } = params; const interval = `${timeSize}${timeUnit}`; const intervalAsSeconds = getIntervalInSeconds(interval); const to = Date.now(); @@ -102,16 +112,24 @@ const getESQuery = (params, sourceConfiguration) => { return query; }; -const buildFiltersForCriteria = criteria => { - let filters = []; +type SupportedESQueryTypes = 'term' | 'match' | 'match_phrase' | 'range'; +type Filter = { + [key in SupportedESQueryTypes]?: object; +}; + +const buildFiltersForCriteria = (criteria: LogDocumentCountAlertParams['criteria']) => { + let filters: Filter[] = []; + criteria.forEach(criterion => { const criterionQuery = buildCriterionQuery(criterion); - filters = [...filters, criterionQuery]; + if (criterionQuery) { + filters = [...filters, criterionQuery]; + } }); return filters; }; -const buildCriterionQuery = criterion => { +const buildCriterionQuery = (criterion: Criterion): Filter | undefined => { const { field, value, comparator } = criterion; const queryType = getQueryMappingForComparator(comparator); @@ -141,7 +159,9 @@ const buildCriterionQuery = criterion => { }; } case 'range': { - const comparatorToRangePropertyMapping = { + const comparatorToRangePropertyMapping: { + [key: string]: string; + } = { [Comparator.LT]: 'lt', [Comparator.LT_OR_EQ]: 'lte', [Comparator.GT]: 'gt', @@ -158,6 +178,9 @@ const buildCriterionQuery = criterion => { }, }; } + default: { + return undefined; + } } }; @@ -177,7 +200,9 @@ const getNegativeComparators = () => { return [Comparator.NOT_EQ, Comparator.NOT_MATCH, Comparator.NOT_MATCH_PHRASE]; }; -const queryMappings = { +const queryMappings: { + [key: string]: string; +} = { [Comparator.GT]: 'range', [Comparator.GT_OR_EQ]: 'range', [Comparator.LT]: 'range', @@ -190,18 +215,22 @@ const queryMappings = { [Comparator.NOT_MATCH_PHRASE]: 'match_phrase', }; -const getQueryMappingForComparator = comparator => { +const getQueryMappingForComparator = (comparator: Comparator) => { return queryMappings[comparator]; }; -const getResults = async (query, index, callCluster) => { +const getResults = async ( + query: object, + index: string, + callCluster: AlertServices['callCluster'] +) => { return await callCluster('count', { body: query, index, }); }; -const createConditionsMessage = criteria => { +const createConditionsMessage = (criteria: LogDocumentCountAlertParams['criteria']) => { const parts = criteria.map((criterion, index) => { const { field, comparator, value } = criterion; return `${index === 0 ? '' : 'and'} ${field} ${comparator} ${value}`; From be0cdb7b2c33377d32ebd0e1dc16623fcfbe38f5 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 20 Apr 2020 16:36:22 +0100 Subject: [PATCH 17/25] Client side typing --- .../logs/expression_editor/criteria.tsx | 18 ++++++- .../logs/expression_editor/criterion.tsx | 41 +++++++++++----- .../logs/expression_editor/document_count.tsx | 20 ++++++-- .../logs/expression_editor/editor.tsx | 49 +++++-------------- .../components/alerting/logs/validation.ts | 12 +++-- .../infra/server/routes/source/index.ts | 5 +- 6 files changed, 83 insertions(+), 62 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx index 3954c8f0e3d83..d170dfb5721ca 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -6,10 +6,24 @@ import React from 'react'; import { EuiFlexItem, EuiFlexGroup } from '@elastic/eui'; +import { IFieldType } from 'src/plugins/data/public'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; import { Criterion } from './criterion'; +import { + LogDocumentCountAlertParams, + Criterion as CriterionType, +} from '../../../../../common/alerting/logs/types'; -export const Criteria: React.FC = ({ +interface Props { + fields: IFieldType[]; + criteria?: LogDocumentCountAlertParams['criteria']; + updateCriterion: (idx: number, params: Partial) => void; + removeCriterion: (idx: number) => void; + errors: IErrorObject; +} + +export const Criteria: React.FC = ({ fields, criteria, updateCriterion, @@ -30,7 +44,7 @@ export const Criteria: React.FC = ({ updateCriterion={updateCriterion} removeCriterion={removeCriterion} canDelete={criteria.length > 1} - errors={errors[idx]} + errors={errors[idx.toString()] as IErrorObject} /> ); })} diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index 1a37b79a8770f..0ac2ee0bcf62a 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -18,10 +18,13 @@ import { EuiFormRow, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { Comparator } from '../../../../../common/alerting/logs/types'; +import { IFieldType } from 'src/plugins/data/public'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; +import { Comparator, Criterion as CriterionType } from '../../../../../common/alerting/logs/types'; -const getCompatibleComparatorsForField = fieldInfo => { - if (fieldInfo.type === 'number') { +const getCompatibleComparatorsForField = (fieldInfo: IFieldType | undefined) => { + if (fieldInfo?.type === 'number') { return [ { value: Comparator.GT, text: Comparator.GT }, { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, @@ -30,7 +33,7 @@ const getCompatibleComparatorsForField = fieldInfo => { { value: Comparator.EQ, text: Comparator.EQ }, { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, ]; - } else if (fieldInfo.aggregatable) { + } else if (fieldInfo?.aggregatable) { return [ { value: Comparator.EQ, text: Comparator.EQ }, { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, @@ -45,13 +48,23 @@ const getCompatibleComparatorsForField = fieldInfo => { } }; -const getFieldInfo = (fields, fieldName) => { +const getFieldInfo = (fields: IFieldType[], fieldName: string): IFieldType | undefined => { return fields.find(field => { return field.name === fieldName; }); }; -export const Criterion: React.FC = ({ +interface Props { + idx: number; + fields: IFieldType[]; + criterion: CriterionType; + updateCriterion: (idx: number, params: Partial) => void; + removeCriterion: (idx: number) => void; + canDelete: boolean; + errors: IErrorObject; +} + +export const Criterion: React.FC = ({ idx, fields, criterion, @@ -69,7 +82,7 @@ export const Criterion: React.FC = ({ }); }, [fields]); - const fieldInfo = useMemo(() => { + const fieldInfo: IFieldType | undefined = useMemo(() => { return getFieldInfo(fields, criterion.field); }, [fields, criterion]); @@ -83,8 +96,10 @@ export const Criterion: React.FC = ({ const nextFieldInfo = getFieldInfo(fields, fieldName); // If the field information we're dealing with has changed, reset the comparator and value. if ( - fieldInfo.type !== nextFieldInfo.type || - fieldInfo.aggregatable !== nextFieldInfo.aggregatable + fieldInfo && + nextFieldInfo && + (fieldInfo.type !== nextFieldInfo.type || + fieldInfo.aggregatable !== nextFieldInfo.aggregatable) ) { const compatibleComparators = getCompatibleComparatorsForField(nextFieldInfo); updateCriterion(idx, { @@ -96,7 +111,7 @@ export const Criterion: React.FC = ({ updateCriterion(idx, { field: fieldName }); } }, - [fieldInfo.aggregatable, fieldInfo.type, fields, idx, updateCriterion] + [fieldInfo, fields, idx, updateCriterion] ); return ( @@ -160,15 +175,15 @@ export const Criterion: React.FC = ({ updateCriterion(idx, { comparator: e.target.value })} + onChange={e => updateCriterion(idx, { comparator: e.target.value as Comparator })} options={compatibleComparatorOptions} /> 0} error={errors.value}> - {fieldInfo.type === 'number' ? ( + {fieldInfo?.type === 'number' ? ( { const number = parseInt(e.target.value, 10); updateCriterion(idx, { value: number ? number : undefined }); diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx index 4309f18fcc682..41f623c27d2ef 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -15,9 +15,14 @@ import { EuiExpression, EuiFormRow, } from '@elastic/eui'; -import { Comparator } from '../../../../../common/alerting/logs/types'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; +import { Comparator, LogDocumentCountAlertParams } from '../../../../../common/alerting/logs/types'; -const getComparatorOptions = () => { +const getComparatorOptions = (): Array<{ + value: Comparator; + text: Comparator; +}> => { return [ { value: Comparator.LT, text: Comparator.LT }, { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, @@ -26,7 +31,14 @@ const getComparatorOptions = () => { ]; }; -export const DocumentCount: React.FC = ({ comparator, value, updateCount, errors }) => { +interface Props { + comparator?: Comparator; + value?: number; + updateCount: (params: Partial) => void; + errors: IErrorObject; +} + +export const DocumentCount: React.FC = ({ comparator, value, updateCount, errors }) => { const [isComparatorPopoverOpen, setComparatorPopoverOpenState] = useState(false); const [isValuePopoverOpen, setIsValuePopoverOpen] = useState(false); @@ -55,7 +67,7 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount, errors updateCount({ comparator: e.target.value })} + onChange={e => updateCount({ comparator: e.target.value as Comparator })} options={getComparatorOptions()} />
diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx index 168e9d35e2149..ea6be2c2431a7 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx @@ -5,43 +5,23 @@ */ import React, { useCallback, useMemo, useEffect, useState } from 'react'; -import { - EuiFlexGroup, - EuiFlexItem, - EuiButtonIcon, - EuiSpacer, - EuiText, - EuiFormRow, - EuiButtonEmpty, -} from '@elastic/eui'; -import { IFieldType } from 'src/plugins/data/public'; +import { EuiButtonEmpty } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; -import { i18n } from '@kbn/i18n'; -import { uniq } from 'lodash'; -import { euiStyled } from '../../../../../../observability/public'; import { ForLastExpression, // eslint-disable-next-line @kbn/eslint/no-restricted-paths } from '../../../../../../triggers_actions_ui/public/common'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths import { useSource } from '../../../../containers/source'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Comparator, TimeUnit } from '../../../../../common/alerting/logs/types'; +import { + LogDocumentCountAlertParams, + Comparator, + TimeUnit, +} from '../../../../../common/alerting/logs/types'; import { DocumentCount } from './document_count'; import { Criteria } from './criteria'; -export interface LogsDocumentCountExpression { - count?: { - value?: number; - comparator?: Comparator; - }; - timeSize?: number; - timeUnit?: TimeUnit; - criteria?: ExpressionCriteria[]; -} - export interface ExpressionCriteria { field?: string; comparator?: Comparator; @@ -50,7 +30,7 @@ export interface ExpressionCriteria { interface Props { errors: IErrorObject; - alertParams: LogsDocumentCountExpression; + alertParams: Partial; setAlertParams(key: string, value: any): void; setAlertProperty(key: string, value: any): void; } @@ -136,7 +116,7 @@ export const ExpressionEditor: React.FC = props => { const removeCriterion = useCallback( idx => { - const nextCriteria = alertParams?.criteria.filter((criterion, index) => { + const nextCriteria = alertParams?.criteria?.filter((criterion, index) => { return index !== idx; }); setAlertParams('criteria', nextCriteria); @@ -144,13 +124,6 @@ export const ExpressionEditor: React.FC = props => { [alertParams, setAlertParams] ); - const emptyError = useMemo(() => { - return { - timeSizeUnit: [], - timeWindowSize: [], - }; - }, []); - // Wait until field info has loaded if (supportedFields.length === 0) return null; @@ -160,7 +133,7 @@ export const ExpressionEditor: React.FC = props => { comparator={alertParams.count?.comparator} value={alertParams.count?.value} updateCount={updateCount} - errors={errors.count} + errors={errors.count as IErrorObject} /> = props => { criteria={alertParams.criteria} updateCriterion={updateCriterion} removeCriterion={removeCriterion} - errors={errors.criteria} + errors={errors.criteria as IErrorObject} /> = props => { timeWindowUnit={timeUnit} onChangeWindowSize={updateTimeSize} onChangeWindowUnit={updateTimeUnit} - errors={errors} + errors={errors as { [key: string]: string[] }} />
diff --git a/x-pack/plugins/infra/public/components/alerting/logs/validation.ts b/x-pack/plugins/infra/public/components/alerting/logs/validation.ts index c9a108b1ce68d..c8c513f57a9d7 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/validation.ts +++ b/x-pack/plugins/infra/public/components/alerting/logs/validation.ts @@ -7,8 +7,14 @@ import { i18n } from '@kbn/i18n'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { ValidationResult } from '../../../../../triggers_actions_ui/public/types'; +import { LogDocumentCountAlertParams } from '../../../../common/alerting/logs/types'; -export function validateExpression({ count, criteria, timeSize, timeUnit }: any): ValidationResult { +export function validateExpression({ + count, + criteria, + timeSize, + timeUnit, +}: Partial): ValidationResult { const validationResult = { errors: {} }; // NOTE: In the case of components provided by the Alerting framework the error property names @@ -37,10 +43,8 @@ export function validateExpression({ count, criteria, timeSize, timeUnit }: any) validationResult.errors = errors; - if (!count && !criteria && !timeSize && !timeUnit) return validationResult; - // Document count validation - if (!count?.value) { + if (typeof count?.value !== 'number') { errors.count.value.push( i18n.translate('xpack.infra.logs.alertFlyout.error.documentCountRequired', { defaultMessage: 'Document count is Required.', diff --git a/x-pack/plugins/infra/server/routes/source/index.ts b/x-pack/plugins/infra/server/routes/source/index.ts index 2f29320d7bb81..62b7fd7ba902f 100644 --- a/x-pack/plugins/infra/server/routes/source/index.ts +++ b/x-pack/plugins/infra/server/routes/source/index.ts @@ -37,7 +37,10 @@ export const initSourceRoute = (libs: InfraBackendLibs) => { try { const { type, sourceId } = request.params; - const source = await libs.sources.getSourceConfiguration(requestContext, sourceId); + const source = await libs.sources.getSourceConfiguration( + requestContext.core.savedObjects.client, + sourceId + ); if (!source) { return response.notFound(); } From 74dcabab8689a230c8cdc3253a4a48e96a8939dd Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Tue, 21 Apr 2020 12:08:00 +0100 Subject: [PATCH 18/25] Amend source config tests --- .../plugins/infra/server/lib/sources/sources.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/sources/sources.test.ts b/x-pack/plugins/infra/server/lib/sources/sources.test.ts index 4a83ca730ff83..57efb0f676b2f 100644 --- a/x-pack/plugins/infra/server/lib/sources/sources.test.ts +++ b/x-pack/plugins/infra/server/lib/sources/sources.test.ts @@ -29,7 +29,9 @@ describe('the InfraSources lib', () => { }, }); - expect(await sourcesLib.getSourceConfiguration(request, 'TEST_ID')).toMatchObject({ + expect( + await sourcesLib.getSourceConfiguration(request.core.savedObjects.client, 'TEST_ID') + ).toMatchObject({ id: 'TEST_ID', version: 'foo', updatedAt: 946684800000, @@ -74,7 +76,9 @@ describe('the InfraSources lib', () => { }, }); - expect(await sourcesLib.getSourceConfiguration(request, 'TEST_ID')).toMatchObject({ + expect( + await sourcesLib.getSourceConfiguration(request.core.savedObjects.client, 'TEST_ID') + ).toMatchObject({ id: 'TEST_ID', version: 'foo', updatedAt: 946684800000, @@ -104,7 +108,9 @@ describe('the InfraSources lib', () => { attributes: {}, }); - expect(await sourcesLib.getSourceConfiguration(request, 'TEST_ID')).toMatchObject({ + expect( + await sourcesLib.getSourceConfiguration(request.core.savedObjects.client, 'TEST_ID') + ).toMatchObject({ id: 'TEST_ID', version: 'foo', updatedAt: 946684800000, From b923d0a88e656b843b7dbe0325d7eaa40f0f02ea Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Tue, 21 Apr 2020 13:21:33 +0100 Subject: [PATCH 19/25] Ensure i18n --- .../infra/common/alerting/logs/types.ts | 37 +++++++++++ .../logs/expression_editor/criterion.tsx | 66 ++++++++++++++----- .../logs/expression_editor/document_count.tsx | 35 ++++++---- .../logs/expression_editor/editor.tsx | 4 ++ 4 files changed, 114 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/logs/types.ts b/x-pack/plugins/infra/common/alerting/logs/types.ts index 472ef04bbd8ff..c08ec98aad0c8 100644 --- a/x-pack/plugins/infra/common/alerting/logs/types.ts +++ b/x-pack/plugins/infra/common/alerting/logs/types.ts @@ -3,6 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; export const LOG_DOCUMENT_COUNT_ALERT_TYPE_ID = 'logs.alert.document.count'; @@ -19,6 +20,42 @@ export enum Comparator { NOT_MATCH_PHRASE = 'does not match phrase', } +export const ComparatorToi18nMap = { + [Comparator.GT]: i18n.translate('xpack.infra.logs.alerting.comparator.gt', { + defaultMessage: 'more than', + }), + [Comparator.GT_OR_EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.gtOrEq', { + defaultMessage: 'more than or equals', + }), + [Comparator.LT]: i18n.translate('xpack.infra.logs.alerting.comparator.lt', { + defaultMessage: 'less than', + }), + [Comparator.LT_OR_EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.ltOrEq', { + defaultMessage: 'less than or equals', + }), + [Comparator.EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { + defaultMessage: 'equals', + }), + [Comparator.NOT_EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.notEq', { + defaultMessage: 'does not equal', + }), + [Comparator.MATCH]: i18n.translate('xpack.infra.logs.alerting.comparator.match', { + defaultMessage: 'matches', + }), + [Comparator.NOT_MATCH]: i18n.translate('xpack.infra.logs.alerting.comparator.notMatch', { + defaultMessage: 'does not match', + }), + [Comparator.MATCH_PHRASE]: i18n.translate('xpack.infra.logs.alerting.comparator.matchPhrase', { + defaultMessage: 'matches phrase', + }), + [Comparator.NOT_MATCH_PHRASE]: i18n.translate( + 'xpack.infra.logs.alerting.comparator.notMatchPhrase', + { + defaultMessage: 'does not match phrase', + } + ), +}; + export enum AlertStates { OK, ALERT, diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index 0ac2ee0bcf62a..e931d0d7f1ea5 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -21,29 +21,61 @@ import { i18n } from '@kbn/i18n'; import { IFieldType } from 'src/plugins/data/public'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; -import { Comparator, Criterion as CriterionType } from '../../../../../common/alerting/logs/types'; +import { + Comparator, + Criterion as CriterionType, + ComparatorToi18nMap, +} from '../../../../../common/alerting/logs/types'; + +const firstCriterionFieldPrefix = i18n.translate( + 'xpack.infra.logs.alertFlyout.firstCriterionFieldPrefix', + { + defaultMessage: 'with', + } +); + +const successiveCriterionFieldPrefix = i18n.translate( + 'xpack.infra.logs.alertFlyout.successiveCriterionFieldPrefix', + { + defaultMessage: 'and', + } +); + +const criterionFieldTitle = i18n.translate('xpack.infra.logs.alertFlyout.criterionFieldTitle', { + defaultMessage: 'Field', +}); + +const criterionComparatorValueTitle = i18n.translate( + 'xpack.infra.logs.alertFlyout.criterionComparatorValueTitle', + { + defaultMessage: 'Comparison : Value', + } +); const getCompatibleComparatorsForField = (fieldInfo: IFieldType | undefined) => { if (fieldInfo?.type === 'number') { return [ - { value: Comparator.GT, text: Comparator.GT }, - { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, - { value: Comparator.LT, text: Comparator.LT }, - { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, - { value: Comparator.EQ, text: Comparator.EQ }, - { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + { value: Comparator.GT, text: ComparatorToi18nMap[Comparator.GT] }, + { value: Comparator.GT_OR_EQ, text: ComparatorToi18nMap[Comparator.GT_OR_EQ] }, + { value: Comparator.LT, text: ComparatorToi18nMap[Comparator.LT] }, + { value: Comparator.LT_OR_EQ, text: ComparatorToi18nMap[Comparator.LT_OR_EQ] }, + { value: Comparator.EQ, text: ComparatorToi18nMap[Comparator.EQ] }, + { value: Comparator.NOT_EQ, text: ComparatorToi18nMap[Comparator.NOT_EQ] }, ]; } else if (fieldInfo?.aggregatable) { return [ - { value: Comparator.EQ, text: Comparator.EQ }, - { value: Comparator.NOT_EQ, text: Comparator.NOT_EQ }, + { value: Comparator.EQ, text: ComparatorToi18nMap[Comparator.EQ] }, + { value: Comparator.NOT_EQ, text: ComparatorToi18nMap[Comparator.NOT_EQ] }, ]; } else { return [ - { value: Comparator.MATCH, text: Comparator.MATCH }, - { value: Comparator.NOT_MATCH, text: Comparator.NOT_MATCH }, - { value: Comparator.MATCH_PHRASE, text: Comparator.MATCH_PHRASE }, - { value: Comparator.NOT_MATCH_PHRASE, text: Comparator.NOT_MATCH_PHRASE }, + { value: Comparator.MATCH, text: ComparatorToi18nMap[Comparator.MATCH] }, + { value: Comparator.NOT_MATCH, text: ComparatorToi18nMap[Comparator.NOT_MATCH] }, + { value: Comparator.MATCH_PHRASE, text: ComparatorToi18nMap[Comparator.MATCH_PHRASE] }, + { + value: Comparator.NOT_MATCH_PHRASE, + text: ComparatorToi18nMap[Comparator.NOT_MATCH_PHRASE], + }, ]; } }; @@ -121,7 +153,7 @@ export const Criterion: React.FC = ({ id="criterion-field" button={ = ({ anchorPosition="downLeft" >
- Field + {criterionFieldTitle} 0} error={errors.field}> = ({ anchorPosition="downLeft" >
- Comparison : Value + {criterionComparatorValueTitle} 0} error={errors.comparator}> = ({ {fieldInfo?.type === 'number' ? ( { const number = parseInt(e.target.value, 10); updateCriterion(idx, { value: number ? number : undefined }); diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx index 41f623c27d2ef..4c35628d2ddb4 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -5,6 +5,7 @@ */ import React, { useState } from 'react'; +import { i18n } from '@kbn/i18n'; import { EuiPopoverTitle, EuiFlexItem, @@ -17,17 +18,29 @@ import { } from '@elastic/eui'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types'; -import { Comparator, LogDocumentCountAlertParams } from '../../../../../common/alerting/logs/types'; +import { + Comparator, + ComparatorToi18nMap, + LogDocumentCountAlertParams, +} from '../../../../../common/alerting/logs/types'; + +const documentCountPrefix = i18n.translate('xpack.infra.logs.alertFlyout.documentCountPrefix', { + defaultMessage: 'when', +}); + +const documentCountValue = i18n.translate('xpack.infra.logs.alertFlyout.documentCountValue', { + defaultMessage: 'Log entries', +}); const getComparatorOptions = (): Array<{ value: Comparator; - text: Comparator; + text: string; }> => { return [ - { value: Comparator.LT, text: Comparator.LT }, - { value: Comparator.LT_OR_EQ, text: Comparator.LT_OR_EQ }, - { value: Comparator.GT, text: Comparator.GT }, - { value: Comparator.GT_OR_EQ, text: Comparator.GT_OR_EQ }, + { value: Comparator.LT, text: ComparatorToi18nMap[Comparator.LT] }, + { value: Comparator.LT_OR_EQ, text: ComparatorToi18nMap[Comparator.LT_OR_EQ] }, + { value: Comparator.GT, text: ComparatorToi18nMap[Comparator.GT] }, + { value: Comparator.GT_OR_EQ, text: ComparatorToi18nMap[Comparator.GT_OR_EQ] }, ]; }; @@ -49,9 +62,9 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount, id="comparator" button={ setComparatorPopoverOpenState(true)} /> @@ -63,7 +76,7 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount, anchorPosition="downLeft" >
- WHEN + {documentCountPrefix} = ({ comparator, value, updateCount, setIsValuePopoverOpen(true)} color={errors.value.length === 0 ? 'secondary' : 'danger'} @@ -94,7 +107,7 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount, anchorPosition="downLeft" >
- LOG ENTRIES + {documentCountValue} 0} error={errors.value}> = props => { const { createDerivedIndexPattern } = useSource({ sourceId: 'default' }); const [timeSize, setTimeSize] = useState(1); const [timeUnit, setTimeUnit] = useState('m'); + const [hasSetDefaults, setHasSetDefaults] = useState(false); const derivedIndexPattern = useMemo(() => createDerivedIndexPattern('logs'), [ createDerivedIndexPattern, ]); @@ -70,6 +71,7 @@ export const ExpressionEditor: React.FC = props => { useEffect(() => { for (const [key, value] of Object.entries(DEFAULT_EXPRESSION)) { setAlertParams(key, value); + setHasSetDefaults(true); } }, []); // eslint-disable-line react-hooks/exhaustive-deps @@ -126,6 +128,8 @@ export const ExpressionEditor: React.FC = props => { // Wait until field info has loaded if (supportedFields.length === 0) return null; + // Wait until the alert param defaults have been set + if (!hasSetDefaults) return null; return ( <> From b91688ccd6ca52ce7bbd22031c294880345d6b59 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Tue, 21 Apr 2020 13:48:40 +0100 Subject: [PATCH 20/25] Tweak / polish styling --- .../logs/expression_editor/criteria.tsx | 2 +- .../logs/expression_editor/criterion.tsx | 176 ++++++++++-------- 2 files changed, 97 insertions(+), 81 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx index d170dfb5721ca..a9b45a117c281 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criteria.tsx @@ -33,7 +33,7 @@ export const Criteria: React.FC = ({ if (!criteria) return null; return ( - + {criteria.map((criterion, idx) => { return ( = ({ return ( - - setIsFieldPopoverOpen(true)} - /> - } - isOpen={isFieldPopoverOpen} - closePopover={() => setIsFieldPopoverOpen(false)} - ownFocus - panelPaddingSize="s" - anchorPosition="downLeft" - > -
- {criterionFieldTitle} - 0} error={errors.field}> - - -
- - - - setIsComparatorPopoverOpen(true)} - /> - } - isOpen={isComparatorPopoverOpen} - closePopover={() => setIsComparatorPopoverOpen(false)} - ownFocus - panelPaddingSize="s" - anchorPosition="downLeft" - > -
- {criterionComparatorValueTitle} - 0} error={errors.comparator}> - updateCriterion(idx, { comparator: e.target.value as Comparator })} - options={compatibleComparatorOptions} - /> - - 0} error={errors.value}> - {fieldInfo?.type === 'number' ? ( - { - const number = parseInt(e.target.value, 10); - updateCriterion(idx, { value: number ? number : undefined }); - }} + + + + setIsFieldPopoverOpen(true)} /> - ) : ( - setIsFieldPopoverOpen(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ {criterionFieldTitle} + 0} error={errors.field}> + + +
+ + + + updateCriterion(idx, { value: e.target.value })} + isActive={isComparatorPopoverOpen} + color={ + errors.comparator.length === 0 && errors.value.length === 0 + ? 'secondary' + : 'danger' + } + onClick={() => setIsComparatorPopoverOpen(true)} /> - )} - -
- + } + isOpen={isComparatorPopoverOpen} + closePopover={() => setIsComparatorPopoverOpen(false)} + ownFocus + panelPaddingSize="s" + anchorPosition="downLeft" + > +
+ {criterionComparatorValueTitle} + + + 0} error={errors.comparator}> + + updateCriterion(idx, { comparator: e.target.value as Comparator }) + } + options={compatibleComparatorOptions} + /> + + + + 0} error={errors.value}> + {fieldInfo?.type === 'number' ? ( + { + const number = parseInt(e.target.value, 10); + updateCriterion(idx, { value: number ? number : undefined }); + }} + /> + ) : ( + updateCriterion(idx, { value: e.target.value })} + /> + )} + + + +
+ + + {canDelete && ( From 7942328f303cf4ae5a1adfec0a692b35b8b3d23f Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Thu, 23 Apr 2020 14:06:31 +0100 Subject: [PATCH 21/25] Add hrefOnly option to useLinkProps --- .../components/alerting/logs/alert_dropdown.tsx | 13 +++++++++---- .../plugins/infra/public/hooks/use_link_props.tsx | 15 +++++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx b/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx index ba5fb09e9de2d..dd888639b6d07 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/alert_dropdown.tsx @@ -13,10 +13,15 @@ import { useLinkProps } from '../../../hooks/use_link_props'; export const AlertDropdown = () => { const [popoverOpen, setPopoverOpen] = useState(false); const [flyoutVisible, setFlyoutVisible] = useState(false); - const manageAlertsLinkProps = useLinkProps({ - app: 'kibana', - hash: 'management/kibana/triggersActions/alerts', - }); + const manageAlertsLinkProps = useLinkProps( + { + app: 'kibana', + hash: 'management/kibana/triggersActions/alerts', + }, + { + hrefOnly: true, + } + ); const closePopover = useCallback(() => { setPopoverOpen(false); diff --git a/x-pack/plugins/infra/public/hooks/use_link_props.tsx b/x-pack/plugins/infra/public/hooks/use_link_props.tsx index 8c522bb7fa764..dec8eaae56f41 100644 --- a/x-pack/plugins/infra/public/hooks/use_link_props.tsx +++ b/x-pack/plugins/infra/public/hooks/use_link_props.tsx @@ -26,12 +26,20 @@ interface LinkProps { onClick?: (e: React.MouseEvent | React.MouseEvent) => void; } -export const useLinkProps = ({ app, pathname, hash, search }: LinkDescriptor): LinkProps => { +interface Options { + hrefOnly?: boolean; +} + +export const useLinkProps = ( + { app, pathname, hash, search }: LinkDescriptor, + options: Options = {} +): LinkProps => { validateParams({ app, pathname, hash, search }); const { prompt } = useNavigationWarningPrompt(); const prefixer = usePrefixPathWithBasepath(); const navigateToApp = useKibana().services.application?.navigateToApp; + const { hrefOnly } = options; const encodedSearch = useMemo(() => { return search ? encodeSearch(search) : undefined; @@ -86,7 +94,10 @@ export const useLinkProps = ({ app, pathname, hash, search }: LinkDescriptor): L return { href, - onClick, + // Sometimes it may not be desirable to have onClick call "navigateToApp". + // E.g. the management section of Kibana cannot be successfully deeplinked to via + // "navigateToApp". In those cases we can choose to defer to legacy behaviour. + onClick: hrefOnly ? undefined : onClick, }; }; From b3ee986e1531b7dd971b2c7ffdbc6d20cdcbd523 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 27 Apr 2020 11:02:44 +0100 Subject: [PATCH 22/25] Differentiate string / number wording --- x-pack/plugins/infra/common/alerting/logs/types.ts | 10 +++++++++- .../alerting/logs/expression_editor/criterion.tsx | 10 +++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/logs/types.ts b/x-pack/plugins/infra/common/alerting/logs/types.ts index c08ec98aad0c8..54ebfeacddb94 100644 --- a/x-pack/plugins/infra/common/alerting/logs/types.ts +++ b/x-pack/plugins/infra/common/alerting/logs/types.ts @@ -20,6 +20,8 @@ export enum Comparator { NOT_MATCH_PHRASE = 'does not match phrase', } +// Maps our comparators to i18n strings, some comparators have more specific wording +// depending on the field type the comparator is being used with. export const ComparatorToi18nMap = { [Comparator.GT]: i18n.translate('xpack.infra.logs.alerting.comparator.gt', { defaultMessage: 'more than', @@ -34,9 +36,15 @@ export const ComparatorToi18nMap = { defaultMessage: 'less than or equals', }), [Comparator.EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { - defaultMessage: 'equals', + defaultMessage: 'is', }), [Comparator.NOT_EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.notEq', { + defaultMessage: 'is not', + }), + [`${Comparator.EQ}:number`]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { + defaultMessage: 'equals', + }), + [`${Comparator.NOT_EQ}:number`]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { defaultMessage: 'does not equal', }), [Comparator.MATCH]: i18n.translate('xpack.infra.logs.alerting.comparator.match', { diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx index 031b82978eb28..e8cafecd94db1 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/criterion.tsx @@ -59,8 +59,8 @@ const getCompatibleComparatorsForField = (fieldInfo: IFieldType | undefined) => { value: Comparator.GT_OR_EQ, text: ComparatorToi18nMap[Comparator.GT_OR_EQ] }, { value: Comparator.LT, text: ComparatorToi18nMap[Comparator.LT] }, { value: Comparator.LT_OR_EQ, text: ComparatorToi18nMap[Comparator.LT_OR_EQ] }, - { value: Comparator.EQ, text: ComparatorToi18nMap[Comparator.EQ] }, - { value: Comparator.NOT_EQ, text: ComparatorToi18nMap[Comparator.NOT_EQ] }, + { value: Comparator.EQ, text: ComparatorToi18nMap[`${Comparator.EQ}:number`] }, + { value: Comparator.NOT_EQ, text: ComparatorToi18nMap[`${Comparator.NOT_EQ}:number`] }, ]; } else if (fieldInfo?.aggregatable) { return [ @@ -189,7 +189,11 @@ export const Criterion: React.FC = ({ id="criterion-comparator-value" button={ Date: Mon, 27 Apr 2020 11:20:43 +0100 Subject: [PATCH 23/25] Re-throw Error --- .../server/lib/alerting/log_threshold/log_threshold_executor.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index c642779bb1f5e..cdec04ab81a8e 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -61,6 +61,8 @@ export const createLogThresholdExecutor = (alertUUID: string, libs: InfraBackend alertInstance.replaceState({ alertState: AlertStates.ERROR, }); + + throw new Error(e); } }; From e3f201ced87129c7f63a319d51cfebdb222a3114 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 27 Apr 2020 12:01:55 +0100 Subject: [PATCH 24/25] Differentiate singular vs plural for document count --- .../alerting/logs/expression_editor/document_count.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx index 4c35628d2ddb4..308165ce08a9b 100644 --- a/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx +++ b/x-pack/plugins/infra/public/components/alerting/logs/expression_editor/document_count.tsx @@ -28,10 +28,6 @@ const documentCountPrefix = i18n.translate('xpack.infra.logs.alertFlyout.documen defaultMessage: 'when', }); -const documentCountValue = i18n.translate('xpack.infra.logs.alertFlyout.documentCountValue', { - defaultMessage: 'Log entries', -}); - const getComparatorOptions = (): Array<{ value: Comparator; text: string; @@ -55,6 +51,11 @@ export const DocumentCount: React.FC = ({ comparator, value, updateCount, const [isComparatorPopoverOpen, setComparatorPopoverOpenState] = useState(false); const [isValuePopoverOpen, setIsValuePopoverOpen] = useState(false); + const documentCountValue = i18n.translate('xpack.infra.logs.alertFlyout.documentCountValue', { + defaultMessage: '{value, plural, one {log entry} other {log entries}}', + values: { value }, + }); + return ( From 92de5b8fbbc8638e67745152b3f25ba4a50ce254 Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Mon, 27 Apr 2020 13:58:58 +0100 Subject: [PATCH 25/25] Fix duplicate i18n IDs --- x-pack/plugins/infra/common/alerting/logs/types.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/logs/types.ts b/x-pack/plugins/infra/common/alerting/logs/types.ts index 54ebfeacddb94..cbfffbfd8f940 100644 --- a/x-pack/plugins/infra/common/alerting/logs/types.ts +++ b/x-pack/plugins/infra/common/alerting/logs/types.ts @@ -41,12 +41,15 @@ export const ComparatorToi18nMap = { [Comparator.NOT_EQ]: i18n.translate('xpack.infra.logs.alerting.comparator.notEq', { defaultMessage: 'is not', }), - [`${Comparator.EQ}:number`]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { + [`${Comparator.EQ}:number`]: i18n.translate('xpack.infra.logs.alerting.comparator.eqNumber', { defaultMessage: 'equals', }), - [`${Comparator.NOT_EQ}:number`]: i18n.translate('xpack.infra.logs.alerting.comparator.eq', { - defaultMessage: 'does not equal', - }), + [`${Comparator.NOT_EQ}:number`]: i18n.translate( + 'xpack.infra.logs.alerting.comparator.notEqNumber', + { + defaultMessage: 'does not equal', + } + ), [Comparator.MATCH]: i18n.translate('xpack.infra.logs.alerting.comparator.match', { defaultMessage: 'matches', }),