Improve reason message of custom threshold rule by adding data view i…

…nformation (elastic#169414) Resolves elastic#162710 ## Summary This PR improves the custom threshold rule reason message by adding the data view indices and adjusting the reason for multiple aggregations. Previously, for multiple aggregations, we repeat some information that is shared between aggregations, such as interval and group information. Also, this PR improves the reason messages for single aggregation based on the selected aggregation and field, similar to what we currently have in the metric threshold rule. |Previous reason message | New reason message| |---|---| |![image](https://github.com/elastic/kibana/assets/12370520/bb7e0048-3590-48f0-adfe-218618c48782)|![image](https://github.com/elastic/kibana/assets/12370520/7a3d9778-f84b-4bbb-a8e0-a99debfe78d1)| ## 🧪 How to test - Create some custom threshold rules and check the reason message - Single condition (different aggregators and comparators) - With a label for the equation - Without a label - Multiple conditions (different aggregators and comparators) - With a label for the equation - Without a label ### Known issue I created an issue for `is not between` comparator and I wasn't able to genarate an alert for it: elastic#169524
yuliacech · Oct 26, 2023 · c15da16 · c15da16
1 parent d942833
commit c15da16
Show file tree

Hide file tree

Showing 14 changed files with 362 additions and 238 deletions.
diff --git a/...plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts b/...plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.test.ts
@@ -17,10 +17,9 @@ import { LifecycleAlertServices } from '@kbn/rule-registry-plugin/server';
 import { ruleRegistryMocks } from '@kbn/rule-registry-plugin/server/mocks';
 import {
   createMetricThresholdExecutor,
-  FIRED_ACTIONS,
   MetricThresholdAlertContext,
-  NO_DATA_ACTIONS,
 } from './custom_threshold_executor';
+import { FIRED_ACTIONS, NO_DATA_ACTIONS } from './translations';
 import { Evaluation } from './lib/evaluate_rule';
 import type { LogMeta, Logger } from '@kbn/logging';
 import { DEFAULT_FLAPPING_SETTINGS } from '@kbn/alerting-plugin/common';
@@ -237,10 +236,9 @@ describe('The metric threshold alert type', () => {
       await execute(Comparator.GT, [0.75]);
       const { action } = mostRecentAction(instanceID);
       expect(action.group).toBeUndefined();
-      expect(action.reason).toContain('is 1');
-      expect(action.reason).toContain('Alert when > 0.75');
-      expect(action.reason).toContain('test.metric.1');
-      expect(action.reason).toContain('in the last 1 min');
+      expect(action.reason).toBe(
+        'test.metric.1 is 1, above the threshold of 0.75. (duration: 1 min, data view: mockedIndexPattern)'
+      );
     });
   });
 
