Skip to content

Commit

Permalink
[8.7] [Security Solution][Bug] Alerts type discrepancy and ui improve…
Browse files Browse the repository at this point in the history
…ments (#150504) (#150649)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Security Solution][Bug] Alerts type discrepancy and ui improvements
(#150504)](#150504)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"christineweng","email":"18648970+christineweng@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-08T22:40:49Z","message":"[Security
Solution][Bug] Alerts type discrepancy and ui improvements
(#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug
fix\r\nhttps://github.com//issues/150278 described a
discrepancy\r\nbetween total alert count in alert by type chart and
everywhere else on\r\nalerts page. This is due to `event.type` being a
multi-select, if an\r\nalert has 3 event types (i.e. creation, info,
denied), it is counted 3\r\ntimes on alert by type graph. This logic is
now updated to categorize an\r\nalert once\r\n- if `denied` event type
exists, such event count => `Prevention`\r\n- total alert count -
prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top
alerts chart no longer shows `Other` when number of grouping is\r\nless
than 10
per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382166-073d2da9-f49d-4bf7-9a08-3795d5948e33.png)\r\n-
Changed `EmptyDonutChart`'s background based on dark/light mode
\r\nBefore ->
After\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382463-1ef44127-1cdf-4a70-85f2-8c78a612c485.png)\r\n-
Loading spinner for donut chart was not showing, it is now
fixed\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382665-93e093e3-119a-4be4-a313-072ef118eec7.png)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat
Hunting","Team: SecuritySolution","Team:Threat
Hunting:Investigations","v8.7.0","v8.8.0"],"number":150504,"url":"https://github.com/elastic/kibana/pull/150504","mergeCommit":{"message":"[Security
Solution][Bug] Alerts type discrepancy and ui improvements
(#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug
fix\r\nhttps://github.com//issues/150278 described a
discrepancy\r\nbetween total alert count in alert by type chart and
everywhere else on\r\nalerts page. This is due to `event.type` being a
multi-select, if an\r\nalert has 3 event types (i.e. creation, info,
denied), it is counted 3\r\ntimes on alert by type graph. This logic is
now updated to categorize an\r\nalert once\r\n- if `denied` event type
exists, such event count => `Prevention`\r\n- total alert count -
prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top
alerts chart no longer shows `Other` when number of grouping is\r\nless
than 10
per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382166-073d2da9-f49d-4bf7-9a08-3795d5948e33.png)\r\n-
Changed `EmptyDonutChart`'s background based on dark/light mode
\r\nBefore ->
After\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382463-1ef44127-1cdf-4a70-85f2-8c78a612c485.png)\r\n-
Loading spinner for donut chart was not showing, it is now
fixed\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382665-93e093e3-119a-4be4-a313-072ef118eec7.png)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/150504","number":150504,"mergeCommit":{"message":"[Security
Solution][Bug] Alerts type discrepancy and ui improvements
(#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug
fix\r\nhttps://github.com//issues/150278 described a
discrepancy\r\nbetween total alert count in alert by type chart and
everywhere else on\r\nalerts page. This is due to `event.type` being a
multi-select, if an\r\nalert has 3 event types (i.e. creation, info,
denied), it is counted 3\r\ntimes on alert by type graph. This logic is
now updated to categorize an\r\nalert once\r\n- if `denied` event type
exists, such event count => `Prevention`\r\n- total alert count -
prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top
alerts chart no longer shows `Other` when number of grouping is\r\nless
than 10
per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382166-073d2da9-f49d-4bf7-9a08-3795d5948e33.png)\r\n-
Changed `EmptyDonutChart`'s background based on dark/light mode
\r\nBefore ->
After\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382463-1ef44127-1cdf-4a70-85f2-8c78a612c485.png)\r\n-
Loading spinner for donut chart was not showing, it is now
fixed\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/217382665-93e093e3-119a-4be4-a313-072ef118eec7.png)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe"}}]}]
BACKPORT-->

