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

unified search - iindex_pattern => data view type improvement #129506

Merged
merged 5 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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 @@ -7,15 +7,16 @@
*/

import { i18n } from '@kbn/i18n';
import { Filter, IIndexPattern } from '../../../../common';
import { Filter } from '../../../../common';
import { DataView, DataViewField } from '../../../../../data_views/public';
import { getIndexPatternFromFilter } from './get_index_pattern_from_filter';

function getValueFormatter(indexPattern?: IIndexPattern, key?: string) {
function getValueFormatter(indexPattern?: DataView, key?: string) {
// checking getFormatterForField exists because there is at least once case where an index pattern
// is an object rather than an IndexPattern class
if (!indexPattern || !indexPattern.getFormatterForField || !key) return;

const field = indexPattern.fields.find((f) => f.name === key);
const field = indexPattern.fields.find((f: DataViewField) => f.name === key);
if (!field) {
throw new Error(
i18n.translate('data.filter.filterBar.fieldNotFound', {
Expand All @@ -27,7 +28,7 @@ function getValueFormatter(indexPattern?: IIndexPattern, key?: string) {
return indexPattern.getFormatterForField(field);
}

export function getDisplayValueFromFilter(filter: Filter, indexPatterns: IIndexPattern[]): string {
export function getDisplayValueFromFilter(filter: Filter, indexPatterns: DataView[]): string {
const { key, value } = filter.meta;
if (typeof value === 'function') {
const indexPattern = getIndexPatternFromFilter(filter, indexPatterns);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
* Side Public License, v 1.
*/

import { Filter, IIndexPattern } from '../../../../common';
import { Filter } from '../../../../common';
import { DataView } from '../../../../../data_views/public';

export function getIndexPatternFromFilter(
filter: Filter,
indexPatterns: IIndexPattern[]
): IIndexPattern | undefined {
indexPatterns: DataView[]
): DataView | undefined {
return indexPatterns.find((indexPattern) => indexPattern.id === filter.meta.index);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import { FormattedMessage } from '@kbn/i18n-react';
import React, { Component } from 'react';
import { getDisplayValueFromFilter, mapAndFlattenFilters } from '../../../data/public';
import { FilterLabel } from '../filter_bar';
import { Filter, IIndexPattern } from '../../../data/common';
import { Filter } from '../../../data/common';
import { DataView } from '../../../data_views/public';

interface Props {
filters: Filter[];
indexPatterns: IIndexPattern[];
indexPatterns: DataView[];
onCancel: () => void;
onSubmit: (filters: Filter[]) => void;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
*/

import React from 'react';
import { IIndexPattern, Filter } from '../../../data/common';
import { Filter } from '../../../data/common';
import { DataView } from '../../../data_views/common';

type CancelFnType = () => void;
type SubmitFnType = (filters: Filter[]) => void;
Expand All @@ -18,7 +19,7 @@ const LazyApplyFiltersPopoverContent = React.lazy(() => import('./apply_filter_p

export const applyFiltersPopover = (
filters: Filter[],
indexPatterns: IIndexPattern[],
indexPatterns: DataView[],
onCancel: CancelFnType,
onSubmit: SubmitFnType
) => {
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/unified_search/public/filter_bar/filter_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import { FilterEditor } from './filter_editor';
import { FILTER_EDITOR_WIDTH, FilterItem } from './filter_item';
import { FilterOptions } from './filter_options';
import { useKibana } from '../../../kibana_react/public';
import { IIndexPattern, UI_SETTINGS } from '../../../data/common';
import { UI_SETTINGS } from '../../../data/common';
import { IDataPluginServices } from '../../../data/public';
import { DataView } from '../../../data_views/public';

export interface Props {
filters: Filter[];
onFiltersUpdated?: (filters: Filter[]) => void;
className: string;
indexPatterns: IIndexPattern[];
indexPatterns: DataView[];
intl: InjectedIntl;
appName: string;
timeRangeForSuggestionsOverride?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ import { Operator } from './lib/filter_operators';
import { PhraseValueInput } from './phrase_value_input';
import { PhrasesValuesInput } from './phrases_values_input';
import { RangeValueInput } from './range_value_input';
import { IIndexPattern, IFieldType } from '../../../../data/common';
import { DataView, IFieldType } from '../../../../data_views/common';
import { getIndexPatternFromFilter } from '../../../../data/public';
import { CodeEditor } from '../../../../kibana_react/public';

export interface Props {
filter: Filter;
indexPatterns: IIndexPattern[];
indexPatterns: DataView[];
onSubmit: (filter: Filter) => void;
onCancel: () => void;
intl: InjectedIntl;
timeRangeForSuggestionsOverride?: boolean;
}

interface State {
selectedIndexPattern?: IIndexPattern;
selectedIndexPattern?: DataView;
selectedField?: IFieldType;
selectedOperator?: Operator;
params: any;
Expand Down Expand Up @@ -432,7 +432,7 @@ class FilterEditorUI extends Component<Props, State> {
return isFilterValid(indexPattern, field, operator, params);
}

private onIndexPatternChange = ([selectedIndexPattern]: IIndexPattern[]) => {
private onIndexPatternChange = ([selectedIndexPattern]: DataView[]) => {
const selectedField = undefined;
const selectedOperator = undefined;
const params = undefined;
Expand Down Expand Up @@ -513,7 +513,7 @@ class FilterEditorUI extends Component<Props, State> {
};
}

function IndexPatternComboBox(props: GenericComboBoxProps<IIndexPattern>) {
function IndexPatternComboBox(props: GenericComboBoxProps<DataView>) {
return GenericComboBox(props);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import { Filter, FieldFilter } from '@kbn/es-query';
import { ES_FIELD_TYPES } from '@kbn/field-types';
import isSemverValid from 'semver/functions/valid';
import { FILTER_OPERATORS, Operator } from './filter_operators';
import { isFilterable, IIndexPattern, IFieldType, IpAddress } from '../../../../../data/common';
import { isFilterable, IFieldType, IpAddress } from '../../../../../data/common';
import { DataView } from '../../../../../data_views/common';

export function getFieldFromFilter(filter: FieldFilter, indexPattern: IIndexPattern) {
export function getFieldFromFilter(filter: FieldFilter, indexPattern: DataView) {
return indexPattern.fields.find((field) => field.name === filter.meta.key);
}

Expand All @@ -23,7 +24,7 @@ export function getOperatorFromFilter(filter: Filter) {
});
}

export function getFilterableFields(indexPattern: IIndexPattern) {
export function getFilterableFields(indexPattern: DataView) {
return indexPattern.fields.filter(isFilterable);
}

Expand Down Expand Up @@ -57,7 +58,7 @@ export function validateParams(params: any, field: IFieldType) {
}

export function isFilterValid(
indexPattern?: IIndexPattern,
indexPattern?: DataView,
field?: IFieldType,
operator?: Operator,
params?: any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import React from 'react';
import { debounce } from 'lodash';

import { withKibana, KibanaReactContextValue } from '../../../../kibana_react/public';
import { IIndexPattern, IFieldType, UI_SETTINGS } from '../../../../data/common';
import { IFieldType, UI_SETTINGS } from '../../../../data/common';
import { DataView } from '../../../../data_views/common';
import { IDataPluginServices } from '../../../../data/public';

export interface PhraseSuggestorProps {
kibana: KibanaReactContextValue<IDataPluginServices>;
indexPattern: IIndexPattern;
indexPattern: DataView;
field: IFieldType;
timeRangeForSuggestionsOverride?: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import React, { MouseEvent, useState, useEffect, HTMLAttributes } from 'react';
import { IUiSettingsClient } from 'src/core/public';
import { FilterEditor } from './filter_editor';
import { FilterView } from './filter_view';
import { IIndexPattern } from '../../../data/common';
import { DataView } from '../../../data_views/public';
import { getIndexPatternFromFilter, getDisplayValueFromFilter } from '../../../data/public';
import { getIndexPatterns } from '../services';

Expand All @@ -29,7 +29,7 @@ type PanelOptions = 'pinFilter' | 'editFilter' | 'negateFilter' | 'disableFilter
export interface FilterItemProps {
id: string;
filter: Filter;
indexPatterns: IIndexPattern[];
indexPatterns: DataView[];
className?: string;
onUpdate: (filter: Filter) => void;
onRemove: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
*/

import { isEmpty } from 'lodash';
import { IndexPatternsContract } from '../../../data/public';
import { DataViewsContract } from '../../../data_views/public';

export async function fetchIndexPatterns(
indexPatternsService: IndexPatternsContract,
indexPatternsService: DataViewsContract,
indexPatternStrings: string[]
) {
if (!indexPatternStrings || isEmpty(indexPatternStrings)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import {
} from '@elastic/eui';
import {
IDataPluginServices,
IIndexPattern,
TimeRange,
TimeHistoryContract,
Query,
getQueryLog,
} from '../../../data/public';
import { DataView } from '../../../data_views/public';
import type { PersistedLog } from '../../../data/public';
import { useKibana, withKibana } from '../../../kibana_react/public';
import QueryStringInputUI from './query_string_input';
Expand All @@ -56,7 +56,7 @@ export interface QueryBarTopRowProps {
disableAutoFocus?: boolean;
fillSubmitButton: boolean;
iconType?: EuiIconProps['type'];
indexPatterns?: Array<IIndexPattern | string>;
indexPatterns?: Array<DataView | string>;
indicateNoData?: boolean;
isClearable?: boolean;
isDirty: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ import { compact, debounce, isEqual, isFunction } from 'lodash';
import { Toast } from '../../../../core/public';
import {
IDataPluginServices,
IIndexPattern,
Query,
QuerySuggestion,
QuerySuggestionTypes,
getQueryLog,
} from '../../../data/public';
import { DataView } from '../../../data_views/public';
import { matchPairs } from './match_pairs';
import { toUser } from './to_user';
import { fromUser } from './from_user';
Expand All @@ -50,7 +50,7 @@ import { onRaf } from '../utils';
import { getTheme } from '../services';

export interface QueryStringInputProps {
indexPatterns: Array<IIndexPattern | string>;
indexPatterns: Array<DataView | string>;
query: Query;
disableAutoFocus?: boolean;
screenTitle?: string;
Expand Down Expand Up @@ -104,7 +104,7 @@ interface State {
suggestionLimit: number;
selectionStart: number | null;
selectionEnd: number | null;
indexPatterns: IIndexPattern[];
indexPatterns: DataView[];

/**
* Part of state because passed down to child components
Expand Down Expand Up @@ -170,7 +170,7 @@ export default class QueryStringInputUI extends PureComponent<Props, State> {
) as string[];
const objectPatterns = this.props.indexPatterns.filter(
(indexPattern) => typeof indexPattern !== 'string'
) as IIndexPattern[];
) as DataView[];

// abort the previous fetch to avoid overriding with outdated data
// issue https://github.com/elastic/kibana/issues/80831
Expand All @@ -181,7 +181,7 @@ export default class QueryStringInputUI extends PureComponent<Props, State> {
const objectPatternsFromStrings = (await fetchIndexPatterns(
this.services.data.indexPatterns,
stringPatterns
)) as IIndexPattern[];
)) as DataView[];

if (!currentAbortController.signal.aborted) {
this.setState({
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/unified_search/public/search_bar/search_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import type { TimeHistoryContract, SavedQuery } from '../../../data/public';
import type { SavedQueryAttributes } from '../../../data/common';
import { IDataPluginServices } from '../../../data/public';
import { FilterBar } from '../filter_bar';
import { TimeRange, IIndexPattern } from '../../../data/common';
import { TimeRange } from '../../../data/common';
import { DataView } from '../../../data_views/public';
import { SavedQueryMeta, SaveQueryForm } from '../saved_query_form';
import { SavedQueryManagementComponent } from '../saved_query_management';

Expand All @@ -38,7 +39,7 @@ export interface SearchBarInjectedDeps {
}

export interface SearchBarOwnProps {
indexPatterns?: IIndexPattern[];
indexPatterns?: DataView[];
isLoading?: boolean;
customSubmitButton?: React.ReactNode;
screenTitle?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { fromKueryExpression } from '@kbn/es-query';

import type { FieldSpec } from '../../../../../../../src/plugins/data/common';
import { QueryStringInput } from '../../../../../../../src/plugins/unified_search/public';
import type { DataView } from '../../../../../../../src/plugins/data_views/public';
import { useStartServices } from '../hooks';
import { INDEX_NAME, AGENTS_PREFIX } from '../constants';

Expand Down Expand Up @@ -79,12 +80,12 @@ export const SearchBar: React.FunctionComponent<Props> = ({
disableLanguageSwitcher={true}
indexPatterns={
indexPatternFields
? [
? ([
{
title: indexPattern,
fields: indexPatternFields,
},
]
] as DataView[])
: []
}
query={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { i18n } from '@kbn/i18n';

import type { FieldSpec } from '../../../../../../../../../../../src/plugins/data/common';
import { QueryStringInput } from '../../../../../../../../../../../src/plugins/unified_search/public';
import type { DataView } from '../../../../../../../../../../../src/plugins/data_views/public';
import { useStartServices } from '../../../../../hooks';

import {
Expand Down Expand Up @@ -53,12 +54,12 @@ export const LogQueryBar: React.FunctionComponent<{
disableLanguageSwitcher={true}
indexPatterns={
indexPatternFields
? [
? ([
{
title: AGENT_LOG_INDEX_PATTERN,
fields: indexPatternFields,
},
]
] as DataView[])
: []
}
query={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Query } from '@kbn/es-query';
import { i18n } from '@kbn/i18n';
import React, { useContext } from 'react';
import { QueryStringInput } from '../../../../../../../src/plugins/unified_search/public';
import { DataView } from '../../../../../../../src/plugins/data_views/public';
import { euiStyled } from '../../../../../../../src/plugins/kibana_react/common';
import { LogCustomizationMenu } from '../../../components/logging/log_customization_menu';
import { LogDatepicker } from '../../../components/logging/log_datepicker';
Expand Down Expand Up @@ -57,7 +58,7 @@ export const LogsToolbar = () => {
<QueryStringInput
disableLanguageSwitcher={true}
iconType="search"
indexPatterns={[derivedDataView]}
indexPatterns={[derivedDataView as DataView]}
Copy link
Member

@weltenwort weltenwort Apr 6, 2022

Choose a reason for hiding this comment

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

Isn't this pretty risky? If the QueryStringInput requires a full DataView instance from now on, is there a way to provide one without all the overhead of persisting it?

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 the QueryStringInput requires a full DataView instance from now on, is there a way to provide one without all the overhead of persisting it?

dataViews.create will create an instance without persisting.

We're working to remove all instances of faked data view instances. Is this happening in the infra code? Perhaps a new issue needs to be created to address this.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that derived data view is "fake". But if data views can now be created without persistence we can replace that. But let's do that separately 👍

isInvalid={!isFilterQueryDraftValid}
onChange={(query: Query) => {
setSurroundingLogsId(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { DataViewBase } from '@kbn/es-query';
import React, { useMemo, useState } from 'react';
import { TimeHistory } from '../../../../../../../src/plugins/data/public';
import { DataView } from '../../../../../../../src/plugins/data_views/public';
import { SearchBar } from '../../../../../../../src/plugins/unified_search/public';
import { Storage } from '../../../../../../../src/plugins/kibana_utils/public';
import { translations } from '../../../config';
Expand Down Expand Up @@ -47,7 +48,7 @@ export function AlertsSearchBar({

return (
<SearchBar
indexPatterns={compatibleIndexPatterns}
indexPatterns={compatibleIndexPatterns as DataView[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ok as far as the prop indexPatterns exposed by SearchBar is the same type DataView
https://github.com/elastic/kibana/pull/129506/files#diff-c82e1b8d881f115132594175097c24185f00063c492eb44af979776de2218f53R42

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkanout Its the responsibility of the owners of x-pack/plugins/observability to determine this is okay or to explain why its not. If its not okay we can review options to resolve the issue or leave this alone for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@XavierM, it would be helpful to get your inputs here, please.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's merge this one as the DataView type is used in both places. Later on, If the type needs to be changed, it will be handled in a different PR.

placeholder={translations.alertsSearchBar.placeholder}
query={{ query: query ?? '', language: queryLanguage }}
timeHistory={timeHistory}
Expand Down
Loading