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

[UnifiedFieldList] Add new fields ingested in background with valid mappings #172329

Merged
merged 45 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
26d3d6d
Add new fields to list with correct types
kertal Nov 30, 2023
7b999db
Improve code
kertal Dec 1, 2023
3a6c239
Remove redundant newFieldsByFieldNameMap
kertal Dec 1, 2023
307a16d
Add multi field support
kertal Dec 4, 2023
54daa2f
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 4, 2023
ca50f4d
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 7, 2023
999a6a9
Add support for Lens
kertal Dec 9, 2023
1a30c70
Merge remote-tracking branch 'origin/unified-field-list-add-new-field…
kertal Dec 9, 2023
fa237a4
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 12, 2023
15cc582
Add functional test
kertal Dec 13, 2023
3cea884
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 13, 2023
065a50c
Add documentation
kertal Dec 14, 2023
e9fd00e
Improve code
kertal Dec 14, 2023
63907e4
Merge branch 'unified-field-list-add-new-fields' of github.com:kertal…
kertal Dec 14, 2023
454592c
Add Lens test
kertal Dec 15, 2023
75cc235
Improve Lens code preventing an endless loop of update
kertal Dec 15, 2023
12616b4
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 18, 2023
3a74600
Update packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts
kertal Dec 19, 2023
15e5168
Update packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
kertal Dec 19, 2023
139b651
Update packages/kbn-unified-field-list/src/services/field_existing/fi…
kertal Dec 19, 2023
ab514e4
Update packages/kbn-unified-field-list/src/services/field_existing/fi…
kertal Dec 19, 2023
5cd087f
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 19, 2023
b424159
Improve code
kertal Dec 20, 2023
9b70dec
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 20, 2023
30dfec6
Add isFieldLensCompatible for new fields in Lens
kertal Dec 20, 2023
f747c2e
Improve multi field code
kertal Dec 21, 2023
9ed0c49
Improve code
kertal Dec 21, 2023
5420c77
Remove redundant field list refreshing
kertal Dec 21, 2023
f340fae
Undo multi field changes
kertal Dec 22, 2023
26c4d72
Address feedback
kertal Dec 22, 2023
ab2c743
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 22, 2023
921a598
Address review feedback
kertal Dec 22, 2023
ec55f89
Fix error
kertal Dec 22, 2023
c76c87e
Add useMemo for new fields
kertal Dec 22, 2023
165c3cd
Address review feedback
kertal Dec 22, 2023
cbee199
Merge branch 'main' into unified-field-list-add-new-fields
jughosta Jan 2, 2024
9170300
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Jan 2, 2024
d62dba7
Merge branch 'main' into unified-field-list-add-new-fields
jughosta Jan 3, 2024
02a4537
[Discover] Small fixes for types and memo deps. Add tests.
jughosta Jan 3, 2024
621a1df
Merge pull request #16 from jughosta/unified-field-list-add-new-field…
kertal Jan 8, 2024
8617b3d
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Jan 8, 2024
223f546
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Jan 8, 2024
8549c3f
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Jan 9, 2024
f751a4e
Address review feedback
kertal Jan 9, 2024
d024027
Apply useLatest feedback
kertal Jan 9, 2024
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 @@ -191,25 +191,6 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
onSelectedFieldFilter,
]);

