Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Alerts visualization free field selection #120610

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
*/

import { DEFAULT_MAX_TABLE_QUERY_SIZE } from '../../../../../common/constants';
import type { AlertsStackByField } from '../common/types';

export const getAlertsCountQuery = (
stackByField: AlertsStackByField,
stackByField: string,
Copy link
Contributor

@michaelolo24 michaelolo24 Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an ECS fields type? Ignore, this can accept any aggregate-able field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we do, but this field can be any arbitrary field present in the documents, so I think string is as specific as we can be. The types used to derive this prop support that as well.

from: string,
to: string,
additionalFilters: Array<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ import * as i18n from './translations';
import { AlertsCount } from './alerts_count';
import type { AlertsCountAggregation } from './types';
import { DEFAULT_STACK_BY_FIELD } from '../common/config';
import type { AlertsStackByField } from '../common/types';
import { KpiPanel, StackBySelect } from '../common/components';
import { KpiPanel, StackByComboBox } from '../common/components';
import { useInspectButton } from '../common/hooks';

export const DETECTIONS_ALERTS_COUNT_ID = 'detections-alerts-count';
Expand All @@ -39,8 +38,7 @@ export const AlertsCountPanel = memo<AlertsCountPanelProps>(

// create a unique, but stable (across re-renders) query id
const uniqueQueryId = useMemo(() => `${DETECTIONS_ALERTS_COUNT_ID}-${uuid.v4()}`, []);
const [selectedStackByOption, setSelectedStackByOption] =
useState<AlertsStackByField>(DEFAULT_STACK_BY_FIELD);
const [selectedStackByOption, setSelectedStackByOption] = useState(DEFAULT_STACK_BY_FIELD);

// TODO: Once we are past experimental phase this code should be removed
// const fetchMethod = useIsExperimentalFeatureEnabled('ruleRegistryEnabled')
Expand Down Expand Up @@ -99,7 +97,7 @@ export const AlertsCountPanel = memo<AlertsCountPanelProps>(
titleSize="s"
hideSubtitle
>
<StackBySelect selected={selectedStackByOption} onSelect={setSelectedStackByOption} />
<StackByComboBox selected={selectedStackByOption} onSelect={setSelectedStackByOption} />
</HeaderSection>
<AlertsCount
data={alertsData}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import moment from 'moment';

import type { HistogramData, AlertsAggregation, AlertsBucket, AlertsGroupBucket } from './types';
import type { AlertSearchResponse } from '../../../containers/detection_engine/alerts/types';
import type { AlertsStackByField } from '../common/types';

const EMPTY_ALERTS_DATA: HistogramData[] = [];

Expand All @@ -33,7 +32,7 @@ export const formatAlertsData = (alertsData: AlertSearchResponse<{}, AlertsAggre
};

export const getAlertsHistogramQuery = (
stackByField: AlertsStackByField,
stackByField: string,
from: string,
to: string,
additionalFilters: Array<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ jest.mock('react-router-dom', () => {
...originalModule,
createHref: jest.fn(),
useHistory: jest.fn(),
useLocation: jest.fn().mockReturnValue({ pathname: '' }),
};
});

Expand All @@ -37,9 +38,21 @@ jest.mock('../../../../common/lib/kibana/kibana_react', () => {
navigateToApp: mockNavigateToApp,
getUrlForApp: jest.fn(),
},
data: {
search: {
search: jest.fn(),
},
},
uiSettings: {
get: jest.fn(),
},
notifications: {
toasts: {
addWarning: jest.fn(),
addError: jest.fn(),
addSuccess: jest.fn(),
},
},
},
}),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { LinkButton } from '../../../../common/components/links';
import { SecurityPageName } from '../../../../app/types';
import { DEFAULT_STACK_BY_FIELD, PANEL_HEIGHT } from '../common/config';
import type { AlertsStackByField } from '../common/types';
import { KpiPanel, StackBySelect } from '../common/components';
import { KpiPanel, StackByComboBox } from '../common/components';

import { useInspectButton } from '../common/hooks';

Expand Down Expand Up @@ -109,7 +109,7 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
const [isInspectDisabled, setIsInspectDisabled] = useState(false);
const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);
const [totalAlertsObj, setTotalAlertsObj] = useState<AlertsTotal>(defaultTotalAlertsObj);
const [selectedStackByOption, setSelectedStackByOption] = useState<AlertsStackByField>(
const [selectedStackByOption, setSelectedStackByOption] = useState<string>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the <string> here as well, I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💅

onlyField == null ? defaultStackByOption : onlyField
);