Co-authored-by: christineweng <18648970+christineweng@users.noreply.github.com>
  • Loading branch information
kibanamachine and christineweng authored Feb 9, 2023
1 parent 346d7b6 commit 179034c
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
import React from 'react';
import styled from 'styled-components';
import { useEuiBackgroundColor } from '@elastic/eui';

interface DonutChartEmptyProps {
size?: number;
Expand All @@ -29,7 +30,7 @@ const SmallRing = styled.div<DonutChartEmptyProps>`
${({ size }) => `
height: ${size}px;
width: ${size}px;
background-color: white;
background-color: ${useEuiBackgroundColor('plain')};
display: inline-block;
vertical-align: middle;`}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { AlertsTypeData, AlertType } from './types';
import { DefaultDraggable } from '../../../../common/components/draggables';
import { FormattedCount } from '../../../../common/components/formatted_number';
import { ALERTS_HEADERS_RULE_NAME } from '../../alerts_table/translations';
import { ALERT_TYPE_COLOR } from './helpers';
import { ALERT_TYPE_COLOR, ALERT_TYPE_LABEL } from './helpers';
import { COUNT_TABLE_TITLE } from '../alerts_count_panel/translations';
import * as i18n from './translations';

Expand Down Expand Up @@ -46,7 +46,7 @@ export const getAlertsTypeTableColumns = (): Array<EuiBasicTableColumn<AlertsTyp
return (
<EuiHealth color={ALERT_TYPE_COLOR[type as AlertType]}>
<EuiText grow={false} size="xs">
{type}
{ALERT_TYPE_LABEL[type as AlertType]}
</EuiText>
</EuiHealth>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@ import { has } from 'lodash';
import type { AlertType, AlertsByTypeAgg, AlertsTypeData } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';
import type { SummaryChartsData, SummaryChartsAgg } from '../alerts_summary_charts_panel/types';
import { DETECTION, PREVENTION } from './translations';

export const ALERT_TYPE_COLOR = {
Detection: '#D36086',
Prevention: '#54B399',
};
export const ALERT_TYPE_LABEL = {
Detection: DETECTION,
Prevention: PREVENTION,
};

export const parseAlertsTypeData = (
response: AlertSearchResponse<{}, AlertsByTypeAgg>
Expand All @@ -22,40 +27,44 @@ export const parseAlertsTypeData = (
? []
: rulesBuckets.flatMap((rule) => {
const events = rule.ruleByEventType?.buckets ?? [];
return getAggregateAlerts(rule.key, events);
return getAlertType(rule.key, rule.doc_count, events);
});
};

const getAggregateAlerts = (
const getAlertType = (
ruleName: string,
ruleCount: number,
ruleEvents: Array<{ key: string; doc_count: number }>
): AlertsTypeData[] => {
let preventions = 0;
let detections = 0;

ruleEvents.map((eventBucket) => {
return eventBucket.key === 'denied'
? (preventions += eventBucket.doc_count)
: (detections += eventBucket.doc_count);
});
const preventions = ruleEvents.find((bucket) => bucket.key === 'denied');
if (!preventions) {
return [
{
rule: ruleName,
type: 'Detection' as AlertType,
value: ruleCount,
color: ALERT_TYPE_COLOR.Detection,
},
];
}

const ret = [];
if (detections > 0) {
if (preventions.doc_count < ruleCount) {
ret.push({
rule: ruleName,
type: 'Detection' as AlertType,
value: detections,
value: ruleCount - preventions.doc_count,
color: ALERT_TYPE_COLOR.Detection,
});
}
if (preventions > 0) {
ret.push({
rule: ruleName,
type: 'Prevention' as AlertType,
value: preventions,
color: ALERT_TYPE_COLOR.Prevention,
});
}

ret.push({
rule: ruleName,
type: 'Prevention' as AlertType,
value: preventions.doc_count,
color: ALERT_TYPE_COLOR.Prevention,
});

return ret;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,17 @@ export const DETECTIONS = i18n.translate(
defaultMessage: 'Detections',
}
);

export const PREVENTION = i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.alertsByType.prevention',
{
defaultMessage: 'Prevention',
}
);

export const DETECTION = i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.alertsByType.detection',
{
defaultMessage: 'Detection',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('parse progress bar data', () => {
expect(res).toEqual(mock.parsedAlerts);
});

test('parse severity without data', () => {
test('parse alerts without data', () => {
const res = parseAlertsGroupingData(
mock.mockAlertsEmptyData as AlertSearchResponse<{}, AlertsByGroupingAgg>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export const parseAlertsGroupingData = (
};
});

topAlerts.push({
key: 'Other',
value: other,
percentage: Math.round((other / total) * 1000) / 10,
label: i18n.OTHER,
});
if (other > 0) {
topAlerts.push({
key: 'Other',
value: other,
percentage: Math.round((other / total) * 1000) / 10,
label: i18n.OTHER,
});
}

return topAlerts;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,4 @@ export const parsedAlerts = [
{ key: 'Host-v5biklvcy8', value: 234, label: 'Host-v5biklvcy8', percentage: 41.1 },
{ key: 'Host-5y1uprxfv2', value: 186, label: 'Host-5y1uprxfv2', percentage: 32.6 },
{ key: 'Host-ssf1mhgy5c', value: 150, label: 'Host-ssf1mhgy5c', percentage: 26.3 },
{ key: 'Other', value: 0, label: 'Other', percentage: 0 },
];
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React, { useCallback, useMemo, useEffect, useState } from 'react';
import React, { useCallback, useMemo } from 'react';
import { isEmpty } from 'lodash/fp';
import { ALERT_SEVERITY } from '@kbn/rule-data-utils';
import styled from 'styled-components';
import { EuiFlexGroup, EuiFlexItem, EuiInMemoryTable, EuiLoadingSpinner } from '@elastic/eui';
import type { SortOrder } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { ShapeTreeNode, ElementClickListener } from '@elastic/charts';
Expand All @@ -17,10 +18,12 @@ import { ChartLabel } from '../../../../overview/components/detection_response/a
import { getSeverityTableColumns } from './columns';
import { getSeverityColor } from './helpers';
import { TOTAL_COUNT_OF_ALERTS } from '../../alerts_table/translations';
import { showInitialLoadingSpinner } from '../alerts_histogram_panel/helpers';

const DONUT_HEIGHT = 150;

const StyledEuiLoadingSpinner = styled(EuiLoadingSpinner)`
margin: auto;
`;
export interface SeverityLevelProps {
data: SeverityData[];
isLoading: boolean;
Expand All @@ -32,7 +35,6 @@ export const SeverityLevelChart: React.FC<SeverityLevelProps> = ({
isLoading,
addFilter,
}) => {
const [isInitialLoading, setIsInitialLoading] = useState(true);
const columns = useMemo(() => getSeverityTableColumns(), []);

const count = useMemo(() => {
Expand Down Expand Up @@ -71,12 +73,6 @@ export const SeverityLevelChart: React.FC<SeverityLevelProps> = ({
[addFilter]
);

useEffect(() => {
if (!showInitialLoadingSpinner({ isInitialLoading, isLoadingAlerts: isLoading })) {
setIsInitialLoading(false);
}
}, [isInitialLoading, isLoading, setIsInitialLoading]);

return (
<EuiFlexGroup gutterSize="s" data-test-subj="severity-level-chart">
<EuiFlexItem>
Expand All @@ -89,8 +85,8 @@ export const SeverityLevelChart: React.FC<SeverityLevelProps> = ({
/>
</EuiFlexItem>
<EuiFlexItem data-test-subj="severity-level-donut">
{isInitialLoading ? (
<EuiLoadingSpinner size="l" />
{isLoading ? (
<StyledEuiLoadingSpinner size="l" />
) : (
<DonutChart
data={data}
Expand Down

0 comments on commit 179034c

Please sign in to comment.