Skip to content

Commit

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

This PR addresses the following:

#### Bug fix
#150278 described a discrepancy
between total alert count in alert by type chart and everywhere else on
alerts page. This is due to `event.type` being a multi-select, if an
alert has 3 event types (i.e. creation, info, denied), it is counted 3
times on alert by type graph. This logic is now updated to categorize an
alert once
- if `denied` event type exists, such event count  => `Prevention`
- total alert count - prevention count => `Detection`.

#### UI improvements
- Top alerts chart no longer shows `Other` when number of grouping is
less than 10 per
#150242 (comment)

![image](https://user-images.githubusercontent.com/18648970/217382166-073d2da9-f49d-4bf7-9a08-3795d5948e33.png)
- Changed `EmptyDonutChart`'s background based on dark/light mode 
Before -> After

![image](https://user-images.githubusercontent.com/18648970/217382463-1ef44127-1cdf-4a70-85f2-8c78a612c485.png)
- Loading spinner for donut chart was not showing, it is now fixed

![image](https://user-images.githubusercontent.com/18648970/217382665-93e093e3-119a-4be4-a313-072ef118eec7.png)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
christineweng and kibanamachine committed Feb 8, 2023
1 parent 8a687f2 commit 2846b8c
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 2846b8c

Please sign in to comment.