Expand Down Expand Up @@ -276,10 +276,12 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
<EuiFlexGroup alignItems="center" gutterSize="none">
<EuiFlexItem grow={false}>
{showStackBy && (
<StackBySelect
selected={selectedStackByOption}
onSelect={setSelectedStackByOption}
/>
<>
<StackByComboBox
selected={selectedStackByOption}
onSelect={setSelectedStackByOption}
/>
</>
)}
{headerChildren != null && headerChildren}
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* 2.0.
*/

import { EuiPanel, EuiSelect } from '@elastic/eui';
import { EuiPanel, EuiComboBox } from '@elastic/eui';
import styled from 'styled-components';
import React, { useCallback } from 'react';
import { PANEL_HEIGHT, MOBILE_PANEL_HEIGHT, alertsStackByOptions } from './config';
import type { AlertsStackByField } from './types';
import React, { useCallback, useMemo } from 'react';
import { PANEL_HEIGHT, MOBILE_PANEL_HEIGHT } from './config';
import { useStackByFields } from './hooks';
import * as i18n from './translations';

export const KpiPanel = styled(EuiPanel)<{ height?: number }>`
Expand All @@ -25,24 +25,45 @@ export const KpiPanel = styled(EuiPanel)<{ height?: number }>`
}
`;
interface StackedBySelectProps {
selected: AlertsStackByField;
onSelect: (selected: AlertsStackByField) => void;
selected: string;
onSelect: (selected: string) => void;
}

export const StackBySelect: React.FC<StackedBySelectProps> = ({ selected, onSelect }) => {
const setSelectedOptionCallback = useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) => {
onSelect(event.target.value as AlertsStackByField);
export const StackByComboBoxWrapper = styled.div`
width: 400px;
`;

export const StackByComboBox: React.FC<StackedBySelectProps> = ({ selected, onSelect }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More a question: I saw that a lot of other modules use React.memo here instead. Is this something that we should do here as well wrt to performance?

const onChange = useCallback(
(options) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is options === selectedOption?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (options && options.length > 0) {
onSelect(options[0].value);
} else {
onSelect('');
}
},
[onSelect]
);

const selectedOptions = useMemo(() => {
return [{ label: selected, value: selected }];
}, [selected]);
const stackOptions = useStackByFields();
const singleSelection = useMemo(() => {
return { asPlainText: true };
}, []);
return (
<EuiSelect
onChange={setSelectedOptionCallback}
options={alertsStackByOptions}
prepend={i18n.STACK_BY_LABEL}
value={selected}
/>
<StackByComboBoxWrapper>
<EuiComboBox
aria-label={i18n.STACK_BY_ARIA_LABEL}
placeholder={i18n.STACK_BY_PLACEHOLDER}
prepend={i18n.STACK_BY_LABEL}
singleSelection={singleSelection}
sortMatchesBy="startsWith"
options={stackOptions}
selectedOptions={selectedOptions}
compressed
onChange={onChange}
/>
</StackByComboBoxWrapper>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,16 @@
* 2.0.
*/

import React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { useInspectButton, UseInspectButtonParams } from './hooks';
import { useInspectButton, UseInspectButtonParams, useStackByFields } from './hooks';
import { mockBrowserFields } from '../../../../common/containers/source/mock';
import { TestProviders } from '../../../../common/mock';

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
return { ...actual, useLocation: jest.fn().mockReturnValue({ pathname: '' }) };
});

describe('hooks', () => {
describe('useInspectButton', () => {
Expand Down Expand Up @@ -43,4 +51,22 @@ describe('hooks', () => {
expect(mockDeleteQuery).toHaveBeenCalledWith({ id: defaultParams.uniqueQueryId });
});
});

describe('useStackByFields', () => {
jest.mock('../../../../common/containers/sourcerer', () => ({
useSourcererDataView: jest.fn().mockReturnValue({ browserFields: mockBrowserFields }),
}));
it('returns only aggregateable fields', () => {
const wrapper = ({ children }: { children: JSX.Element }) => (
<TestProviders>{children}</TestProviders>
);
const { result, unmount } = renderHook(() => useStackByFields(), { wrapper });
const aggregateableFields = result.current;
unmount();
expect(aggregateableFields?.find((field) => field.label === 'agent.id')).toBeTruthy();
expect(
aggregateableFields?.find((field) => field.label === 'nestedField.firstAttributes')
).toBe(undefined);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@
* 2.0.
*/

import { useEffect } from 'react';
import { useEffect, useState, useMemo } from 'react';
import { useLocation } from 'react-router-dom';
import type { EuiComboBoxOptionOption } from '@elastic/eui';
import type { BrowserField } from '../../../../../../timelines/common';
import type { GlobalTimeArgs } from '../../../../common/containers/use_global_time';
import { getScopeFromPath, useSourcererDataView } from '../../../../common/containers/sourcerer';
import { getAllFieldsByName } from '../../../../common/containers/source';

export interface UseInspectButtonParams extends Pick<GlobalTimeArgs, 'setQuery' | 'deleteQuery'> {
response: string;
Expand All @@ -15,6 +20,7 @@ export interface UseInspectButtonParams extends Pick<GlobalTimeArgs, 'setQuery'
uniqueQueryId: string;
loading: boolean;
}

/**
* * Add query to inspect button utility.
* * Delete query from inspect button utility when component unmounts
Expand Down Expand Up @@ -48,3 +54,28 @@ export const useInspectButton = ({
};
}, [setQuery, loading, response, request, refetch, uniqueQueryId, deleteQuery]);
};

function fieldsToOptions(fields: { [fieldName: string]: Partial<BrowserField> }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this to getAggregatableFields

return Object.entries(fields).reduce<EuiComboBoxOptionOption[]>(
(filteredOptions: EuiComboBoxOptionOption[], [key, field]) => {
if (field.aggregatable === true) {
return [...filteredOptions, { label: key, value: key }];
} else {
return filteredOptions;
}
},
[]
);
}

export const useStackByFields = () => {
const { pathname } = useLocation();

const { browserFields } = useSourcererDataView(getScopeFromPath(pathname));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does useSourcererDataView() update during runtime sometimes or can we assume browserFields is static at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I believe it can be changed by a form control present on most pages that allows a user to change the index patter...data views that are being used to generate the data on a page, this hook will be re-evaluated then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

const allFields = useMemo(() => getAllFieldsByName(browserFields), [browserFields]);
const [stackByFieldOptions, setStackByFieldOptions] = useState(() => fieldsToOptions(allFields));
useEffect(() => {
setStackByFieldOptions(fieldsToOptions(allFields));
}, [allFields]);
return useMemo(() => stackByFieldOptions, [stackByFieldOptions]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,17 @@ export const STACK_BY_LABEL = i18n.translate(
defaultMessage: 'Stack by',
}
);

export const STACK_BY_PLACEHOLDER = i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.histogram.stackByOptions.stackByPlaceholder',
{
defaultMessage: 'Select a field to stack by',
}
);

export const STACK_BY_ARIA_LABEL = i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.histogram.stackByOptions.stackByAriaLabel',
{
defaultMessage: 'Stack the alerts histogram by a field value',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { useSourcererDataView } from '../../../common/containers/sourcerer';
import { createStore, State } from '../../../common/store';
import { mockHistory, Router } from '../../../common/mock/router';
import { mockTimelines } from '../../../common/mock/mock_timelines_plugin';
import { mockBrowserFields } from '../../../common/containers/source/mock';

// Test will fail because we will to need to mock some core services to make the test work
// For now let's forget about SiemSearchBar and QueryBar
Expand Down Expand Up @@ -71,6 +72,9 @@ jest.mock('../../../common/lib/kibana', () => {
siem: { crud_alerts: true, read_alerts: true },
},
},
uiSettings: {
get: jest.fn(),
},
timelines: { ...mockTimelines },
data: {
query: {
Expand Down Expand Up @@ -113,6 +117,7 @@ describe('DetectionEnginePageComponent', () => {
(useSourcererDataView as jest.Mock).mockReturnValue({
indicesExist: true,
indexPattern: {},
browserFields: mockBrowserFields,
});
});

Expand Down