github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 01:38:56 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution] Improve rules exception flyout opening for the indices with huge amount of fields (#159216)

Browse source 
## Summary

Original ticket:
[#158751](https://github.com/elastic/kibana/issues/158751)

These changes improve the rule's exceptions flyout opening experience.
We had a few complaints that it is very slow to open it and sometimes it
throws an exception about the limited response size.

To fix this, we decided to load extended field's data (conflicts and
unmapped info) only when user selects some field instead of fetching
this data for all fields on flyout opening.

## NOTES:

After these changes we gonna do next steps related to fields loading
when user creates/edits rule exceptions:
1. We will call `_fields_for_wildcard` **WITHOUT**
`include_unmapped=true` parameter to fetch all fields specs on exception
flyout loading
2. We will call `_fields_for_wildcard` **WITH** `include_unmapped=true`
for only one field when user selects it from the dropdown menu

With these changes we will improve slow exception flyout opening when
user has lots of fields which are unmapped in different indices. If for
some reason user has a lot of (thousands) conflicting fields around
indices then the loading is still might be slow as the
`_fields_for_wildcard` call will return conflicts information even
without `include_unmapped=true` parameter.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Ievgen Sorokopud 2023-06-15 14:57:15 +02:00 • committed by GitHub
parent f20705528c
commit 31b34771c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 107 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

15
x-pack/plugins/lists/public/exceptions/components/builder/entry_renderer.test.tsx
View file

@ -197,14 +197,15 @@ describe('BuilderEntryItem', () => {
);
});
test('it render mapping issues warning text when field has mapping conflicts', () => {
test('it render mapping issues warning text when field has mapping conflicts', async () => {
const field = getField('mapping issues');
wrapper = mount(
<BuilderEntryItem
autocompleteService={autocompleteStartMock}
entry={{
correspondingKeywordField: undefined,
entryIndex: 0,
field: getField('mapping issues'),
field,
id: '123',
nested: undefined,
operator: isOperator,
@ -223,12 +224,16 @@ describe('BuilderEntryItem', () => {
setWarningsExist={jest.fn()}
showLabel
allowCustomOptions
getExtendedFields={(): Promise<FieldSpec[]> => Promise.resolve([field])}
/>
);
expect(wrapper.find('.euiFormHelpText.euiFormRow__text').text()).toMatch(
/This field is defined as different types across the following indices or is unmapped. This can cause unexpected query results./
);
await waitFor(() => {
wrapper.update();
expect(wrapper.find('.euiFormHelpText.euiFormRow__text').text()).toMatch(
/This field is defined as different types across the following indices or is unmapped. This can cause unexpected query results./
);
});
});
test('it renders field values correctly when operator is "isOperator"', () => {

41
x-pack/plugins/lists/public/exceptions/components/builder/entry_renderer.tsx
View file

@ -5,7 +5,7 @@
* 2.0.
*/
import React, { useCallback, useMemo } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import {
EuiAccordion,
@ -26,6 +26,7 @@ import {
} from '@kbn/securitysolution-io-ts-list-types';
import {
BuilderEntry,
DataViewField,
EXCEPTION_OPERATORS_ONLY_LISTS,
FormattedBuilderEntry,
OperatorOption,
@ -87,6 +88,7 @@ export interface EntryItemProps {
isDisabled?: boolean;
operatorsList?: OperatorOption[];
allowCustomOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}
export const BuilderEntryItem: React.FC<EntryItemProps> = ({
@ -106,6 +108,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
isDisabled = false,
operatorsList,
allowCustomOptions = false,
getExtendedFields,
}): JSX.Element => {
const sPaddingSize = useEuiPaddingSize('s');
@ -175,6 +178,22 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
[onChange, entry]
);
const [extendedField, setExtendedField] = useState<DataViewField | null>(null);
useEffect(() => {
if (!entry.field?.name) {
setExtendedField(null);
}
const fetchExtendedField = async (): Promise<void> => {
const fieldName = entry.field?.name;
if (getExtendedFields && fieldName) {
const extendedFields = await getExtendedFields([fieldName]);
const field = extendedFields.find((f) => f.name === fieldName) ?? null;
setExtendedField(field);
}
};
fetchExtendedField();
}, [entry.field?.name, getExtendedFields]);
const isFieldComponentDisabled = useMemo(
(): boolean =>
isDisabled ||
@ -212,8 +231,11 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
);
const warningIconCss = { marginRight: `${sPaddingSize}` };
const getMappingConflictsWarning = (field: DataViewFieldBase): React.ReactNode | null => {
const conflictsInfo = getMappingConflictsInfo(field);
const getMappingConflictsWarning = (): React.ReactNode | null => {
if (!extendedField) {
return null;
}
const conflictsInfo = getMappingConflictsInfo(extendedField);
if (!conflictsInfo) {
return null;
}
@ -238,13 +260,13 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
data-test-subj="mappingConflictsAccordion"
>
<div data-test-subj="mappingConflictsDescription">
{conflictsInfo.map((info) => {
{conflictsInfo.map((info, idx) => {
const groupDetails = info.groupedIndices.map(
({ name, count }) =>
`${count > 1 ? i18n.CONFLICT_MULTIPLE_INDEX_DESCRIPTION(name, count) : name}`
);
return (
<>
<EuiFlexItem key={`${idx}`}>
<EuiSpacer size="s" />
{`${
info.totalIndexCount > 1
@ -254,7 +276,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
)
: info.type
}: ${groupDetails.join(', ')}`}
</>
</EuiFlexItem>
);
})}
<EuiSpacer size="s" />
@ -268,12 +290,12 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
entry.nested == null && allowCustomOptions ? i18n.CUSTOM_COMBOBOX_OPTION_TEXT : undefined;
const helpText =
entry.field?.conflictDescriptions == null ? (
extendedField?.conflictDescriptions == null ? (
customOptionText
) : (
<>
{customOptionText}
{getMappingConflictsWarning(entry.field)}
{getMappingConflictsWarning()}
</>
);
return (
@ -295,8 +317,9 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
osTypes,
isDisabled,
handleFieldChange,
sPaddingSize,
allowCustomOptions,
sPaddingSize,
extendedField,
]
);

4
x-pack/plugins/lists/public/exceptions/components/builder/exception_item_renderer.tsx
View file

@ -13,6 +13,7 @@ import { HttpStart } from '@kbn/core/public';
import { ExceptionListType, OsTypeArray } from '@kbn/securitysolution-io-ts-list-types';
import {
BuilderEntry,
DataViewField,
ExceptionsBuilderExceptionItem,
FormattedBuilderEntry,
OperatorOption,
@ -65,6 +66,7 @@ interface BuilderExceptionListItemProps {
isDisabled?: boolean;
operatorsList?: OperatorOption[];
allowCustomOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}
export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionListItemProps>(
@ -88,6 +90,7 @@ export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionList
isDisabled = false,
operatorsList,
allowCustomOptions = false,
getExtendedFields,
}) => {
const handleEntryChange = useCallback(
(entry: BuilderEntry, entryIndex: number): void => {
@ -161,6 +164,7 @@ export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionList
}
operatorsList={operatorsList}
allowCustomOptions={allowCustomOptions}
getExtendedFields={getExtendedFields}
/>
</MyOverflowContainer>
<BuilderEntryDeleteButtonComponent

4
x-pack/plugins/lists/public/exceptions/components/builder/exception_items_renderer.tsx
View file

@ -22,6 +22,7 @@ import {
} from '@kbn/securitysolution-io-ts-list-types';
import {
CreateExceptionListItemBuilderSchema,
DataViewField,
ExceptionsBuilderExceptionItem,
ExceptionsBuilderReturnExceptionItem,
OperatorOption,
@ -98,6 +99,7 @@ export interface ExceptionBuilderProps {
operatorsList?: OperatorOption[];
exceptionItemName?: string;
allowCustomFieldOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}
export const ExceptionBuilderComponent = ({
@ -121,6 +123,7 @@ export const ExceptionBuilderComponent = ({
osTypes,
operatorsList,
allowCustomFieldOptions = false,
getExtendedFields,
}: ExceptionBuilderProps): JSX.Element => {
const [state, dispatch] = useReducer(exceptionsBuilderReducer(), {
...initialState,
@ -442,6 +445,7 @@ export const ExceptionBuilderComponent = ({
isDisabled={isDisabled}
operatorsList={operatorsList}
allowCustomOptions={allowCustomFieldOptions}
getExtendedFields={getExtendedFields}
/>
</EuiFlexItem>
</EuiFlexGroup>

24
x-pack/plugins/security_solution/public/common/containers/source/index.tsx
View file

@ -42,11 +42,7 @@ export const getAllFieldsByName = (
keyBy('name', getAllBrowserFields(browserFields));
export const getIndexFields = memoizeOne(
(
title: string,
fields: IIndexPatternFieldList,
_includeUnmapped: boolean = false
): DataViewBase =>
(title: string, fields: IIndexPatternFieldList): DataViewBase =>
fields && fields.length > 0
? {
fields: fields.map((field) =>
@ -66,10 +62,7 @@ export const getIndexFields = memoizeOne(
title,
}
: { fields: [], title },
(newArgs, lastArgs) =>
newArgs[0] === lastArgs[0] &&
newArgs[1].length === lastArgs[1].length &&
newArgs[2] === lastArgs[2]
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1].length === lastArgs[1].length
);
const DEFAULT_BROWSER_FIELDS = {};
@ -96,8 +89,7 @@ interface FetchIndexReturn {
export const useFetchIndex = (
indexNames: string[],
onlyCheckIfIndicesExist: boolean = false,
strategy: 'indexFields' | 'dataView' | typeof ENDPOINT_FIELDS_SEARCH_STRATEGY = 'indexFields',
includeUnmapped: boolean = false
strategy: 'indexFields' | 'dataView' | typeof ENDPOINT_FIELDS_SEARCH_STRATEGY = 'indexFields'
): [boolean, FetchIndexReturn] => {
const { data } = useKibana().services;
const abortCtrl = useRef(new AbortController());
@ -121,11 +113,7 @@ export const useFetchIndex = (
abortCtrl.current = new AbortController();
const dv = await data.dataViews.create({ title: iNames.join(','), allowNoIndex: true });
const dataView = dv.toSpec();
const { browserFields } = getDataViewStateFromIndexFields(
iNames,
dataView.fields,
includeUnmapped
);
const { browserFields } = getDataViewStateFromIndexFields(iNames, dataView.fields);
previousIndexesName.current = dv.getIndexPattern().split(',');
@ -135,7 +123,7 @@ export const useFetchIndex = (
browserFields,
indexes: dv.getIndexPattern().split(','),
indexExists: dv.getIndexPattern().split(',').length > 0,
indexPatterns: getIndexFields(dv.getIndexPattern(), dv.fields, includeUnmapped),
indexPatterns: getIndexFields(dv.getIndexPattern(), dv.fields),
});
} catch (exc) {
setState({
@ -152,7 +140,7 @@ export const useFetchIndex = (
asyncSearch();
},
[addError, data.dataViews, includeUnmapped, indexNames, state]
[addError, data.dataViews, indexNames, state]
);
useEffect(() => {

11
x-pack/plugins/security_solution/public/common/containers/source/use_data_view.tsx
View file

@ -48,11 +48,7 @@ interface DataViewInfo {
* VERY mutatious on purpose to improve the performance of the transform.
*/
export const getDataViewStateFromIndexFields = memoizeOne(
(
_title: string,
fields: DataViewSpec['fields'],
_includeUnmapped: boolean = false
): DataViewInfo => {
(_title: string, fields: DataViewSpec['fields']): DataViewInfo => {
// Adds two dangerous casts to allow for mutations within this function
type DangerCastForMutation = Record<string, {}>;
if (fields == null) {
@ -72,10 +68,7 @@ export const getDataViewStateFromIndexFields = memoizeOne(
return { browserFields: browserFields as DangerCastForBrowserFieldsMutation };
}
},
(newArgs, lastArgs) =>
newArgs[0] === lastArgs[0] &&
newArgs[1]?.length === lastArgs[1]?.length &&
newArgs[2] === lastArgs[2]
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1]?.length === lastArgs[1]?.length
);
export const useDataView = (): {

3
x-pack/plugins/security_solution/public/common/containers/sourcerer/index.tsx
View file

@ -438,8 +438,7 @@ export const useSourcererDataView = (
const browserFields = useCallback(() => {
const { browserFields: dataViewBrowserFields } = getDataViewStateFromIndexFields(
sourcererDataView.patternList.join(','),
sourcererDataView.fields,
false
sourcererDataView.fields
);
return dataViewBrowserFields;
}, [sourcererDataView.fields, sourcererDataView.patternList]);

2
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.test.tsx
View file

@ -85,6 +85,7 @@ describe('When the add exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: false,
indexPatterns: stubIndexPattern,
getExtendedFields: () => Promise.resolve([]),
}));
mockUseSignalIndex.mockImplementation(() => ({
@ -153,6 +154,7 @@ describe('When the add exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: true,
indexPatterns: { fields: [], title: 'foo' },
getExtendedFields: () => Promise.resolve([]),
}));
wrapper = mount(

3
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx
View file

@ -114,7 +114,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
onCancel,
onConfirm,
}: AddExceptionFlyoutProps) {
const { isLoading, indexPatterns } = useFetchIndexPatterns(rules);
const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
const [isSubmitting, submitNewExceptionItems] = useAddNewExceptionItems();
const [isClosingAlerts, closeAlerts] = useCloseAlertsFromExceptions();
const invalidateFetchRuleByIdQuery = useInvalidateFetchRuleByIdQuery();
@ -501,6 +501,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
onFilterIndexPatterns={filterIndexPatterns}
getExtendedFields={getExtendedFields}
/>
{listType !== ExceptionListTypeEnum.ENDPOINT && !sharedListToAddTo?.length && (

2
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.test.tsx
View file

@ -124,6 +124,7 @@ describe('When the edit exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: false,
indexPatterns: stubIndexPattern,
getExtendedFields: () => Promise.resolve([]),
}));
mockUseFindExceptionListReferences.mockImplementation(() => [
false,
@ -168,6 +169,7 @@ describe('When the edit exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: true,
indexPatterns: { fields: [], title: 'foo' },
getExtendedFields: () => Promise.resolve([]),
}));
const wrapper = mount(

3
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx
View file

@ -109,7 +109,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
const rules = useMemo(() => (rule != null ? [rule] : null), [rule]);
const listType = useMemo((): ExceptionListTypeEnum => list.type as ExceptionListTypeEnum, [list]);
const { isLoading, indexPatterns } = useFetchIndexPatterns(rules);
const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
const [isSubmitting, submitEditExceptionItems] = useEditExceptionItems();
const [isClosingAlerts, closeAlerts] = useCloseAlertsFromExceptions();
@ -370,6 +370,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
onFilterIndexPatterns={filterIndexPatterns}
getExtendedFields={getExtendedFields}
/>
{!openedFromListDetailPage && listType === ExceptionListTypeEnum.DETECTION && (
<>

5
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/flyout_components/item_conditions/index.tsx
View file

@ -20,6 +20,7 @@ import type {
} from '@kbn/securitysolution-io-ts-list-types';
import { ExceptionListTypeEnum } from '@kbn/securitysolution-io-ts-list-types';
import type {
DataViewField,
ExceptionsBuilderExceptionItem,
ExceptionsBuilderReturnExceptionItem,
} from '@kbn/securitysolution-list-utils';
@ -89,6 +90,8 @@ interface ExceptionsFlyoutConditionsComponentProps {
type: ExceptionListType,
osTypes?: Array<'linux' | 'macos' | 'windows'> | undefined
) => DataViewBase;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}
const ExceptionsConditionsComponent: React.FC<ExceptionsFlyoutConditionsComponentProps> = ({
@ -105,6 +108,7 @@ const ExceptionsConditionsComponent: React.FC<ExceptionsFlyoutConditionsComponen
onExceptionItemAdd,
onSetErrorExists,
onFilterIndexPatterns,
getExtendedFields,
}): JSX.Element => {
const { http, unifiedSearch } = useKibana().services;
const isEndpointException = useMemo(
@ -267,6 +271,7 @@ const ExceptionsConditionsComponent: React.FC<ExceptionsFlyoutConditionsComponen
onChange: handleBuilderOnChange,
isDisabled: isExceptionBuilderFormDisabled,
allowCustomFieldOptions: !isEndpointException,
getExtendedFields,
})}
</>
);

56
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/logic/use_exception_flyout_data.tsx
View file

@ -5,9 +5,11 @@
* 2.0.
*/
import { useEffect, useState, useMemo } from 'react';
import { useEffect, useState, useMemo, useCallback } from 'react';
import type { DataViewBase } from '@kbn/es-query';
import type { FieldSpec, DataViewSpec } from '@kbn/data-views-plugin/common';
import { useAppToasts } from '../../../common/hooks/use_app_toasts';
import type { Rule } from '../../rule_management/logic/types';
import { useGetInstalledJob } from '../../../common/components/ml/hooks/use_get_jobs';
@ -19,6 +21,7 @@ import * as i18n from '../../../common/containers/source/translations';
export interface ReturnUseFetchExceptionFlyoutData {
isLoading: boolean;
indexPatterns: DataViewBase;
getExtendedFields: (fields: string[]) => Promise<FieldSpec[]>;
}
/**
@ -84,15 +87,14 @@ export const useFetchIndexPatterns = (rules: Rule[] | null): ReturnUseFetchExcep
}
}, [jobs, isMLRule, memoDataViewId, memoNonDataViewIndexPatterns]);
const [isIndexPatternLoading, { indexPatterns: indexIndexPatterns }] = useFetchIndex(
memoRuleIndices,
false,
'indexFields',
true
);
const [
isIndexPatternLoading,
{ indexPatterns: indexIndexPatterns, dataView: indexDataViewSpec },
] = useFetchIndex(memoRuleIndices, false, 'indexFields');
// Data view logic
const [dataViewIndexPatterns, setDataViewIndexPatterns] = useState<DataViewBase | null>(null);
const [dataViewSpec, setDataViewSpec] = useState<DataViewSpec | null>(null);
useEffect(() => {
const fetchSingleDataView = async () => {
// ensure the memoized data view includes a space id, otherwise
@ -101,25 +103,36 @@ export const useFetchIndexPatterns = (rules: Rule[] | null): ReturnUseFetchExcep
if (activeSpaceId !== '' && memoDataViewId) {
setDataViewLoading(true);
const dv = await data.dataViews.get(memoDataViewId);
let fieldsWithUnmappedInfo = null;
try {
fieldsWithUnmappedInfo = await data.dataViews.getFieldsForIndexPattern(dv, {
pattern: '',
includeUnmapped: true,
});
} catch (error) {
addWarning(error, { title: i18n.FETCH_FIELDS_WITH_UNMAPPED_DATA_ERROR });
}
setDataViewLoading(false);
setDataViewIndexPatterns({
...dv,
...(fieldsWithUnmappedInfo ? { fields: fieldsWithUnmappedInfo } : {}),
});
setDataViewIndexPatterns(dv);
setDataViewSpec(dv.toSpec());
}
};
fetchSingleDataView();
}, [memoDataViewId, data.dataViews, setDataViewIndexPatterns, activeSpaceId, addWarning]);
}, [memoDataViewId, data.dataViews, setDataViewIndexPatterns, activeSpaceId]);
// Fetch extended fields information
const getExtendedFields = useCallback(
async (fields: string[]) => {
let extendedFields: FieldSpec[] = [];
const dv = dataViewSpec ?? indexDataViewSpec;
if (!dv) {
return extendedFields;
}
try {
extendedFields = await data.dataViews.getFieldsForIndexPattern(dv, {
pattern: '',
includeUnmapped: true,
fields,
});
} catch (error) {
addWarning(error, { title: i18n.FETCH_FIELDS_WITH_UNMAPPED_DATA_ERROR });
}
return extendedFields;
},
[addWarning, data.dataViews, dataViewSpec, indexDataViewSpec]
);
// Determine whether to use index patterns or data views
const indexPatternsToUse = useMemo(
@ -131,5 +144,6 @@ export const useFetchIndexPatterns = (rules: Rule[] | null): ReturnUseFetchExcep
return {
isLoading: isIndexPatternLoading || mlJobLoading || dataViewLoading,
indexPatterns: indexPatternsToUse,
getExtendedFields,
};
};