[Metrics UI] Remove UUID from Alert Instance IDs (elastic#71335)

* [Metrics UI] Use alertId instead of uuid for alertInstanceIds

x-pack/plugins/alerts/README.md

@@ -482,13 +482,15 @@ A schedule is structured such that the key specifies the format you wish to use
 We currently support the _Interval format_ which specifies the interval in seconds, minutes, hours or days at which the alert should execute.
 Example: `{ interval: "10s" }`, `{ interval: "5m" }`, `{ interval: "1h" }`, `{ interval: "1d" }`.
 
-There are plans to support multiple other schedule formats in the near fuiture.
+There are plans to support multiple other schedule formats in the near future.
 
 ## Alert instance factory
 
 **alertInstanceFactory(id)**
 
-One service passed in to alert types is an alert instance factory. This factory creates instances of alerts and must be used in order to execute actions. The id you give to the alert instance factory is a unique identifier to the alert instance (ex: server identifier if the instance is about the server). The instance factory will use this identifier to retrieve the state of previous instances with the same id. These instances support state persisting between alert type execution, but will clear out once the alert instance stops executing.
+One service passed in to alert types is an alert instance factory. This factory creates instances of alerts and must be used in order to execute actions. The `id` you give to the alert instance factory is a unique identifier to the alert instance (ex: server identifier if the instance is about the server). The instance factory will use this identifier to retrieve the state of previous instances with the same `id`. These instances support state persisting between alert type execution, but will clear out once the alert instance stops executing.
+
+Note that the `id` only needs to be unique **within the scope of a specific alert**, not unique across all alerts or alert types. For example, Alert 1 and Alert 2 can both create an alert instance with an `id` of `"a"` without conflicting with one another. But if Alert 1 creates 2 alert instances, then they must be differentiated with `id`s of `"a"` and `"b"`.
 
 This factory returns an instance of `AlertInstance`. The alert instance class has the following methods, note that we have removed the methods that you shouldn't touch.
 

x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts

@@ -29,10 +29,10 @@ interface InventoryMetricThresholdParams {
   alertOnNoData?: boolean;
 }
 
-export const createInventoryMetricThresholdExecutor = (
-  libs: InfraBackendLibs,
-  alertId: string
-) => async ({ services, params }: AlertExecutorOptions) => {
+export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) => async ({
+  services,
+  params,
+}: AlertExecutorOptions) => {
   const {
     criteria,
     filterQuery,
@@ -54,7 +54,7 @@ export const createInventoryMetricThresholdExecutor = (
 
   const inventoryItems = Object.keys(first(results) as any);
   for (const item of inventoryItems) {
-    const alertInstance = services.alertInstanceFactory(`${item}::${alertId}`);
+    const alertInstance = services.alertInstanceFactory(`${item}`);
     // AND logic; all criteria must be across the threshold
     const shouldAlertFire = results.every((result) => result[item].shouldFire);
 

x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts

@@ -5,8 +5,6 @@
  */
 import { i18n } from '@kbn/i18n';
 import { schema } from '@kbn/config-schema';
-import { curry } from 'lodash';
-import uuid from 'uuid';
 import {
   createInventoryMetricThresholdExecutor,
   FIRED_ACTIONS,
@@ -43,7 +41,7 @@ export const registerMetricInventoryThresholdAlertType = (libs: InfraBackendLibs
   defaultActionGroupId: FIRED_ACTIONS.id,
   actionGroups: [FIRED_ACTIONS],
   producer: 'metrics',
-  executor: curry(createInventoryMetricThresholdExecutor)(libs, uuid.v4()),
+  executor: createInventoryMetricThresholdExecutor(libs),
   actionVariables: {
     context: [
       {

x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts

@@ -24,7 +24,7 @@ let persistAlertInstances = false; // eslint-disable-line
 
 describe('The metric threshold alert type', () => {
   describe('querying the entire infrastructure', () => {
-    const instanceID = '*::test';
+    const instanceID = '*';
     const execute = (comparator: Comparator, threshold: number[], sourceId: string = 'default') =>
       executor({
         services,
@@ -118,8 +118,8 @@ describe('The metric threshold alert type', () => {
           ],
         },
       });
-    const instanceIdA = 'a::test';
-    const instanceIdB = 'b::test';
+    const instanceIdA = 'a';
+    const instanceIdB = 'b';
     test('sends an alert when all groups pass the threshold', async () => {
       await execute(Comparator.GT, [0.75]);
       expect(mostRecentAction(instanceIdA).id).toBe(FIRED_ACTIONS.id);
@@ -175,28 +175,28 @@ describe('The metric threshold alert type', () => {
         },
       });
     test('sends an alert when all criteria cross the threshold', async () => {
-      const instanceID = '*::test';
+      const instanceID = '*';
       await execute(Comparator.GT_OR_EQ, [1.0], [3.0]);
       expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
       expect(getState(instanceID).alertState).toBe(AlertStates.ALERT);
     });
     test('sends no alert when some, but not all, criteria cross the threshold', async () => {
-      const instanceID = '*::test';
+      const instanceID = '*';
       await execute(Comparator.LT_OR_EQ, [1.0], [3.0]);
       expect(mostRecentAction(instanceID)).toBe(undefined);
       expect(getState(instanceID).alertState).toBe(AlertStates.OK);
     });
     test('alerts only on groups that meet all criteria when querying with a groupBy parameter', async () => {
-      const instanceIdA = 'a::test';
-      const instanceIdB = 'b::test';
+      const instanceIdA = 'a';
+      const instanceIdB = 'b';
       await execute(Comparator.GT_OR_EQ, [1.0], [3.0], 'something');
       expect(mostRecentAction(instanceIdA).id).toBe(FIRED_ACTIONS.id);
       expect(getState(instanceIdA).alertState).toBe(AlertStates.ALERT);
       expect(mostRecentAction(instanceIdB)).toBe(undefined);
       expect(getState(instanceIdB).alertState).toBe(AlertStates.OK);
     });
     test('sends all criteria to the action context', async () => {
-      const instanceID = '*::test';
+      const instanceID = '*';
       await execute(Comparator.GT_OR_EQ, [1.0], [3.0]);
       const { action } = mostRecentAction(instanceID);
       const reasons = action.reason.split('\n');
@@ -210,7 +210,7 @@ describe('The metric threshold alert type', () => {
     });
   });
   describe('querying with the count aggregator', () => {
-    const instanceID = '*::test';
+    const instanceID = '*';
     const execute = (comparator: Comparator, threshold: number[]) =>
       executor({
         services,
@@ -236,7 +236,7 @@ describe('The metric threshold alert type', () => {
     });
   });
   describe('querying with the p99 aggregator', () => {
-    const instanceID = '*::test';
+    const instanceID = '*';
     const execute = (comparator: Comparator, threshold: number[]) =>
       executor({
         services,
@@ -262,7 +262,7 @@ describe('The metric threshold alert type', () => {
     });
   });
   describe('querying with the p95 aggregator', () => {
-    const instanceID = '*::test';
+    const instanceID = '*';
     const execute = (comparator: Comparator, threshold: number[]) =>
       executor({
         services,
@@ -288,7 +288,7 @@ describe('The metric threshold alert type', () => {
     });
   });
   describe("querying a metric that hasn't reported data", () => {
-    const instanceID = '*::test';
+    const instanceID = '*';
     const execute = (alertOnNoData: boolean) =>
       executor({
         services,
@@ -317,9 +317,10 @@ describe('The metric threshold alert type', () => {
   });
 
   // describe('querying a metric that later recovers', () => {
-  //   const instanceID = '*::test';
+  //   const instanceID = '*';
   //   const execute = (threshold: number[]) =>
   //     executor({
+  //
   //       services,
   //       params: {
   //         criteria: [
@@ -377,7 +378,7 @@ const mockLibs: any = {
   configuration: createMockStaticConfiguration({}),
 };
 
-const executor = createMetricThresholdExecutor(mockLibs, 'test') as (opts: {
+const executor = createMetricThresholdExecutor(mockLibs) as (opts: {
   params: AlertExecutorOptions['params'];
   services: { callCluster: AlertExecutorOptions['params']['callCluster'] };
 }) => Promise<void>;

x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts

@@ -17,7 +17,7 @@ import {
 import { AlertStates } from './types';
 import { evaluateAlert } from './lib/evaluate_alert';
 
-export const createMetricThresholdExecutor = (libs: InfraBackendLibs, alertId: string) =>
+export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
   async function (options: AlertExecutorOptions) {
     const { services, params } = options;
     const { criteria } = params;
@@ -36,7 +36,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs, alertId: s
     // Because each alert result has the same group definitions, just grap the groups from the first one.
     const groups = Object.keys(first(alertResults) as any);
     for (const group of groups) {
-      const alertInstance = services.alertInstanceFactory(`${group}::${alertId}`);
+      const alertInstance = services.alertInstanceFactory(`${group}`);
 
       // AND logic; all criteria must be across the threshold
       const shouldAlertFire = alertResults.every((result) =>

x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts

@@ -4,9 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { i18n } from '@kbn/i18n';
-import uuid from 'uuid';
 import { schema } from '@kbn/config-schema';
-import { curry } from 'lodash';
 import { METRIC_EXPLORER_AGGREGATIONS } from '../../../../common/http_api/metrics_explorer';
 import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor';
 import { METRIC_THRESHOLD_ALERT_TYPE_ID, Comparator } from './types';
@@ -107,7 +105,7 @@ export function registerMetricThresholdAlertType(libs: InfraBackendLibs) {
     },
     defaultActionGroupId: FIRED_ACTIONS.id,
     actionGroups: [FIRED_ACTIONS],
-    executor: curry(createMetricThresholdExecutor)(libs, uuid.v4()),
+    executor: createMetricThresholdExecutor(libs),
     actionVariables: {
       context: [
         { name: 'group', description: groupActionVariableDescription },