From 7a0620f132fe34c03c5ffe70cc118d52b5ec4dbd Mon Sep 17 00:00:00 2001 From: Steph Milovic Date: Mon, 1 May 2023 15:58:23 -0600 Subject: [PATCH] [Security solution] Grouping count bug (#156206) --- .../src/components/grouping.mock.tsx | 7 +- .../src/components/grouping.tsx | 14 +-- .../src/components/types.ts | 3 + .../src/containers/query/index.test.ts | 100 ++++++++++++------ .../src/containers/query/index.ts | 34 +++++- .../alerts_table/alerts_grouping.test.tsx | 12 ++- .../alerts_table/alerts_sub_grouping.tsx | 11 +- .../group_panel_renderers.tsx | 7 +- .../grouping_settings/query_builder.test.ts | 12 +++ .../grouping_settings/query_builder.ts | 8 -- 10 files changed, 134 insertions(+), 74 deletions(-) diff --git a/packages/kbn-securitysolution-grouping/src/components/grouping.mock.tsx b/packages/kbn-securitysolution-grouping/src/components/grouping.mock.tsx index 412644910db99..8f06c877d3f95 100644 --- 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 index 8182d204f81dd..74cc899121b91 100644 --- a/packages/kbn-securitysolution-grouping/src/components/grouping.tsx +++ b/packages/kbn-securitysolution-grouping/src/components/grouping.tsx @@ -78,20 +78,13 @@ const GroupingComponent = ({ const [trigger, setTrigger] = useState>( {} ); - 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 = ({ const nullGroupMessage = isNullGroup ? NULL_GROUP(selectedGroup, unit(groupBucket.doc_count)) : undefined; - if (isNullGroup) { - setNullCount({ unit: groupBucket.doc_count, group: 1 }); - } return ( diff --git a/packages/kbn-securitysolution-grouping/src/components/types.ts b/packages/kbn-securitysolution-grouping/src/components/types.ts index 3ed5cc43ff877..955a55d51ef6f 100644 --- 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 { unitsCount?: { value?: number | null; }; + unitsCountWithoutNull?: { + value?: number | null; + }; } export type GroupingFieldTotalAggregation = 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 index c3ab014119db7..b04c21c49d50c 100644 --- 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 index e4440bc50e249..8e29c95be2aa0 100644 --- 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 = ( - buckets: Array> -): Array & GroupingBucket> => - buckets.map((group) => { + aggs?: GroupingAggregation +): GroupingAggregation | {} => { + if (!aggs) { + return {}; + } + const groupByFields = aggs?.groupByFields?.buckets?.map((group) => { if (!Array.isArray(group.key)) { return group; } @@ -135,3 +147,15 @@ export const parseGroupingQuery = ( 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/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.test.tsx b/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.test.tsx index dae31cb56a79b..04a20e3675642 100644 --- a/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.test.tsx +++ b/x-pack/plugins/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/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx b/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx index 6fefc30e556cc..316c0eabe0ab3 100644 --- a/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx @@ -193,8 +193,8 @@ export const GroupedSubLevelComponent: React.FC = ({ 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 = ({ activePage: pageIndex, data: { ...alertsGroupsData?.aggregations, - groupByFields: { - ...alertsGroupsData?.aggregations?.groupByFields, - buckets, - }, + ...aggs, }, groupingLevel, inspectButton: inspect, @@ -268,7 +265,7 @@ export const GroupedSubLevelComponent: React.FC = ({ getGrouping, pageIndex, alertsGroupsData, - buckets, + aggs, groupingLevel, inspect, loading, diff --git a/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/group_panel_renderers.tsx b/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/group_panel_renderers.tsx index 2781b78247cf6..ac703e496d617 100644 --- a/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/group_panel_renderers.tsx +++ b/x-pack/plugins/security_solution/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 }) => ( - + { _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/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/query_builder.ts b/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/query_builder.ts index cde272af8d498..b7503778e2de5 100644 --- a/x-pack/plugins/security_solution/public/detections/components/alerts_table/grouping_settings/query_builder.ts +++ b/x-pack/plugins/security_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,