[Alerts] Uses aggregations in RulesClient.aggregate() method (#119852) (#120166)

* Replaces multiple find requests with aggregations * Updates unit tests * Removes commented out code * Removes unused import * Adds muted and enabled aggregations * Updates tests * Updates snapshot * Fixes functional test * Fixes functional test * Review feedback, fixes API tests * Logs audit event and updates tests Co-authored-by: Claudio Procida <claudio.procida@elastic.co>
2025-04-23 17:28:26 -04:00 · 2021-12-01 16:48:59 -05:00 · 2021-12-01 16:48:59 -05:00 · 4529047901
commit 4529047901
parent e94156f456
8 changed files with 271 additions and 84 deletions
--- a/x-pack/plugins/alerting/common/alert.ts
+++ b/x-pack/plugins/alerting/common/alert.ts
@ -54,6 +54,8 @@ export interface AlertAction {

 export interface AlertAggregations {
  alertExecutionStatus: { [status: string]: number };
+  ruleEnabledStatus: { enabled: number; disabled: number };
+  ruleMutedStatus: { muted: number; unmuted: number };
 }

 export interface Alert<Params extends AlertTypeParams = never> {
--- a/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts
+++ b/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts
@ -49,6 +49,14 @@ describe('aggregateRulesRoute', () => {
        pending: 1,
        unknown: 0,
      },
+      ruleEnabledStatus: {
+        disabled: 1,
+        enabled: 40,
+      },
+      ruleMutedStatus: {
+        muted: 2,
+        unmuted: 39,
+      },
    };
    rulesClient.aggregate.mockResolvedValueOnce(aggregateResult);

@ -65,6 +73,10 @@ describe('aggregateRulesRoute', () => {
    expect(await handler(context, req, res)).toMatchInlineSnapshot(`
      Object {
        "body": Object {
+          "rule_enabled_status": Object {
+            "disabled": 1,
+            "enabled": 40,
+          },
          "rule_execution_status": Object {
            "active": 23,
            "error": 2,
@ -72,6 +84,10 @@ describe('aggregateRulesRoute', () => {
            "pending": 1,
            "unknown": 0,
          },
+          "rule_muted_status": Object {
+            "muted": 2,
+            "unmuted": 39,
+          },
        },
      }
    `);
@ -89,6 +105,10 @@ describe('aggregateRulesRoute', () => {

    expect(res.ok).toHaveBeenCalledWith({
      body: {
+        rule_enabled_status: {
+          disabled: 1,
+          enabled: 40,
+        },
        rule_execution_status: {
          ok: 15,
          error: 2,
@ -96,6 +116,10 @@ describe('aggregateRulesRoute', () => {
          pending: 1,
          unknown: 0,
        },
+        rule_muted_status: {
+          muted: 2,
+          unmuted: 39,
+        },
      },
    });
  });
--- a/x-pack/plugins/alerting/server/routes/aggregate_rules.ts
+++ b/x-pack/plugins/alerting/server/routes/aggregate_rules.ts
@ -47,10 +47,14 @@ const rewriteQueryReq: RewriteRequestCase<AggregateOptions> = ({
 });
 const rewriteBodyRes: RewriteResponseCase<AggregateResult> = ({
  alertExecutionStatus,
+  ruleEnabledStatus,
+  ruleMutedStatus,
  ...rest
 }) => ({
  ...rest,
  rule_execution_status: alertExecutionStatus,
+  rule_enabled_status: ruleEnabledStatus,
+  rule_muted_status: ruleMutedStatus,
 });

 export const aggregateRulesRoute = (
--- a/x-pack/plugins/alerting/server/rules_client/audit_events.ts
+++ b/x-pack/plugins/alerting/server/rules_client/audit_events.ts
@ -22,6 +22,7 @@ export enum RuleAuditAction {
  UNMUTE = 'rule_unmute',
  MUTE_ALERT = 'rule_alert_mute',
  UNMUTE_ALERT = 'rule_alert_unmute',
+  AGGREGATE = 'rule_aggregate',
 }

 type VerbsTuple = [string, string, string];
@ -40,6 +41,7 @@ const eventVerbs: Record<RuleAuditAction, VerbsTuple> = {
  rule_unmute: ['unmute', 'unmuting', 'unmuted'],
  rule_alert_mute: ['mute alert of', 'muting alert of', 'muted alert of'],
  rule_alert_unmute: ['unmute alert of', 'unmuting alert of', 'unmuted alert of'],
+  rule_aggregate: ['access', 'accessing', 'accessed'],
 };

 const eventTypes: Record<RuleAuditAction, EcsEventType> = {
@ -56,6 +58,7 @@ const eventTypes: Record<RuleAuditAction, EcsEventType> = {
  rule_unmute: 'change',
  rule_alert_mute: 'change',
  rule_alert_unmute: 'change',
+  rule_aggregate: 'access',
 };

 export interface RuleAuditEventParams {
--- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts
+++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts
@ -93,6 +93,29 @@ export type InvalidateAPIKeyResult =
  | { apiKeysEnabled: false }
  | { apiKeysEnabled: true; result: SecurityPluginInvalidateAPIKeyResult };

+export interface RuleAggregation {
+  status: {
+    buckets: Array<{
+      key: string;
+      doc_count: number;
+    }>;
+  };
+  muted: {
+    buckets: Array<{
+      key: number;
+      key_as_string: string;
+      doc_count: number;
+    }>;
+  };
+  enabled: {
+    buckets: Array<{
+      key: number;
+      key_as_string: string;
+      doc_count: number;
+    }>;
+  };
+}
+
 export interface ConstructorOptions {
  logger: Logger;
  taskManager: TaskManagerStartContract;
@ -150,6 +173,8 @@ interface IndexType {

 export interface AggregateResult {
  alertExecutionStatus: { [status: string]: number };
+  ruleEnabledStatus?: { enabled: number; disabled: number };
+  ruleMutedStatus?: { muted: number; unmuted: number };
 }

 export interface FindResult<Params extends AlertTypeParams> {
@ -646,42 +671,100 @@ export class RulesClient {
  }

  public async aggregate({
-    options: { fields, ...options } = {},
+    options: { fields, filter, ...options } = {},
  }: { options?: AggregateOptions } = {}): Promise<AggregateResult> {
-    // Replace this when saved objects supports aggregations https://github.com/elastic/kibana/pull/64002
-    const alertExecutionStatus = await Promise.all(
-      AlertExecutionStatusValues.map(async (status: string) => {
-        const { filter: authorizationFilter } = await this.authorization.getFindAuthorizationFilter(
-          AlertingAuthorizationEntity.Rule,
-          alertingAuthorizationFilterOpts
-        );
-        const filter = options.filter
-          ? `${options.filter} and alert.attributes.executionStatus.status:(${status})`
-          : `alert.attributes.executionStatus.status:(${status})`;
-        const { total } = await this.unsecuredSavedObjectsClient.find<RawAlert>({
-          ...options,
-          filter:
-            (authorizationFilter && filter
-              ? nodeBuilder.and([
-                  esKuery.fromKueryExpression(filter),
-                  authorizationFilter as KueryNode,
-                ])
-              : authorizationFilter) ?? filter,
-          page: 1,
-          perPage: 0,
-          type: 'alert',
-        });
+    let authorizationTuple;
+    try {
+      authorizationTuple = await this.authorization.getFindAuthorizationFilter(
+        AlertingAuthorizationEntity.Rule,
+        alertingAuthorizationFilterOpts
+      );
+    } catch (error) {
+      this.auditLogger?.log(
+        ruleAuditEvent({
+          action: RuleAuditAction.AGGREGATE,
+          error,
+        })
+      );
+      throw error;
+    }
+    const { filter: authorizationFilter } = authorizationTuple;
+    const resp = await this.unsecuredSavedObjectsClient.find<RawAlert, RuleAggregation>({
+      ...options,
+      filter:
+        (authorizationFilter && filter
+          ? nodeBuilder.and([esKuery.fromKueryExpression(filter), authorizationFilter as KueryNode])
+          : authorizationFilter) ?? filter,
+      page: 1,
+      perPage: 0,
+      type: 'alert',
+      aggs: {
+        status: {
+          terms: { field: 'alert.attributes.executionStatus.status' },
+        },
+        enabled: {
+          terms: { field: 'alert.attributes.enabled' },
+        },
+        muted: {
+          terms: { field: 'alert.attributes.muteAll' },
+        },
+      },
+    });

-        return { [status]: total };
+    if (!resp.aggregations) {
+      // Return a placeholder with all zeroes
+      const placeholder: AggregateResult = {
+        alertExecutionStatus: {},
+        ruleEnabledStatus: {
+          enabled: 0,
+          disabled: 0,
+        },
+        ruleMutedStatus: {
+          muted: 0,
+          unmuted: 0,
+        },
+      };
+
+      for (const key of AlertExecutionStatusValues) {
+        placeholder.alertExecutionStatus[key] = 0;
+      }
+
+      return placeholder;
+    }
+
+    const alertExecutionStatus = resp.aggregations.status.buckets.map(
+      ({ key, doc_count: docCount }) => ({
+        [key]: docCount,
      })
    );

-    return {
+    const ret: AggregateResult = {
      alertExecutionStatus: alertExecutionStatus.reduce(
        (acc, curr: { [status: string]: number }) => Object.assign(acc, curr),
        {}
      ),
    };
+
+    // Fill missing keys with zeroes
+    for (const key of AlertExecutionStatusValues) {
+      if (!ret.alertExecutionStatus.hasOwnProperty(key)) {
+        ret.alertExecutionStatus[key] = 0;
+      }
+    }
+
+    const enabledBuckets = resp.aggregations.enabled.buckets;
+    ret.ruleEnabledStatus = {
+      enabled: enabledBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0,
+      disabled: enabledBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0,
+    };
+
+    const mutedBuckets = resp.aggregations.muted.buckets;
+    ret.ruleMutedStatus = {
+      muted: mutedBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0,
+      unmuted: mutedBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0,
+    };
+
+    return ret;
  }

  public async delete({ id }: { id: string }) {
--- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts
+++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts
@ -14,8 +14,9 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s
 import { actionsAuthorizationMock } from '../../../../actions/server/mocks';
 import { AlertingAuthorization } from '../../authorization/alerting_authorization';
 import { ActionsAuthorization } from '../../../../actions/server';
+import { httpServerMock } from '../../../../../../src/core/server/mocks';
+import { auditServiceMock } from '../../../../security/server/audit/index.mock';
 import { getBeforeSetup, setGlobalDate } from './lib';
-import { AlertExecutionStatusValues } from '../../types';
 import { RecoveredActionGroup } from '../../../common';
 import { RegistryRuleType } from '../../rule_type_registry';

@ -26,6 +27,7 @@ const unsecuredSavedObjectsClient = savedObjectsClientMock.create();
 const encryptedSavedObjects = encryptedSavedObjectsMock.createClient();
 const authorization = alertingAuthorizationMock.create();
 const actionsAuthorization = actionsAuthorizationMock.create();
+const auditLogger = auditServiceMock.create().asScoped(httpServerMock.createKibanaRequest());

 const kibanaVersion = 'v7.10.0';
 const rulesClientParams: jest.Mocked<ConstructorOptions> = {
@ -47,6 +49,7 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {

 beforeEach(() => {
  getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry);
+  (auditLogger.log as jest.Mock).mockClear();
 });

 setGlobalDate();
@ -70,37 +73,36 @@ describe('aggregate()', () => {
    authorization.getFindAuthorizationFilter.mockResolvedValue({
      ensureRuleTypeIsAuthorized() {},
    });
-    unsecuredSavedObjectsClient.find
-      .mockResolvedValueOnce({
-        total: 10,
-        per_page: 0,
-        page: 1,
-        saved_objects: [],
-      })
-      .mockResolvedValueOnce({
-        total: 8,
-        per_page: 0,
-        page: 1,
-        saved_objects: [],
-      })
-      .mockResolvedValueOnce({
-        total: 6,
-        per_page: 0,
-        page: 1,
-        saved_objects: [],
-      })
-      .mockResolvedValueOnce({
-        total: 4,
-        per_page: 0,
-        page: 1,
-        saved_objects: [],
-      })
-      .mockResolvedValueOnce({
-        total: 2,
-        per_page: 0,
-        page: 1,
-        saved_objects: [],
-      });
+    unsecuredSavedObjectsClient.find.mockResolvedValueOnce({
+      total: 30,
+      per_page: 0,
+      page: 1,
+      saved_objects: [],
+      aggregations: {
+        status: {
+          buckets: [
+            { key: 'active', doc_count: 8 },
+            { key: 'error', doc_count: 6 },
+            { key: 'ok', doc_count: 10 },
+            { key: 'pending', doc_count: 4 },
+            { key: 'unknown', doc_count: 2 },
+          ],
+        },
+        enabled: {
+          buckets: [
+            { key: 0, key_as_string: '0', doc_count: 2 },
+            { key: 1, key_as_string: '1', doc_count: 28 },
+          ],
+        },
+        muted: {
+          buckets: [
+            { key: 0, key_as_string: '0', doc_count: 27 },
+            { key: 1, key_as_string: '1', doc_count: 3 },
+          ],
+        },
+      },
+    });
+
    ruleTypeRegistry.list.mockReturnValue(listedTypes);
    authorization.filterByRuleTypeAuthorization.mockResolvedValue(
      new Set([
@ -134,41 +136,82 @@ describe('aggregate()', () => {
          "pending": 4,
          "unknown": 2,
        },
+        "ruleEnabledStatus": Object {
+          "disabled": 2,
+          "enabled": 28,
+        },
+        "ruleMutedStatus": Object {
+          "muted": 3,
+          "unmuted": 27,
+        },
      }
    `);
-    expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(
-      AlertExecutionStatusValues.length
-    );
-    AlertExecutionStatusValues.forEach((status: string, ndx: number) => {
-      expect(unsecuredSavedObjectsClient.find.mock.calls[ndx]).toEqual([
-        {
-          fields: undefined,
-          filter: `alert.attributes.executionStatus.status:(${status})`,
-          page: 1,
-          perPage: 0,
-          type: 'alert',
+    expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(1);
+
+    expect(unsecuredSavedObjectsClient.find.mock.calls[0]).toEqual([
+      {
+        filter: undefined,
+        page: 1,
+        perPage: 0,
+        type: 'alert',
+        aggs: {
+          status: {
+            terms: { field: 'alert.attributes.executionStatus.status' },
+          },
+          enabled: {
+            terms: { field: 'alert.attributes.enabled' },
+          },
+          muted: {
+            terms: { field: 'alert.attributes.muteAll' },
+          },
        },
-      ]);
-    });
+      },
+    ]);
  });

  test('supports filters when aggregating', async () => {
    const rulesClient = new RulesClient(rulesClientParams);
    await rulesClient.aggregate({ options: { filter: 'someTerm' } });

-    expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(
-      AlertExecutionStatusValues.length
-    );
-    AlertExecutionStatusValues.forEach((status: string, ndx: number) => {
-      expect(unsecuredSavedObjectsClient.find.mock.calls[ndx]).toEqual([
-        {
-          fields: undefined,
-          filter: `someTerm and alert.attributes.executionStatus.status:(${status})`,
-          page: 1,
-          perPage: 0,
-          type: 'alert',
+    expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.find.mock.calls[0]).toEqual([
+      {
+        fields: undefined,
+        filter: 'someTerm',
+        page: 1,
+        perPage: 0,
+        type: 'alert',
+        aggs: {
+          status: {
+            terms: { field: 'alert.attributes.executionStatus.status' },
+          },
+          enabled: {
+            terms: { field: 'alert.attributes.enabled' },
+          },
+          muted: {
+            terms: { field: 'alert.attributes.muteAll' },
+          },
        },
-      ]);
-    });
+      },
+    ]);
+  });
+
+  test('logs audit event when not authorized to aggregate rules', async () => {
+    const rulesClient = new RulesClient({ ...rulesClientParams, auditLogger });
+    authorization.getFindAuthorizationFilter.mockRejectedValue(new Error('Unauthorized'));
+
+    await expect(rulesClient.aggregate()).rejects.toThrow();
+    expect(auditLogger.log).toHaveBeenCalledWith(
+      expect.objectContaining({
+        event: expect.objectContaining({
+          action: 'rule_aggregate',
+          outcome: 'failure',
+        }),
+        error: {
+          code: 'Error',
+          message: 'Unauthorized',
+        },
+      })
+    );
  });
 });
--- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts
+++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts
@ -12,10 +12,14 @@ import { AsApiContract, RewriteRequestCase } from '../../../../../actions/common

 const rewriteBodyRes: RewriteRequestCase<AlertAggregations> = ({
  rule_execution_status: alertExecutionStatus,
+  rule_enabled_status: ruleEnabledStatus,
+  rule_muted_status: ruleMutedStatus,
  ...rest
 }: any) => ({
  ...rest,
  alertExecutionStatus,
+  ruleEnabledStatus,
+  ruleMutedStatus,
 });

 export async function loadAlertAggregations({
--- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts
+++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts
@ -26,6 +26,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext)

      expect(response.status).to.eql(200);
      expect(response.body).to.eql({
+        rule_enabled_status: {
+          disabled: 0,
+          enabled: 0,
+        },
        rule_execution_status: {
          ok: 0,
          active: 0,
@ -33,6 +37,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext)
          pending: 0,
          unknown: 0,
        },
+        rule_muted_status: {
+          muted: 0,
+          unmuted: 0,
+        },
      });
    });

@ -93,6 +101,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext)

      expect(reponse.status).to.eql(200);
      expect(reponse.body).to.eql({
+        rule_enabled_status: {
+          disabled: 0,
+          enabled: 7,
+        },
        rule_execution_status: {
          ok: NumOkAlerts,
          active: NumActiveAlerts,
@ -100,6 +112,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext)
          pending: 0,
          unknown: 0,
        },
+        rule_muted_status: {
+          muted: 0,
+          unmuted: 7,
+        },
      });
    });

@ -168,6 +184,14 @@ export default function createAggregateTests({ getService }: FtrProviderContext)
            pending: 0,
            unknown: 0,
          },
+          ruleEnabledStatus: {
+            disabled: 0,
+            enabled: 7,
+          },
+          ruleMutedStatus: {
+            muted: 0,
+            unmuted: 7,
+          },
        });
      });
    });