From 7339e3b82e4593b5682197f1855f788bc393fd1a Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Tue, 23 Nov 2021 17:40:42 -0600 Subject: [PATCH] [Metrics Alerts] Handle invalid KQL in filterQuery --- .../infra/common/alerting/metrics/types.ts | 1 + .../inventory/components/expression.tsx | 15 ++++++++----- .../inventory/components/expression_chart.tsx | 2 +- .../inventory/components/validation.tsx | 13 ++++++++++- .../components/expression.tsx | 20 +++++++++++------ .../components/validation.tsx | 14 +++++++++++- .../public/alerting/metric_threshold/types.ts | 2 +- .../inventory_view/hooks/use_snaphot.ts | 2 +- x-pack/plugins/infra/public/utils/kuery.ts | 14 ++++++------ .../server/lib/alerting/common/messages.ts | 8 +++++++ .../inventory_metric_threshold_executor.ts | 20 +++++++++++++++++ .../metric_threshold/lib/evaluate_alert.ts | 4 +++- .../metric_threshold_executor.ts | 22 +++++++++++++++++++ 13 files changed, 112 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/infra/common/alerting/metrics/types.ts b/x-pack/plugins/infra/common/alerting/metrics/types.ts index 9e8935ddb9968..19812a7d37517 100644 --- a/x-pack/plugins/infra/common/alerting/metrics/types.ts +++ b/x-pack/plugins/infra/common/alerting/metrics/types.ts @@ -75,6 +75,7 @@ export interface InventoryMetricConditions { export interface InventoryMetricThresholdParams { criteria: InventoryMetricConditions[]; filterQuery?: string; + filterQueryText?: string; nodeType: InventoryItemType; sourceId?: string; alertOnNoData?: boolean; diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx index 95ea78ac98ee9..12cde43e4b852 100644 --- a/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/inventory/components/expression.tsx @@ -70,6 +70,7 @@ import { useKibanaContextForPlugin } from '../../../hooks/use_kibana'; import { ExpressionChart } from './expression_chart'; const FILTER_TYPING_DEBOUNCE_MS = 500; +export const QUERY_INVALID = Symbol('QUERY_INVALID'); export interface AlertContextMeta { options?: Partial; @@ -84,7 +85,7 @@ type Props = Omit< { criteria: Criteria; nodeType: InventoryItemType; - filterQuery?: string; + filterQuery?: string | symbol; filterQueryText?: string; sourceId: string; alertOnNoData?: boolean; @@ -157,10 +158,14 @@ export const Expressions: React.FC = (props) => { const onFilterChange = useCallback( (filter: any) => { setAlertParams('filterQueryText', filter || ''); - setAlertParams( - 'filterQuery', - convertKueryToElasticSearchQuery(filter, derivedIndexPattern) || '' - ); + try { + setAlertParams( + 'filterQuery', + convertKueryToElasticSearchQuery(filter, derivedIndexPattern, false) || '' + ); + } catch (e) { + setAlertParams('filterQuery', QUERY_INVALID); + } }, [derivedIndexPattern, setAlertParams] ); diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/expression_chart.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/expression_chart.tsx index b1a58a869e011..bbfa724fd363b 100644 --- a/x-pack/plugins/infra/public/alerting/inventory/components/expression_chart.tsx +++ b/x-pack/plugins/infra/public/alerting/inventory/components/expression_chart.tsx @@ -36,7 +36,7 @@ import { useWaffleOptionsContext } from '../../../pages/metrics/inventory_view/h interface Props { expression: InventoryMetricConditions; - filterQuery?: string; + filterQuery?: string | symbol; nodeType: InventoryItemType; sourceId: string; } diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/validation.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/validation.tsx index c8bab76d79a4e..561bb39c6dce7 100644 --- a/x-pack/plugins/infra/public/alerting/inventory/components/validation.tsx +++ b/x-pack/plugins/infra/public/alerting/inventory/components/validation.tsx @@ -13,11 +13,14 @@ import { } from '../../../../server/lib/alerting/inventory_metric_threshold/types'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { ValidationResult } from '../../../../../triggers_actions_ui/public/types'; +import { QUERY_INVALID } from './expression'; export function validateMetricThreshold({ criteria, + filterQuery, }: { criteria: InventoryMetricConditions[]; + filterQuery?: string | symbol; }): ValidationResult { const validationResult = { errors: {} }; const errors: { @@ -34,9 +37,17 @@ export function validateMetricThreshold({ }; metric: string[]; }; - } = {}; + } & { filterQuery?: string[] } = {}; validationResult.errors = errors; + if (filterQuery === QUERY_INVALID) { + errors.filterQuery = [ + i18n.translate('xpack.infra.metrics.alertFlyout.error.invalidFilterQuery', { + defaultMessage: 'Filter query is invalid.', + }), + ]; + } + if (!criteria || !criteria.length) { return validationResult; } diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx index 095ce9f78cdff..443c24a400c77 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx @@ -42,6 +42,7 @@ import { ExpressionChart } from './expression_chart'; import { useKibanaContextForPlugin } from '../../../hooks/use_kibana'; const FILTER_TYPING_DEBOUNCE_MS = 500; +export const QUERY_INVALID = Symbol('QUERY_INVALID'); type Props = Omit< AlertTypeParamsExpressionProps, @@ -117,10 +118,14 @@ export const Expressions: React.FC = (props) => { const onFilterChange = useCallback( (filter: any) => { setAlertParams('filterQueryText', filter); - setAlertParams( - 'filterQuery', - convertKueryToElasticSearchQuery(filter, derivedIndexPattern) || '' - ); + try { + setAlertParams( + 'filterQuery', + convertKueryToElasticSearchQuery(filter, derivedIndexPattern, false) || '' + ); + } catch (e) { + setAlertParams('filterQuery', QUERY_INVALID); + } }, [setAlertParams, derivedIndexPattern] ); @@ -281,15 +286,16 @@ export const Expressions: React.FC = (props) => { }, [alertParams.groupBy]); const redundantFilterGroupBy = useMemo(() => { - if (!alertParams.filterQuery || !groupByFilterTestPatterns) return []; + const { filterQuery } = alertParams; + if (typeof filterQuery !== 'string' || !groupByFilterTestPatterns) return []; return groupByFilterTestPatterns .map(({ groupName, pattern }) => { - if (pattern.test(alertParams.filterQuery!)) { + if (pattern.test(filterQuery)) { return groupName; } }) .filter((g) => typeof g === 'string') as string[]; - }, [alertParams.filterQuery, groupByFilterTestPatterns]); + }, [alertParams, groupByFilterTestPatterns]); return ( <> diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/validation.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/validation.tsx index 69b2f1d1bcc8f..8df313aa1627a 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/validation.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/validation.tsx @@ -13,11 +13,14 @@ import { } from '../../../../server/lib/alerting/metric_threshold/types'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { ValidationResult } from '../../../../../triggers_actions_ui/public/types'; +import { QUERY_INVALID } from './expression'; export function validateMetricThreshold({ criteria, + filterQuery, }: { criteria: MetricExpressionParams[]; + filterQuery?: string | symbol; }): ValidationResult { const validationResult = { errors: {} }; const errors: { @@ -35,9 +38,17 @@ export function validateMetricThreshold({ }; metric: string[]; }; - } = {}; + } & { filterQuery?: string[] } = {}; validationResult.errors = errors; + if (filterQuery === QUERY_INVALID) { + errors.filterQuery = [ + i18n.translate('xpack.infra.metrics.alertFlyout.error.invalidFilterQuery', { + defaultMessage: 'Filter query is invalid.', + }), + ]; + } + if (!criteria || !criteria.length) { return validationResult; } @@ -59,6 +70,7 @@ export function validateMetricThreshold({ threshold1: [], }, metric: [], + filterQuery: [], }; if (!c.aggType) { errors[id].aggField.push( diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts b/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts index dd15faf2b11c3..0d1c85087f33d 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts @@ -57,7 +57,7 @@ export interface ExpressionChartData { export interface AlertParams { criteria: MetricExpression[]; groupBy?: string | string[]; - filterQuery?: string; + filterQuery?: string | symbol; sourceId: string; filterQueryText?: string; alertOnNoData?: boolean; diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_snaphot.ts b/x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_snaphot.ts index f9ebcde4cd301..9d8560ffb4bc4 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_snaphot.ts +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_snaphot.ts @@ -24,7 +24,7 @@ import { } from '../../../../../common/inventory_models/types'; export function useSnapshot( - filterQuery: string | null | undefined, + filterQuery: string | null | symbol | undefined, metrics: Array<{ type: SnapshotMetricType }>, groupBy: SnapshotGroupBy, nodeType: InventoryItemType, diff --git a/x-pack/plugins/infra/public/utils/kuery.ts b/x-pack/plugins/infra/public/utils/kuery.ts index c7528b237cce5..afd40c23af677 100644 --- a/x-pack/plugins/infra/public/utils/kuery.ts +++ b/x-pack/plugins/infra/public/utils/kuery.ts @@ -5,20 +5,20 @@ * 2.0. */ -import { DataViewBase } from '@kbn/es-query'; -import { esKuery } from '../../../../../src/plugins/data/public'; +import { DataViewBase, fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query'; export const convertKueryToElasticSearchQuery = ( kueryExpression: string, - indexPattern: DataViewBase + indexPattern: DataViewBase, + swallowErrors: boolean = true ) => { try { return kueryExpression - ? JSON.stringify( - esKuery.toElasticsearchQuery(esKuery.fromKueryExpression(kueryExpression), indexPattern) - ) + ? JSON.stringify(toElasticsearchQuery(fromKueryExpression(kueryExpression), indexPattern)) : ''; } catch (err) { - return ''; + if (swallowErrors) { + return ''; + } else throw err; } }; diff --git a/x-pack/plugins/infra/server/lib/alerting/common/messages.ts b/x-pack/plugins/infra/server/lib/alerting/common/messages.ts index 23c89abf4a7aa..0a3b41190a088 100644 --- a/x-pack/plugins/infra/server/lib/alerting/common/messages.ts +++ b/x-pack/plugins/infra/server/lib/alerting/common/messages.ts @@ -173,6 +173,14 @@ export const buildErrorAlertReason = (metric: string) => }, }); +export const buildInvalidQueryAlertReason = (filterQueryText: string) => + i18n.translate('xpack.infra.metrics.alerting.threshold.queryErrorAlertReason', { + defaultMessage: 'Alert is using a malformed KQL query: {filterQueryText}', + values: { + filterQueryText, + }, + }); + export const groupActionVariableDescription = i18n.translate( 'xpack.infra.metrics.alerting.groupActionVariableDescription', { diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts index 26f2ecbc10197..b37e3bef8de17 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts @@ -6,6 +6,7 @@ */ import { i18n } from '@kbn/i18n'; +import { fromKueryExpression } from '@kbn/es-query'; import { ALERT_REASON, ALERT_RULE_PARAMS } from '@kbn/rule-data-utils'; import moment from 'moment'; import { first, get, last } from 'lodash'; @@ -31,6 +32,7 @@ import { buildNoDataAlertReason, // buildRecoveredAlertReason, stateToAlertMessage, + buildInvalidQueryAlertReason, } from '../common/messages'; import { evaluateCondition } from './evaluate_condition'; @@ -74,6 +76,24 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = }, }); + if (!params.filterQuery && params.filterQueryText) { + try { + fromKueryExpression(params.filterQueryText); + } catch (e) { + const actionGroupId = FIRED_ACTIONS.id; // Change this to an Error action group when able + const reason = buildInvalidQueryAlertReason(params.filterQueryText); + const alertInstance = alertInstanceFactory('*', reason); + alertInstance.scheduleActions(actionGroupId, { + group: '*', + alertState: stateToAlertMessage[AlertStates.ERROR], + reason, + timestamp: moment().toISOString(), + value: null, + metric: mapToConditionsLookup(criteria, (c) => c.metric), + }); + return {}; + } + } const source = await libs.sources.getSourceConfiguration( savedObjectsClient, sourceId || 'default' diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts index 8991c884336d3..e8910572d4a09 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts @@ -56,7 +56,8 @@ interface CompositeAggregationsResponse { export interface EvaluatedAlertParams { criteria: MetricExpressionParams[]; groupBy: string | undefined | string[]; - filterQuery: string | undefined; + filterQuery?: string; + filterQueryText?: string; shouldDropPartialBuckets?: boolean; } @@ -68,6 +69,7 @@ export const evaluateAlert = { const { criteria, groupBy, filterQuery, shouldDropPartialBuckets } = params; + return Promise.all( criteria.map(async (criterion) => { const currentValues = await getMetric( diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index 0abf4c41e7cc9..63206b9840601 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -7,6 +7,7 @@ import { first, last, isEqual } from 'lodash'; import { i18n } from '@kbn/i18n'; +import { fromKueryExpression } from '@kbn/es-query'; import moment from 'moment'; import { ALERT_REASON } from '@kbn/rule-data-utils'; import { @@ -23,6 +24,7 @@ import { buildNoDataAlertReason, // buildRecoveredAlertReason, stateToAlertMessage, + buildInvalidQueryAlertReason, } from '../common/messages'; import { UNGROUPED_FACTORY_KEY } from '../common/utils'; import { createFormatter } from '../../../../common/formatters'; @@ -85,6 +87,26 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => alertOnGroupDisappear: boolean | undefined; }; + if (!params.filterQuery && params.filterQueryText) { + try { + fromKueryExpression(params.filterQueryText); + } catch (e) { + const timestamp = moment().toISOString(); + const actionGroupId = FIRED_ACTIONS.id; // Change this to an Error action group when able + const reason = buildInvalidQueryAlertReason(params.filterQueryText); + const alertInstance = alertInstanceFactory(UNGROUPED_FACTORY_KEY, reason); + alertInstance.scheduleActions(actionGroupId, { + group: UNGROUPED_FACTORY_KEY, + alertState: stateToAlertMessage[AlertStates.ERROR], + reason, + timestamp, + value: null, + metric: mapToConditionsLookup(criteria, (c) => c.metric), + }); + return { groups: [], groupBy: params.groupBy, filterQuery: params.filterQuery }; + } + } + // For backwards-compatibility, interpret undefined alertOnGroupDisappear as true const alertOnGroupDisappear = _alertOnGroupDisappear !== false;