useEffect(() => {
if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(calculateMultiFields(allFields, selectedFieldsState.selectedFieldsMap));
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFields,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const popularFieldsLimit = useMemo(
() => core.uiSettings.get(FIELDS_LIMIT_SETTING),
[core.uiSettings]
Expand All @@ -226,7 +207,11 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
[searchMode, stateService.creationOptions.disableMultiFieldsGroupingByParent]
);

const { fieldListFiltersProps, fieldListGroupedProps } = useGroupedFields<DataViewField>({
const {
fieldListFiltersProps,
fieldListGroupedProps,
allFields: allFieldsModified,
} = useGroupedFields<DataViewField>({
dataViewId: (searchMode === 'documents' && dataView?.id) || null, // passing `null` for text-based queries
allFields,
popularFieldsLimit:
Expand All @@ -245,6 +230,27 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
onOverrideFieldGroupDetails: stateService.creationOptions.onOverrideFieldGroupDetails,
});

useEffect(() => {
Copy link
Member Author

@kertal kertal Dec 14, 2023

Choose a reason for hiding this comment

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

note, this code was just moved to have access to the new modified list of allFields

if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(
calculateMultiFields(allFieldsModified, selectedFieldsState.selectedFieldsMap)
);
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFieldsModified,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const renderFieldItem: FieldListGroupedProps<DataViewField>['renderFieldItem'] = useCallback(
({ field, groupName, groupIndex, itemIndex, fieldSearchHighlight }) => (
<li key={`field${field.name}`} data-attr-field={field.name}>
Expand Down
20 changes: 19 additions & 1 deletion packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { type DataPublicPluginStart } from '@kbn/data-plugin/public';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/common';
import { getEsQueryConfig } from '@kbn/data-service/src/es_query';
import { reportPerformanceMetricEvent } from '@kbn/ebt-tools';
import { DataViewField } from '@kbn/data-views-plugin/common';
kertal marked this conversation as resolved.
Show resolved Hide resolved
import { loadFieldExisting } from '../services/field_existing';
import { ExistenceFetchStatus } from '../types';

Expand All @@ -24,6 +25,7 @@ const generateId = htmlIdGenerator();
export interface ExistingFieldsInfo {
fetchStatus: ExistenceFetchStatus;
existingFieldsByFieldNameMap: Record<string, boolean>;
newFields?: DataViewField[];
numberOfFetches: number;
hasDataViewRestrictions?: boolean;
}
Expand Down Expand Up @@ -54,6 +56,7 @@ export interface ExistingFieldsReader {
hasFieldData: (dataViewId: string, fieldName: string) => boolean;
getFieldsExistenceStatus: (dataViewId: string) => ExistenceFetchStatus;
isFieldsExistenceInfoUnavailable: (dataViewId: string) => boolean;
getNewFields: (dataViewId: string) => DataViewField[];
}

const initialData: ExistingFieldsByDataViewMap = {};
Expand Down Expand Up @@ -157,6 +160,7 @@ export const useExistingFieldsFetcher = (
}

info.existingFieldsByFieldNameMap = booleanMap(existingFieldNames);
info.newFields = result.newFields;
info.fetchStatus = ExistenceFetchStatus.succeeded;
} catch (error) {
info.fetchStatus = ExistenceFetchStatus.failed;
Expand Down Expand Up @@ -286,6 +290,19 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
[existingFieldsByDataViewMap]
);

const getNewFields = useCallback(
(dataViewId: string) => {
const info = existingFieldsByDataViewMap[dataViewId];

if (info?.fetchStatus === ExistenceFetchStatus.succeeded) {
return info?.newFields ?? [];
}

return [];
},
[existingFieldsByDataViewMap]
);

const getFieldsExistenceInfo = useCallback(
(dataViewId: string) => {
return dataViewId ? existingFieldsByDataViewMap[dataViewId] : unknownInfo;
Expand Down Expand Up @@ -321,8 +338,9 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
hasFieldData,
getFieldsExistenceStatus,
isFieldsExistenceInfoUnavailable,
getNewFields,
}),
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable]
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable, getNewFields]
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -156,6 +157,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -438,6 +440,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== knownDataViewId,
getNewFields: () => [],
})
);

Expand Down
49 changes: 38 additions & 11 deletions packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { groupBy } from 'lodash';
import { useEffect, useMemo, useState } from 'react';
import { useEffect, useMemo, useState, useRef } from 'react';
import { i18n } from '@kbn/i18n';
import { type CoreStart } from '@kbn/core-lifecycle-browser';
import { type DataView, type DataViewField } from '@kbn/data-views-plugin/common';
Expand Down Expand Up @@ -41,6 +41,7 @@ export interface GroupedFieldsParams<T extends FieldListItem> {
onOverrideFieldGroupDetails?: OverrideFieldGroupDetails;
onSupportedFieldFilter?: (field: T) => boolean;
onSelectedFieldFilter?: (field: T) => boolean;
isCompatibleField?: (fields: DataViewField) => boolean;
kertal marked this conversation as resolved.
Show resolved Hide resolved
}

export interface GroupedFieldsResult<T extends FieldListItem> {
Expand All @@ -52,6 +53,8 @@ export interface GroupedFieldsResult<T extends FieldListItem> {
fieldsExistInIndex: boolean;
screenReaderDescriptionId?: string;
};
allFields: T[] | null; // `null` is for loading indicator
kertal marked this conversation as resolved.
Show resolved Hide resolved
hasNewFields: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it being used somewhere?

Copy link
Member Author

@kertal kertal Dec 19, 2023

Choose a reason for hiding this comment

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

yes, it's here

const { fieldListFiltersProps, fieldListGroupedProps, hasNewFields } =
needed for Lens to indicate there are new fields

}

export function useGroupedFields<T extends FieldListItem = DataViewField>({
Expand All @@ -65,6 +68,7 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
onOverrideFieldGroupDetails,
onSupportedFieldFilter,
onSelectedFieldFilter,
isCompatibleField,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could use onSupportedFieldFilter prop instead of introducing isCompatibleField prop.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will check, it definitely looks similar, so not having to introduce another prop is a good option 👍

}: GroupedFieldsParams<T>): GroupedFieldsResult<T> {
const fieldsExistenceReader = useExistingFieldsReader();
const fieldListFilters = useFieldFilters<T>({
Expand All @@ -73,6 +77,8 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
getCustomFieldType,
onSupportedFieldFilter,
});
const allFieldsInclNew = useRef(allFields);
const hasNewFields = useRef(false);
const onFilterFieldList = fieldListFilters.onFilterField;
const [dataView, setDataView] = useState<DataView | null>(null);
const isAffectedByTimeFilter = Boolean(dataView?.timeFieldName);
Expand Down Expand Up @@ -120,13 +126,30 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
};

const selectedFields = sortedSelectedFields || [];
const sortedFields = [...(allFields || [])].sort(sortFields);
const newFields = dataViewId
? fieldsExistenceReader
.getNewFields(dataViewId)
.filter((field) => (isCompatibleField ? isCompatibleField(field) : true))
: [];
// remove fields from allFields that are available in newFields, because they can be provided in unmapped state
const allFieldsWithoutNewFields = !newFields.length
? allFields
: allFields?.filter((field) => !newFields.find((newField) => newField.name === field.name));
// append new fields to the end of the list allFieldsWithoutNewFields
const allFieldsWithNewFields = allFieldsWithoutNewFields
? [...allFieldsWithoutNewFields, ...newFields]
: newFields;

const sortedFields = [...((allFieldsWithNewFields as unknown as T[]) || [])].sort(sortFields);
allFieldsInclNew.current = sortedFields;
hasNewFields.current = Boolean(newFields.length);
const groupedFields = {
...getDefaultFieldGroups(),
...groupBy(sortedFields, (field) => {
if (!sortedSelectedFields && onSelectedFieldFilter && onSelectedFieldFilter(field)) {
selectedFields.push(field);
}

if (onSupportedFieldFilter && !onSupportedFieldFilter(field)) {
return 'skippedFields';
}
Expand Down Expand Up @@ -311,18 +334,20 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({

return fieldGroupDefinitions;
}, [
sortedSelectedFields,
allFields,
onSupportedFieldFilter,
onSelectedFieldFilter,
onOverrideFieldGroupDetails,
dataView,
dataViewId,
hasFieldDataHandler,
fieldsExistenceInfoUnavailable,
popularFieldsLimit,
isAffectedByGlobalFilter,
isAffectedByTimeFilter,
popularFieldsLimit,
sortedSelectedFields,
dataViewId,
fieldsExistenceInfoUnavailable,
onOverrideFieldGroupDetails,
hasFieldDataHandler,
fieldsExistenceReader,
onSelectedFieldFilter,
onSupportedFieldFilter,
dataView,
isCompatibleField,
]);

const fieldGroups: FieldListGroups<T> = useMemo(() => {
Expand Down Expand Up @@ -381,6 +406,8 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
return {
fieldListGroupedProps,
fieldListFiltersProps: fieldListFilters.fieldListFiltersProps,
allFields: allFieldsInclNew.current,
hasNewFields: hasNewFields.current,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { RuntimeField } from '@kbn/data-views-plugin/common';
import { DataViewField, RuntimeField } from '@kbn/data-views-plugin/common';
kertal marked this conversation as resolved.
Show resolved Hide resolved
import type { DataViewsContract, DataView, FieldSpec } from '@kbn/data-views-plugin/common';
import type { IKibanaSearchRequest } from '@kbn/data-plugin/common';

Expand Down Expand Up @@ -49,15 +49,22 @@ export async function fetchFieldExistence({
metaFields: string[];
dataViewsService: DataViewsContract;
}) {
const allFields = buildFieldList(dataView, metaFields);
const existingFieldList = await dataViewsService.getFieldsForIndexPattern(dataView, {
// filled in by data views service
pattern: '',
indexFilter: toQuery(timeFieldName, fromDate, toDate, dslQuery),
});
const newFields = existingFieldList
.filter((field) => dataView.getFieldByName(field.name) === undefined)
.map((field) => dataView.getFieldByName(field.name) ?? new DataViewField(field));
kertal marked this conversation as resolved.
Show resolved Hide resolved
if (newFields.length) {
await dataViewsService.refreshFields(dataView, false);
}
const allFields = buildFieldList(dataView, metaFields);
return {
indexPatternTitle: dataView.title,
existingFieldNames: existingFields(existingFieldList, allFields),
newFields,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { IUiSettingsClient } from '@kbn/core/public';
import { type DataPublicPluginStart } from '@kbn/data-plugin/public';
import { UI_SETTINGS } from '@kbn/data-service/src/constants';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/common';
import type { DataView, DataViewField, DataViewsContract } from '@kbn/data-views-plugin/common';
import { lastValueFrom } from 'rxjs';
import { fetchFieldExistence } from './field_existing_utils';

Expand All @@ -27,6 +27,7 @@ interface FetchFieldExistenceParams {
export type LoadFieldExistingHandler = (params: FetchFieldExistenceParams) => Promise<{
existingFieldNames: string[];
indexPatternTitle: string;
newFields?: DataViewField[];
}>;

export const loadFieldExisting: LoadFieldExistingHandler = async ({
Expand Down
Loading