[Metrics Alerts] Handle invalid KQL in filterQuery

elastic · Nov 23, 2021 · 7339e3b · 7339e3b
1 parent a5d692d
commit 7339e3b
Show file tree

Hide file tree

Showing 13 changed files with 112 additions and 25 deletions.
diff --git 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
@@ -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<InfraWaffleMapOptions>;
@@ -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> = (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
@@ -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
@@ -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
@@ -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<AlertTypeParams & AlertParams, AlertContextMeta>,
@@ -117,10 +118,14 @@ export const Expressions: React.FC<Props> = (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> = (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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...fra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/...fra/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
@@ -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 = <Params extends EvaluatedAlertParams = EvaluatedAle
   timeframe?: { start?: number; end: number }
 ) => {
   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
@@ -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;