[8.8] [Security solution] Grouping count bug (#156206) (#156310)

# Backport This will backport the following commits from `main` to `8.8`: - [[Security solution] Grouping count bug (#156206)](#156206)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Steph Milovic <[email protected]>
elastic · May 1, 2023 · 064a141 · 064a141
1 parent 775569b
commit 064a141
Show file tree

Hide file tree

Showing 10 changed files with 134 additions and 74 deletions.
diff --git a/packages/kbn-securitysolution-grouping/src/components/grouping.mock.tsx b/packages/kbn-securitysolution-grouping/src/components/grouping.mock.tsx
@@ -15,7 +15,7 @@ export const mockGroupingProps = {
   activePage: 0,
   data: {
     groupsCount: {
-      value: 2,
+      value: 3,
     },
     groupByFields: {
       doc_count_error_upper_bound: 0,
@@ -121,7 +121,10 @@ export const mockGroupingProps = {
       ],
     },
     unitsCount: {
-      value: 3,
+      value: 14,
+    },
+    unitsCountWithoutNull: {
+      value: 14,
     },
   },
   groupingId: 'test-grouping-id',

diff --git a/packages/kbn-securitysolution-grouping/src/components/grouping.tsx b/packages/kbn-securitysolution-grouping/src/components/grouping.tsx
@@ -78,20 +78,13 @@ const GroupingComponent = <T,>({
   const [trigger, setTrigger] = useState<Record<string, { state: 'open' | 'closed' | undefined }>>(
     {}
   );
-  const [nullCount, setNullCount] = useState({ unit: 0, group: 0 });
 
-  const unitCount = useMemo(
-    () => (data?.unitsCount?.value ?? 0) + nullCount.unit,
-    [data?.unitsCount?.value, nullCount.unit]
-  );
+  const unitCount = useMemo(() => data?.unitsCount?.value ?? 0, [data?.unitsCount?.value]);
   const unitCountText = useMemo(() => {
     return `${unitCount.toLocaleString()} ${unit && unit(unitCount)}`;
   }, [unitCount, unit]);
 
-  const groupCount = useMemo(
-    () => (data?.groupsCount?.value ?? 0) + nullCount.group,
-    [data?.groupsCount?.value, nullCount.group]
-  );
+  const groupCount = useMemo(() => data?.groupsCount?.value ?? 0, [data?.groupsCount?.value]);
   const groupCountText = useMemo(
     () => `${groupCount.toLocaleString()} ${GROUPS_UNIT(groupCount)}`,
     [groupCount]
@@ -106,9 +99,6 @@ const GroupingComponent = <T,>({
         const nullGroupMessage = isNullGroup
           ? NULL_GROUP(selectedGroup, unit(groupBucket.doc_count))
           : undefined;
-        if (isNullGroup) {
-          setNullCount({ unit: groupBucket.doc_count, group: 1 });
-        }
 
         return (
           <span key={groupKey}>

diff --git a/packages/kbn-securitysolution-grouping/src/components/types.ts b/packages/kbn-securitysolution-grouping/src/components/types.ts
@@ -32,6 +32,9 @@ export interface RootAggregation<T> {
   unitsCount?: {
     value?: number | null;
   };
+  unitsCountWithoutNull?: {
+    value?: number | null;
+  };
 }
 
 export type GroupingFieldTotalAggregation<T> = Record<

diff --git a/packages/kbn-securitysolution-grouping/src/containers/query/index.test.ts b/packages/kbn-securitysolution-grouping/src/containers/query/index.test.ts
@@ -9,6 +9,7 @@
 import type { GroupingQueryArgs } from './types';
 import { getGroupingQuery, parseGroupingQuery } from '.';
 import { getEmptyValue } from './helpers';
+import { GroupingAggregation } from '../../..';
 
 const testProps: GroupingQueryArgs = {
   additionalFilters: [],
@@ -154,43 +155,76 @@ describe('group selector', () => {
       });
     });
   });
-
+  const groupingAggs = {
+    groupByFields: {
+      buckets: [
+        {
+          key: ['20.80.64.28', '20.80.64.28'],
+          key_as_string: '20.80.64.28|20.80.64.28',
+          doc_count: 75,
+        },
+        {
+          key: ['0.0.0.0', '0.0.0.0'],
+          key_as_string: '0.0.0.0|0.0.0.0',
+          doc_count: 75,
+        },
+        {
+          key: ['0.0.0.0', '::'],
+          key_as_string: '0.0.0.0|::',
+          doc_count: 75,
+        },
+      ],
+    },
+    unitsCount: {
+      value: 100,
+    },
+    unitsCountWithoutNull: {
+      value: 100,
+    },
+    groupsCount: {
+      value: 20,
+    },
+  };
   it('parseGroupingQuery finds and flags the null group', () => {
-    const data = [
-      {
-        key: ['20.80.64.28', '20.80.64.28'],
-        key_as_string: '20.80.64.28|20.80.64.28',
-        doc_count: 75,
-      },
-      {
-        key: ['0.0.0.0', '0.0.0.0'],
-        key_as_string: '0.0.0.0|0.0.0.0',
-        doc_count: 75,
-      },
-      {
-        key: ['0.0.0.0', '::'],
-        key_as_string: '0.0.0.0|::',
-        doc_count: 75,
+    const result = parseGroupingQuery(groupingAggs);
+    expect(result).toEqual({
+      groupByFields: {
+        buckets: [
+          {
+            key: ['20.80.64.28'],
+            key_as_string: '20.80.64.28',
+            doc_count: 75,
+          },
+          {
+            key: ['0.0.0.0'],
+            key_as_string: '0.0.0.0',
+            doc_count: 75,
+          },
+          {
+            key: [getEmptyValue()],
+            key_as_string: getEmptyValue(),
+            isNullGroup: true,
+            doc_count: 75,
+          },
+        ],
       },
-    ];
-    const result = parseGroupingQuery(data);
-    expect(result).toEqual([
-      {
-        key: ['20.80.64.28'],
-        key_as_string: '20.80.64.28',
-        doc_count: 75,
+      unitsCount: {
+        value: 100,
       },
-      {
-        key: ['0.0.0.0'],
-        key_as_string: '0.0.0.0',
-        doc_count: 75,
+      unitsCountWithoutNull: {
+        value: 100,
       },
-      {
-        key: [getEmptyValue()],
-        key_as_string: getEmptyValue(),
-        isNullGroup: true,
-        doc_count: 75,
+      groupsCount: {
+        value: 20,
       },
-    ]);
+    });
+  });
+  it('parseGroupingQuery adjust group count when null field group is present', () => {
+    const result: GroupingAggregation<{}> = parseGroupingQuery({
+      ...groupingAggs,
+      unitsCountWithoutNull: { value: 99 },
+    });
+
+    expect(result.groupsCount?.value).toEqual(21);
   });
 });
diff --git a/packages/kbn-securitysolution-grouping/src/containers/query/index.ts b/packages/kbn-securitysolution-grouping/src/containers/query/index.ts
@@ -7,8 +7,7 @@
  */
 
 import { getEmptyValue, getFieldTypeMissingValues } from './helpers';
-import { GroupingBucket } from '../..';
-import { RawBucket } from '../../..';
+import { GroupingAggregation } from '../..';
 import type { GroupingQueryArgs, GroupingQuery } from './types';
 /** The maximum number of groups to render */
 export const DEFAULT_GROUP_BY_FIELD_SIZE = 10;
@@ -84,6 +83,16 @@ export const getGroupingQuery = ({
           : {}),
       },
     },
+
+    unitsCountWithoutNull: { value_count: { field: groupByField } },
+    unitsCount: {
+      value_count: {
+        field: groupByField,
+        missing: getFieldTypeMissingValues(selectedGroupEsTypes)[0],
+      },
+    },
+    groupsCount: { cardinality: { field: groupByField } },
+
     ...(rootAggregations
       ? rootAggregations.reduce((aggObj, subAgg) => Object.assign(aggObj, subAgg), {})
       : {}),
@@ -113,9 +122,12 @@ export const getGroupingQuery = ({
  * @param buckets buckets returned from the grouping query
  */
 export const parseGroupingQuery = <T>(
-  buckets: Array<RawBucket<T>>
-): Array<RawBucket<T> & GroupingBucket> =>
-  buckets.map((group) => {
+  aggs?: GroupingAggregation<T>
+): GroupingAggregation<T> | {} => {
+  if (!aggs) {
+    return {};
+  }
+  const groupByFields = aggs?.groupByFields?.buckets?.map((group) => {
     if (!Array.isArray(group.key)) {
       return group;
     }
@@ -135,3 +147,15 @@ export const parseGroupingQuery = <T>(
           isNullGroup: true,
         };
   });
+
+  return {
+    ...aggs,
+    groupByFields: { buckets: groupByFields },
+    groupsCount: {
+      value:
+        (aggs.unitsCount?.value !== aggs.unitsCountWithoutNull?.value
+          ? (aggs.groupsCount?.value ?? 0) + 1
+          : aggs.groupsCount?.value) ?? 0,
+    },
+  };
+};
diff --git a/...gins/security_solution/public/detections/components/alerts_table/alerts_grouping.test.tsx b/...gins/security_solution/public/detections/components/alerts_table/alerts_grouping.test.tsx
@@ -245,7 +245,17 @@ describe('GroupedAlertsTable', () => {
             },
           },
           groupsCount: { cardinality: { field: 'kibana.alert.rule.name' } },
-          unitsCount: { value_count: { field: 'kibana.alert.rule.name' } },
+          unitsCount: {
+            value_count: {
+              field: 'kibana.alert.rule.name',
+              missing: '-',
+            },
+          },
+          unitsCountWithoutNull: {
+            value_count: {
+              field: 'kibana.alert.rule.name',
+            },
+          },
         },
         query: {
           bool: {

diff --git a/...ugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx b/...ugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx
@@ -193,8 +193,8 @@ export const GroupedSubLevelComponent: React.FC<AlertsTableComponentProps> = ({
     skip: isNoneGroup([selectedGroup]),
   });
 
-  const buckets = useMemo(
-    () => parseGroupingQuery(alertsGroupsData?.aggregations?.groupByFields?.buckets ?? []),
+  const aggs = useMemo(
+    () => parseGroupingQuery(alertsGroupsData?.aggregations),
     [alertsGroupsData]
   );
 
@@ -248,10 +248,7 @@ export const GroupedSubLevelComponent: React.FC<AlertsTableComponentProps> = ({
         activePage: pageIndex,
         data: {
           ...alertsGroupsData?.aggregations,
-          groupByFields: {
-            ...alertsGroupsData?.aggregations?.groupByFields,
-            buckets,
-          },
+          ...aggs,
         },
         groupingLevel,
         inspectButton: inspect,
@@ -268,7 +265,7 @@ export const GroupedSubLevelComponent: React.FC<AlertsTableComponentProps> = ({
       getGrouping,
       pageIndex,
       alertsGroupsData,
-      buckets,
+      aggs,
       groupingLevel,
       inspect,
       loading,

diff --git a/...ion/public/detections/components/alerts_table/grouping_settings/group_panel_renderers.tsx b/...ion/public/detections/components/alerts_table/grouping_settings/group_panel_renderers.tsx
@@ -95,12 +95,7 @@ RuleNameGroupContent.displayName = 'RuleNameGroup';
 
 const HostNameGroupContent = React.memo<{ hostName: string | string[]; nullGroupMessage?: string }>(
   ({ hostName, nullGroupMessage }) => (
-    <EuiFlexGroup
-      data-test-subj="host-name-group-renderer"
-      gutterSize="s"
-      alignItems="center"
-      justifyContent="center"
-    >
+    <EuiFlexGroup data-test-subj="host-name-group-renderer" gutterSize="s" alignItems="center">
       <EuiFlexItem
         grow={false}
         style={{

diff --git a/...olution/public/detections/components/alerts_table/grouping_settings/query_builder.test.ts b/...olution/public/detections/components/alerts_table/grouping_settings/query_builder.test.ts
@@ -45,6 +45,12 @@ describe('getAlertsGroupingQuery', () => {
       _source: false,
       aggs: {
         unitsCount: {
+          value_count: {
+            field: 'kibana.alert.rule.name',
+            missing: '-',
+          },
+        },
+        unitsCountWithoutNull: {
           value_count: {
             field: 'kibana.alert.rule.name',
           },
@@ -197,6 +203,12 @@ describe('getAlertsGroupingQuery', () => {
       _source: false,
       aggs: {
         unitsCount: {
+          value_count: {
+            field: 'process.name',
+            missing: '-',
+          },
+        },
+        unitsCountWithoutNull: {
           value_count: {
             field: 'process.name',
           },

diff --git a/...ity_solution/public/detections/components/alerts_table/grouping_settings/query_builder.ts b/...ity_solution/public/detections/components/alerts_table/grouping_settings/query_builder.ts
@@ -41,14 +41,6 @@ export const getAlertsGroupingQuery = ({
       ? getAggregationsByGroupField(selectedGroup)
       : [],
     pageNumber: pageIndex * pageSize,
-    rootAggregations: [
-      {
-        unitsCount: { value_count: { field: selectedGroup } },
-      },
-      ...(!isNoneGroup([selectedGroup])
-        ? [{ groupsCount: { cardinality: { field: selectedGroup } } }]
-        : []),
-    ],
     runtimeMappings,
     selectedGroupEsTypes,
     size: pageSize,