[ResponseOps][Alerts] Add null-value bucket detection to alerts group…

… aggregations endpoint (elastic#190305) ## Summary - Adds null-value bucket detection to server-side alerts aggregations and marks those groups with a `--` key and `isNullGroup = true`. - Improves alerts grouping types with default aggregations. - Improves documentation ## To verify 1. Temporarily merge [elastic#189958](elastic#189958) into this branch 2. Create a rule that fires alerts in Observability > Alerts (i.e. Custom Threshold, ES Query, ...) 3. Once you start to see some alerts in the Alerts page, toggle the grouped alerts view using the dropdown at the top-right of the table (`Group alerts by: ...`), selecting a custom field that doesn't have a value in alert documents (to find one, open the alert flyout and look at the fields table) 4. Check that the group based on the empty field shows `--` as a title 5. Check that the alerts table in the expanded group panel is filtered correctly ### References Refs [elastic#189958](elastic#189958) ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
PhilippeOberti · Aug 19, 2024 · 0299a7a · 0299a7a
1 parent 965b0a6
commit 0299a7a
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 31 deletions.
diff --git a/packages/kbn-alerts-grouping/index.ts b/packages/kbn-alerts-grouping/index.ts
@@ -7,5 +7,9 @@
  */
 
 export { AlertsGrouping } from './src/components/alerts_grouping';
-export { type AlertsGroupingProps } from './src/types';
+export {
+  type AlertsGroupingProps,
+  type BaseAlertsGroupAggregations,
+  type AlertsGroupAggregationBucket,
+} from './src/types';
 export { useAlertsGroupingState } from './src/contexts/alerts_grouping_context';
diff --git a/packages/kbn-alerts-grouping/src/components/alerts_grouping.tsx b/packages/kbn-alerts-grouping/src/components/alerts_grouping.tsx
@@ -23,7 +23,7 @@ import { i18n } from '@kbn/i18n';
 import { useAlertsDataView } from '@kbn/alerts-ui-shared/src/common/hooks/use_alerts_data_view';
 import useLocalStorage from 'react-use/lib/useLocalStorage';
 import { AlertsGroupingLevel, AlertsGroupingLevelProps } from './alerts_grouping_level';
-import { AlertsGroupingProps } from '../types';
+import type { AlertsGroupingProps, BaseAlertsGroupAggregations } from '../types';
 import {
   AlertsGroupingContextProvider,
   useAlertsGroupingState,
@@ -40,7 +40,10 @@ const NextLevel = ({
   parentGroupingFilter,
   groupingFilters,
   getLevel,
-}: Pick<AlertsGroupingLevelProps, 'children' | 'parentGroupingFilter'> & {
+}: Pick<
+  AlertsGroupingLevelProps<BaseAlertsGroupAggregations>,
+  'children' | 'parentGroupingFilter'
+> & {
   level: number;
   selectedGroups: string[];
   groupingFilters: Filter[];
@@ -56,7 +59,9 @@ const NextLevel = ({
   return children(nextGroupingFilters)!;
 };
 
-const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
+const AlertsGroupingInternal = <T extends BaseAlertsGroupAggregations>(
+  props: AlertsGroupingProps<T>
+) => {
   const {
     groupingId,
     services,
@@ -230,6 +235,8 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
   return getLevel(0, selectedGroups[0]);
 };
 
+const typedMemo: <T>(c: T) => T = memo;
+
 /**
  * A coordinator component to show multiple alert tables grouped by one or more fields
  *
@@ -243,7 +250,7 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
  *
  *
  * return (
- *   <AlertsGrouping
+ *   <AlertsGrouping<YourAggregationsType>
  *     featureIds={[...]}
  *     globalQuery={{ query: ..., language: 'kql' }}
  *     globalFilters={...}
@@ -274,11 +281,25 @@ const AlertsGroupingInternal = (props: AlertsGroupingProps) => {
  *   </AlertsGrouping>
  * );
  * ```
+ *
+ * To define your aggregations result type, extend the `BaseAlertsGroupAggregations` type:
+ *
+ * ```ts
+ * import { BaseAlertsGroupAggregations } from '@kbn/alerts-grouping';
+ *
+ * interface YourAggregationsType extends BaseAlertsGroupAggregations {
+ *   // Your custom aggregations here
+ * }
+ * ```
+ *
+ * Check {@link useGetAlertsGroupAggregationsQuery} for more info on alerts aggregations.
  */
-export const AlertsGrouping = memo((props: AlertsGroupingProps) => {
-  return (
-    <AlertsGroupingContextProvider>
-      <AlertsGroupingInternal {...props} />
-    </AlertsGroupingContextProvider>
-  );
-});
+export const AlertsGrouping = typedMemo(
+  <T extends BaseAlertsGroupAggregations>(props: AlertsGroupingProps<T>) => {
+    return (
+      <AlertsGroupingContextProvider>
+        <AlertsGroupingInternal {...props} />
+      </AlertsGroupingContextProvider>
+    );
+  }
+);
diff --git a/packages/kbn-alerts-grouping/src/components/alerts_grouping_level.tsx b/packages/kbn-alerts-grouping/src/components/alerts_grouping_level.tsx
@@ -18,10 +18,11 @@ import {
   useGetAlertsGroupAggregationsQuery,
   UseGetAlertsGroupAggregationsQueryProps,
 } from '@kbn/alerts-ui-shared';
-import { AlertsGroupingProps } from '../types';
+import { AlertsGroupingProps, BaseAlertsGroupAggregations } from '../types';
 
-export interface AlertsGroupingLevelProps<T extends Record<string, unknown> = {}>
-  extends AlertsGroupingProps<T> {
+export interface AlertsGroupingLevelProps<
+  T extends BaseAlertsGroupAggregations = BaseAlertsGroupAggregations
+> extends AlertsGroupingProps<T> {
   getGrouping: (
     props: Omit<DynamicGroupingProps<T>, 'groupSelector' | 'pagination'>
   ) => ReactElement;
@@ -40,8 +41,9 @@ const DEFAULT_FILTERS: Filter[] = [];
 /**
  * Renders an alerts grouping level
  */
-export const AlertsGroupingLevel = memo(
-  <T extends Record<string, unknown> = {}>({
+const typedMemo: <T>(c: T) => T = memo;
+export const AlertsGroupingLevel = typedMemo(
+  <T extends BaseAlertsGroupAggregations>({
     featureIds,
     defaultFilters = DEFAULT_FILTERS,
     from,

diff --git a/packages/kbn-alerts-grouping/src/types.ts b/packages/kbn-alerts-grouping/src/types.ts
@@ -29,7 +29,9 @@ export interface AlertsGroupingState {
   [groupingId: string]: GroupModel;
 }
 
-export interface AlertsGroupingProps<T extends Record<string, unknown> = {}> {
+export interface AlertsGroupingProps<
+  T extends BaseAlertsGroupAggregations = BaseAlertsGroupAggregations
+> {
   /**
    * The leaf component that will be rendered in the grouping panels
    */
@@ -96,3 +98,26 @@ export interface AlertsGroupingProps<T extends Record<string, unknown> = {}> {
     http: HttpSetup;
   };
 }
+
+export interface AlertsGroupAggregationBucket {
+  key: string;
+  doc_count: number;
+  isNullGroup?: boolean;
+  unitsCount?: {
+    value: number;
+  };
+}
+
+export interface BaseAlertsGroupAggregations {
+  groupByFields: {
+    doc_count_error_upper_bound: number;
+    sum_other_doc_count: number;
+    buckets: AlertsGroupAggregationBucket[];
+  };
+  groupsCount: {
+    value: number;
+  };
+  unitsCount: {
+    value: number;
+  };
+}
diff --git a/packages/kbn-alerts-ui-shared/src/common/hooks/use_get_alerts_group_aggregations_query.ts b/packages/kbn-alerts-ui-shared/src/common/hooks/use_get_alerts_group_aggregations_query.ts
@@ -45,6 +45,8 @@ export interface UseGetAlertsGroupAggregationsQueryProps {
  *
  * The provided `aggregations` are applied within `groupByFields`. Here the `groupByField` runtime
  * field can be used to perform grouping-based aggregations.
+ * `groupByField` buckets computed over a field with a null/absent value are marked with the
+ * `isNullGroup` flag set to true and their key is set to the `--` string.
  *
  * Applies alerting RBAC through featureIds.
  */

diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts
@@ -27,12 +27,18 @@ import {
 } from '@kbn/rule-data-utils';
 
 import {
+  AggregateName,
+  AggregationsAggregate,
+  AggregationsMultiBucketAggregateBase,
   InlineScript,
   MappingRuntimeFields,
   QueryDslQueryContainer,
   SortCombinations,
-} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
-import { RuleTypeParams, PluginStartContract as AlertingStart } from '@kbn/alerting-plugin/server';
+} from '@elastic/elasticsearch/lib/api/types';
+import type {
+  RuleTypeParams,
+  PluginStartContract as AlertingStart,
+} from '@kbn/alerting-plugin/server';
 import {
   ReadOperations,
   AlertingAuthorization,
@@ -279,7 +285,7 @@ export class AlertsClient {
   /**
    * Searches alerts by id or query and audits the results
    */
-  private async searchAlerts({
+  private async searchAlerts<TAggregations = Record<AggregateName, AggregationsAggregate>>({
     id,
     query,
     aggs,
@@ -335,7 +341,7 @@ export class AlertsClient {
         };
       }
 
-      const result = await this.esClient.search<ParsedTechnicalFields>({
+      const result = await this.esClient.search<ParsedTechnicalFields, TAggregations>({
         index: index ?? '.alerts-*',
         ignore_unavailable: true,
         body: queryBody,
@@ -975,7 +981,10 @@ export class AlertsClient {
     }
   }
 
-  public async find<Params extends RuleTypeParams = never>({
+  public async find<
+    Params extends RuleTypeParams = never,
+    TAggregations = Record<AggregateName, AggregationsAggregate>
+  >({
     aggs,
     featureIds,
     index,
@@ -1007,7 +1016,7 @@ export class AlertsClient {
         }
       }
 
-      const alertsSearchResponse = await this.searchAlerts({
+      const alertsSearchResponse = await this.searchAlerts<TAggregations>({
         query,
         aggs,
         _source,
@@ -1036,7 +1045,7 @@ export class AlertsClient {
   /**
    * Performs a `find` query to extract aggregations on alert groups
    */
-  public getGroupAggregations({
+  public async getGroupAggregations({
     featureIds,
     groupByField,
     aggregations,
@@ -1086,7 +1095,10 @@ export class AlertsClient {
         `The number of documents is too high. Paginating through more than ${MAX_PAGINATED_ALERTS} documents is not possible.`
       );
     }
-    return this.find({
+    const searchResult = await this.find<
+      never,
+      { groupByFields: AggregationsMultiBucketAggregateBase<{ key: string }> }
+    >({
       featureIds,
       aggs: {
         groupByFields: {
@@ -1139,6 +1151,20 @@ export class AlertsClient {
       size: 0,
       _source: false,
     });
+    // Replace artificial uuid values with '--' in null-value buckets and mark them with `isNullGroup = true`
+    const groupsAggregation = searchResult.aggregations?.groupByFields;
+    if (groupsAggregation) {
+      const buckets = Array.isArray(groupsAggregation?.buckets)
+        ? groupsAggregation.buckets
+        : Object.values(groupsAggregation?.buckets ?? {});
+      buckets.forEach((bucket) => {
+        if (bucket.key === uniqueValue) {
+          bucket.key = '--';
+          (bucket as { isNullGroup?: boolean }).isNullGroup = true;
+        }
+      });
+    }
+    return searchResult;
   }
 
   public async getAuthorizedAlertsIndices(featureIds: string[]): Promise<string[] | undefined> {

diff --git a/...lugins/rule_registry/server/alert_data_client/tests/get_alerts_group_aggregations.test.ts b/...lugins/rule_registry/server/alert_data_client/tests/get_alerts_group_aggregations.test.ts
@@ -72,7 +72,7 @@ beforeEach(() => {
 describe('getGroupAggregations()', () => {
   test('calls find() with the correct params', async () => {
     const alertsClient = new AlertsClient(alertsClientParams);
-    alertsClient.find = jest.fn();
+    alertsClient.find = jest.fn().mockResolvedValue({ aggregations: {} });
 
     const featureIds = [AlertConsumers.STACK_ALERTS];
     const groupByField = 'kibana.alert.rule.name';
@@ -141,27 +141,57 @@ describe('getGroupAggregations()', () => {
     });
   });
 
+  test('replaces the key of null-value buckets and marks them with the `isNullGroup` flag', async () => {
+    const alertsClient = new AlertsClient(alertsClientParams);
+    alertsClient.find = jest.fn().mockResolvedValue({
+      aggregations: {
+        groupByFields: {
+          buckets: [
+            {
+              key: 'unique-value',
+              doc_count: 1,
+            },
+          ],
+        },
+      },
+    });
+
+    const result = await alertsClient.getGroupAggregations({
+      featureIds: [AlertConsumers.STACK_ALERTS],
+      groupByField: 'kibana.alert.rule.name',
+      aggregations: {},
+      filters: [],
+      pageIndex: 0,
+      pageSize: DEFAULT_ALERTS_GROUP_BY_FIELD_SIZE,
+    });
+
+    const firstBucket = (result.aggregations as any).groupByFields.buckets[0];
+
+    expect(firstBucket.isNullGroup).toBe(true);
+    expect(firstBucket.key).toEqual('--');
+  });
+
   test('rejects with invalid pagination options', async () => {
     const alertsClient = new AlertsClient(alertsClientParams);
 
-    expect(() =>
+    await expect(() =>
       alertsClient.getGroupAggregations({
         featureIds: ['apm', 'infrastructure', 'logs', 'observability', 'slo', 'uptime'],
         groupByField: 'kibana.alert.rule.name',
         pageIndex: 101,
         pageSize: 50,
       })
-    ).toThrowErrorMatchingInlineSnapshot(
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
       `"The provided pageIndex value is too high. The maximum allowed pageIndex value is 100."`
     );
-    expect(() =>
+    await expect(() =>
       alertsClient.getGroupAggregations({
         featureIds: ['apm', 'infrastructure', 'logs', 'observability', 'slo', 'uptime'],
         groupByField: 'kibana.alert.rule.name',
         pageIndex: 10,
         pageSize: 5000,
       })
-    ).toThrowErrorMatchingInlineSnapshot(
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
       `"The number of documents is too high. Paginating through more than 10000 documents is not possible."`
     );
   });