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

[Performance][Security Solution][3/4] - Sourcerer performance #212482

Merged
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -13,7 +13,6 @@ import { SecuritySolutionAppWrapper } from '../../common/components/page';

import { HelpMenu } from '../../common/components/help_menu';
import { getScopeFromPath } from '../../sourcerer/containers/sourcerer_paths';
import { useSourcererDataView } from '../../sourcerer/containers';
import { GlobalHeader } from './global_header';
import { ConsoleManager } from '../../management/components/console/components/console_manager';

Expand All @@ -34,13 +33,11 @@ interface HomePageProps {

const HomePageComponent: React.FC<HomePageProps> = ({ children }) => {
const { pathname } = useLocation();
useInitSourcerer(getScopeFromPath(pathname));
const { browserFields } = useInitSourcerer(getScopeFromPath(pathname));
useUrlState();
useUpdateBrowserTitle();
useUpdateExecutionContext();

const { browserFields } = useSourcererDataView(getScopeFromPath(pathname));

// side effect: this will attempt to upgrade the endpoint package if it is not up to date
// this will run when a user navigates to the Security Solution app and when they navigate between
// tabs in the app. This is useful for keeping the endpoint package as up to date as possible until
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { noop } from 'lodash/fp';
import type { Dispatch } from 'react';
import React, { useEffect, useReducer, createContext, useContext } from 'react';
import React, { useEffect, useReducer, createContext, useContext, useMemo } from 'react';

import { useAlertsPrivileges } from '../../containers/detection_engine/alerts/use_alerts_privileges';
import { useSignalIndex } from '../../containers/detection_engine/alerts/use_signal_index';
Expand Down Expand Up @@ -353,19 +353,38 @@ export const useUserInfo = (): State => {
signalIndexMappingOutdated,
]);

return {
loading,
isSignalIndexExists,
isAuthenticated,
hasEncryptionKey,
canUserCRUD,
canUserREAD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexRead,
hasIndexUpdateDelete,
signalIndexName,
signalIndexMappingOutdated,
};
const userInfo = useMemo(
() => ({
loading,
isSignalIndexExists,
isAuthenticated,
hasEncryptionKey,
canUserCRUD,
canUserREAD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexRead,
hasIndexUpdateDelete,
signalIndexName,
signalIndexMappingOutdated,
}),
[
canUserCRUD,
canUserREAD,
hasEncryptionKey,
hasIndexMaintenance,
hasIndexManage,
hasIndexRead,
hasIndexUpdateDelete,
hasIndexWrite,
isAuthenticated,
isSignalIndexExists,
loading,
signalIndexMappingOutdated,
signalIndexName,
]
);

return userInfo;
};
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ export const useSourcererDataView = (
if (selectedDataView == null || missingPatterns.length > 0) {
// old way of fetching indices, legacy timeline
setLegacyPatterns(selectedPatterns);
} else {
} else if (legacyPatterns.length > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sourcerer was unnecessarily re-rendering due to a reference change that took place because of an empty array being replaced with a new empty array. It could also be solved by pulling out a const emptyArray = [] and re-using that but just went with the if check instead...

// Only create a new array reference if legacyPatterns is not empty
setLegacyPatterns([]);
}
}, [missingPatterns, selectedDataView, selectedPatterns]);
}, [legacyPatterns.length, missingPatterns, selectedDataView, selectedPatterns]);

const sourcererDataView = useMemo(() => {
const _dv =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export const useInitSourcerer = (
!loadingSignalIndex &&
signalIndexName != null &&
signalIndexNameSourcerer == null &&
(activeTimeline == null || activeTimeline.savedObjectId == null) &&
activeTimeline?.savedObjectId == null &&
initialTimelineSourcerer.current &&
defaultDataView.id.length > 0
) {
Expand Down Expand Up @@ -209,7 +209,7 @@ export const useInitSourcerer = (
);
} else if (
signalIndexNameSourcerer != null &&
(activeTimeline == null || activeTimeline.savedObjectId == null) &&
activeTimeline?.savedObjectId == null &&
initialTimelineSourcerer.current &&
defaultDataView.id.length > 0
) {
Expand Down Expand Up @@ -240,14 +240,14 @@ export const useInitSourcerer = (
);
}
}, [
activeTimeline,
activeTimeline?.savedObjectId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only care about the id value, don't re call due to activeTimeline ref changes.

defaultDataView,
dispatch,
loadingSignalIndex,
signalIndexName,
signalIndexNameSourcerer,
]);
const { dataViewId } = useSourcererDataView(scopeId);
const { dataViewId, browserFields } = useSourcererDataView(scopeId);

const updateSourcererDataView = useCallback(
(newSignalsIndex: string) => {
Expand Down Expand Up @@ -370,4 +370,11 @@ export const useInitSourcerer = (
signalIndexName,
signalIndexNameSourcerer,
]);

return useMemo(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return browser fields from useInitSourcerer as there was another call to useSourcererDataView in the home component that only returned browser fields...and we already made that call in the useInit, so now we just return it from there instead.

Other change here:
https://github.com/elastic/kibana/pull/212482/files#diff-c46dd8c3c8b81dc65ae1d303e29c3c0e966fcd5e4588731828b46142b18d494aL42

() => ({
browserFields,
}),
[browserFields]
);
};