From 966c91c302078bc3bc8a70b1be76e80863fe1138 Mon Sep 17 00:00:00 2001 From: "Quynh Nguyen (Quinn)" <43350163+qn895@users.noreply.github.com> Date: Wed, 30 Aug 2023 11:50:18 -0500 Subject: [PATCH] [ML] Improve loading time for populated fields in Transform creation wizard (#164790) ## Summary Follow up of https://github.com/elastic/kibana/pull/163371. This PR reduces the extra call in Transform as the same request to fetch 500 sample docs have already been made via the Field Stats provider. It also reduces the number of docs fetched from 1000 to 500 for consistency. If the performance journey improves, we can make the same change to Data Frame Analytics. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../field_stats_flyout_provider.tsx | 2 +- x-pack/plugins/ml/public/shared.ts | 2 + .../public/app/hooks/use_index_data.ts | 63 ++++++++++--------- .../use_search_items/use_search_items.ts | 5 +- .../step_define/step_define_form.tsx | 16 ++++- 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx b/x-pack/plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx index f72fb8d00f173..02c1a2c70aa92 100644 --- a/x-pack/plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx +++ b/x-pack/plugins/ml/public/application/components/field_stats_flyout/field_stats_flyout_provider.tsx @@ -76,7 +76,7 @@ export const FieldStatsFlyoutProvider: FC<{ fields: ['*'], _source: false, ...queryAndRunTimeMappings, - size: 1000, + size: 500, }, }; const cacheKey = stringHash(JSON.stringify(esSearchRequestParams)).toString(); diff --git a/x-pack/plugins/ml/public/shared.ts b/x-pack/plugins/ml/public/shared.ts index 73688aebc94f8..57db3c66a7c3b 100644 --- a/x-pack/plugins/ml/public/shared.ts +++ b/x-pack/plugins/ml/public/shared.ts @@ -16,3 +16,5 @@ export * from '../common/util/validators'; export * from './application/formatters/metric_change_description'; export * from './application/components/field_stats_flyout'; export * from './application/data_frame_analytics/common'; + +export { useFieldStatsFlyoutContext } from './application/components/field_stats_flyout/use_field_stats_flytout_context'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_index_data.ts b/x-pack/plugins/transform/public/app/hooks/use_index_data.ts index 7b6911636c600..3a6781615b70b 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_index_data.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_index_data.ts @@ -51,7 +51,8 @@ export const useIndexData = ( dataView: SearchItems['dataView'], query: TransformConfigQuery, combinedRuntimeMappings?: StepDefineExposedState['runtimeMappings'], - timeRangeMs?: TimeRangeMs + timeRangeMs?: TimeRangeMs, + populatedFields?: Set | null ): UseIndexDataReturnType => { const { analytics } = useAppDependencies(); @@ -96,47 +97,53 @@ export const useIndexData = ( // (for example, as part of filebeat/metricbeat/ECS based indices) // to the data grid component which would significantly slow down the page. const fetchDataGridSampleDocuments = async function () { - setErrorMessage(''); - setStatus(INDEX_STATUS.LOADING); - - const esSearchRequest = { - index: indexPattern, - body: { - fields: ['*'], - _source: false, - query: { - function_score: { - query: defaultQuery, - random_score: {}, + let populatedDataViewFields = populatedFields ? [...populatedFields] : []; + let isMissingFields = populatedDataViewFields.length === 0; + + // If populatedFields are not provided, make own request to calculate + if (populatedFields === undefined) { + setErrorMessage(''); + setStatus(INDEX_STATUS.LOADING); + + const esSearchRequest = { + index: indexPattern, + body: { + fields: ['*'], + _source: false, + query: { + function_score: { + query: defaultQuery, + random_score: {}, + }, }, + size: 500, }, - size: 500, - }, - }; + }; - const resp = await dataSearch(esSearchRequest, abortController.signal); + const resp = await dataSearch(esSearchRequest, abortController.signal); - if (!isEsSearchResponse(resp)) { - setErrorMessage(getErrorMessage(resp)); - setStatus(INDEX_STATUS.ERROR); - return; - } + if (!isEsSearchResponse(resp)) { + setErrorMessage(getErrorMessage(resp)); + setStatus(INDEX_STATUS.ERROR); + return; + } + const docs = resp.hits.hits.map((d) => getProcessedFields(d.fields ?? {})); + isMissingFields = resp.hits.hits.every((d) => typeof d.fields === 'undefined'); + populatedDataViewFields = [...new Set(docs.map(Object.keys).flat(1))]; + } const isCrossClusterSearch = indexPattern.includes(':'); - const isMissingFields = resp.hits.hits.every((d) => typeof d.fields === 'undefined'); - - const docs = resp.hits.hits.map((d) => getProcessedFields(d.fields ?? {})); // Get all field names for each returned doc and flatten it // to a list of unique field names used across all docs. const allDataViewFields = getFieldsFromKibanaIndexPattern(dataView); - const populatedFields = [...new Set(docs.map(Object.keys).flat(1))] + const filteredDataViewFields = populatedDataViewFields .filter((d) => allDataViewFields.includes(d)) .sort(); setCcsWarning(isCrossClusterSearch && isMissingFields); setStatus(INDEX_STATUS.LOADED); - setDataViewFields(populatedFields); + setDataViewFields(filteredDataViewFields); }; fetchDataGridSampleDocuments(); @@ -145,7 +152,7 @@ export const useIndexData = ( abortController.abort(); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [timeRangeMs]); + }, [timeRangeMs, populatedFields?.size]); const columns: EuiDataGridColumn[] = useMemo(() => { if (typeof dataViewFields === 'undefined') { diff --git a/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts b/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts index c4ccff9944dd4..f2a59e0cb1a61 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts @@ -39,7 +39,10 @@ export const useSearchItems = (defaultSavedObjectId: string | undefined) => { } try { - fetchedSavedSearch = await appDeps.savedSearch.get(id); + // If data view already found, no need to get saved search + if (!fetchedDataView) { + fetchedSavedSearch = await appDeps.savedSearch.get(id); + } } catch (e) { // Just let fetchedSavedSearch stay undefined in case it doesn't exist. } diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_define/step_define_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_define/step_define_form.tsx index 9796d9f01de65..81bdb47735a37 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_define/step_define_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_define/step_define_form.tsx @@ -58,7 +58,7 @@ import { import { useDocumentationLinks } from '../../../../hooks/use_documentation_links'; import { useIndexData } from '../../../../hooks/use_index_data'; import { useTransformConfigData } from '../../../../hooks/use_transform_config_data'; -import { useToastNotifications } from '../../../../app_dependencies'; +import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; import { SearchItems } from '../../../../hooks/use_search_items'; import { getAggConfigFromEsAgg } from '../../../../common/pivot_aggs'; @@ -120,8 +120,20 @@ export const StepDefineForm: FC = React.memo((props) => { const { transformConfigQuery } = stepDefineForm.searchBar.state; const { runtimeMappings } = stepDefineForm.runtimeMappingsEditor.state; + const appDependencies = useAppDependencies(); + const { + ml: { useFieldStatsFlyoutContext }, + } = appDependencies; + + const fieldStatsContext = useFieldStatsFlyoutContext(); const indexPreviewProps = { - ...useIndexData(dataView, transformConfigQuery, runtimeMappings, timeRangeMs), + ...useIndexData( + dataView, + transformConfigQuery, + runtimeMappings, + timeRangeMs, + fieldStatsContext?.populatedFields ?? null + ), dataTestSubj: 'transformIndexPreview', toastNotifications, };