@@ -997,16 +995,10 @@ describe('The metric threshold alert type', () => {
       const instanceID = '*';
       await execute(Comparator.GT_OR_EQ, [1.0], [3.0]);
       const { action } = mostRecentAction(instanceID);
-      const reasons = action.reason.split('\n');
-      expect(reasons.length).toBe(2);
-      expect(reasons[0]).toContain('test.metric.1');
-      expect(reasons[1]).toContain('test.metric.2');
-      expect(reasons[0]).toContain('is 1');
-      expect(reasons[1]).toContain('is 3');
-      expect(reasons[0]).toContain('Alert when >= 1');
-      expect(reasons[1]).toContain('Alert when >= 3');
-      expect(reasons[0]).toContain('in the last 1 min');
-      expect(reasons[1]).toContain('in the last 1 min');
+      const reasons = action.reason;
+      expect(reasons).toBe(
+        'test.metric.1 is 1, above the threshold of 1; test.metric.2 is 3, above the threshold of 3. (duration: 1 min, data view: mockedIndexPattern)'
+      );
     });
   });
   describe('querying with the count aggregator', () => {

diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/custom_threshold_executor.ts
@@ -7,7 +7,6 @@
 
 import { isEqual } from 'lodash';
 import { TypeOf } from '@kbn/config-schema';
-import { i18n } from '@kbn/i18n';
 import {
   ALERT_ACTION_GROUP,
   ALERT_EVALUATION_VALUES,
@@ -23,11 +22,10 @@ import {
 import { Alert, RuleTypeState } from '@kbn/alerting-plugin/server';
 import { IBasePath, Logger } from '@kbn/core/server';
 import { LifecycleRuleExecutor } from '@kbn/rule-registry-plugin/server';
-import { AlertsLocatorParams, getAlertUrl, TimeUnitChar } from '../../../../common';
-import { createFormatter } from '../../../../common/custom_threshold_rule/formatters';
-import { Comparator } from '../../../../common/custom_threshold_rule/types';
+import { AlertsLocatorParams, getAlertUrl } from '../../../../common';
 import { ObservabilityConfig } from '../../..';
 import { AlertStates, searchConfigurationSchema } from './types';
+import { FIRED_ACTIONS, NO_DATA_ACTIONS } from './translations';
 
 import {
   buildFiredAlertReason,
@@ -45,6 +43,7 @@ import {
   getFormattedGroupBy,
 } from './utils';
 
+import { formatAlertResult } from './lib/format_alert_result';
 import { EvaluatedRuleParams, evaluateRule } from './lib/evaluate_rule';
 import { MissingGroupsRecord } from './lib/check_missing_group';
 import { convertStringsToMissingGroupsRecord } from './lib/convert_strings_to_missing_groups_record';
@@ -64,7 +63,8 @@ export interface MetricThresholdAlertContext extends Record<string, unknown> {
   group?: object;
   reason?: string;
   timestamp: string; // ISO string
-  value?: Array<number | null> | null;
+  // String type is for [NO DATA]
+  value?: Array<number | string | null>;
 }
 
 export const FIRED_ACTIONS_ID = 'custom_threshold.fired';
@@ -240,14 +240,7 @@ export const createMetricThresholdExecutor = ({
 
       let reason;
       if (nextState === AlertStates.ALERT) {
-        reason = alertResults
-          .map((result) =>
-            buildFiredAlertReason({
-              ...formatAlertResult(result[group]),
-              group,
-            })
-          )
-          .join('\n');
+        reason = buildFiredAlertReason(alertResults, group, dataView);
       }
 
       /* NO DATA STATE HANDLING
@@ -383,61 +376,3 @@ export const createMetricThresholdExecutor = ({
       },
     };
   };
-
-export const FIRED_ACTIONS = {
-  id: 'custom_threshold.fired',
-  name: i18n.translate('xpack.observability.customThreshold.rule.alerting.custom_threshold.fired', {
-    defaultMessage: 'Alert',
-  }),
-};
-
-export const NO_DATA_ACTIONS = {
-  id: 'custom_threshold.nodata',
-  name: i18n.translate(
-    'xpack.observability.customThreshold.rule.alerting.custom_threshold.nodata',
-    {
-      defaultMessage: 'No Data',
-    }
-  ),
-};
-
-const formatAlertResult = <AlertResult>(
-  alertResult: {
-    metric: string;
-    currentValue: number | null;
-    threshold: number[];
-    comparator: Comparator;
-    timeSize: number;
-    timeUnit: TimeUnitChar;
-  } & AlertResult
-) => {
-  const { metric, currentValue, threshold, comparator } = alertResult;
-  const noDataValue = i18n.translate(
-    'xpack.observability.customThreshold.rule.alerting.threshold.noDataFormattedValue',
-    { defaultMessage: '[NO DATA]' }
-  );
-
-  if (metric.endsWith('.pct')) {
-    const formatter = createFormatter('percent');
-    return {
-      ...alertResult,
-      currentValue:
-        currentValue !== null && currentValue !== undefined ? formatter(currentValue) : noDataValue,
-      threshold: Array.isArray(threshold)
-        ? threshold.map((v: number) => formatter(v))
-        : formatter(threshold),
-      comparator,
-    };
-  }
-
-  const formatter = createFormatter('highPrecision');
-  return {
-    ...alertResult,
-    currentValue:
-      currentValue !== null && currentValue !== undefined ? formatter(currentValue) : noDataValue,
-    threshold: Array.isArray(threshold)
-      ? threshold.map((v: number) => formatter(v))
-      : formatter(threshold),
-    comparator,
-  };
-};
diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/evaluate_rule.ts
@@ -8,14 +8,26 @@
 import moment from 'moment';
 import { ElasticsearchClient } from '@kbn/core/server';
 import type { Logger } from '@kbn/logging';
-import { MetricExpressionParams } from '../../../../../common/custom_threshold_rule/types';
-import { isCustom } from './metric_expression_params';
+import {
+  Aggregators,
+  CustomMetricExpressionParams,
+  MetricExpressionParams,
+} from '../../../../../common/custom_threshold_rule/types';
 import { AdditionalContext, getIntervalInSeconds } from '../utils';
 import { SearchConfigurationType } from '../custom_threshold_executor';
-import { CUSTOM_EQUATION_I18N, DOCUMENT_COUNT_I18N } from '../messages';
+import {
+  AVERAGE_I18N,
+  CARDINALITY_I18N,
+  CUSTOM_EQUATION_I18N,
+  DOCUMENT_COUNT_I18N,
+  MAX_I18N,
+  MIN_I18N,
+  SUM_I18N,
+} from '../translations';
 import { createTimerange } from './create_timerange';
 import { getData } from './get_data';
 import { checkMissingGroups, MissingGroupsRecord } from './check_missing_group';
+import { isCustom } from './metric_expression_params';
 
 export interface EvaluatedRuleParams {
   criteria: MetricExpressionParams[];
@@ -33,6 +45,26 @@ export type Evaluation = Omit<MetricExpressionParams, 'metric'> & {
   context?: AdditionalContext;
 };
 
+const getMetric = (criterion: CustomMetricExpressionParams) => {
+  if (!criterion.label && criterion.metrics.length === 1) {
+    switch (criterion.metrics[0].aggType) {
+      case Aggregators.COUNT:
+        return DOCUMENT_COUNT_I18N;
+      case Aggregators.AVERAGE:
+        return AVERAGE_I18N(criterion.metrics[0].field!);
+      case Aggregators.MAX:
+        return MAX_I18N(criterion.metrics[0].field!);
+      case Aggregators.MIN:
+        return MIN_I18N(criterion.metrics[0].field!);
+      case Aggregators.CARDINALITY:
+        return CARDINALITY_I18N(criterion.metrics[0].field!);
+      case Aggregators.SUM:
+        return SUM_I18N(criterion.metrics[0].field!);
+    }
+  }
+  return criterion.label || CUSTOM_EQUATION_I18N;
+};
+
 export const evaluateRule = async <Params extends EvaluatedRuleParams = EvaluatedRuleParams>(
   esClient: ElasticsearchClient,
   params: Params,
@@ -104,10 +136,8 @@ export const evaluateRule = async <Params extends EvaluatedRuleParams = Evaluate
             metric:
               criterion.aggType === 'count'
                 ? DOCUMENT_COUNT_I18N
-                : isCustom(criterion) && criterion.label
-                ? criterion.label
-                : criterion.aggType === 'custom'
-                ? CUSTOM_EQUATION_I18N
+                : isCustom(criterion)
+                ? getMetric(criterion)
                 : criterion.metric,
             currentValue: result.value,
             timestamp: moment(calculatedTimerange.end).toISOString(),

diff --git a/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/format_alert_result.ts b/x-pack/plugins/observability/server/lib/rules/custom_threshold/lib/format_alert_result.ts
@@ -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
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { i18n } from '@kbn/i18n';
+import { createFormatter } from '../../../../../common/custom_threshold_rule/formatters';
+import { Evaluation } from './evaluate_rule';
+
+export type FormattedEvaluation = Omit<Evaluation, 'currentValue' | 'threshold'> & {
+  currentValue: string;
+  threshold: string[];
+};
+
+export const formatAlertResult = (evaluationResult: Evaluation): FormattedEvaluation => {
+  const { metric, currentValue, threshold, comparator } = evaluationResult;
+  const noDataValue = i18n.translate(
+    'xpack.observability.customThreshold.rule.alerting.threshold.noDataFormattedValue',
+    { defaultMessage: '[NO DATA]' }
+  );
+
+  if (metric.endsWith('.pct')) {
+    const formatter = createFormatter('percent');
+    return {
+      ...evaluationResult,
+      currentValue:
+        currentValue !== null && currentValue !== undefined ? formatter(currentValue) : noDataValue,
+      threshold: Array.isArray(threshold)
+        ? threshold.map((v: number) => formatter(v))
+        : [formatter(threshold)],
+      comparator,
+    };
+  }
+
+  const formatter = createFormatter('highPrecision');
+  return {
+    ...evaluationResult,
+    currentValue:
+      currentValue !== null && currentValue !== undefined ? formatter(currentValue) : noDataValue,
+    threshold: Array.isArray(threshold)
+      ? threshold.map((v: number) => formatter(v))
+      : [formatter(threshold)],
+    comparator,
+  };
+};