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

TSVB doesn't communicate it's index-patterns to dashboard #82964

Merged
merged 6 commits into from
Nov 16, 2020
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
1 change: 1 addition & 0 deletions src/plugins/vis_type_timeseries/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
*/

export const MAX_BUCKETS_SETTING = 'metrics:max_buckets';
export const INDEXES_SEPARATOR = ',';
20 changes: 19 additions & 1 deletion src/plugins/vis_type_timeseries/public/metrics_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import { EditorController } from './application';
// @ts-ignore
import { PANEL_TYPES } from '../common/panel_types';
import { VisEditor } from './application/components/vis_editor_lazy';
import { VIS_EVENT_TO_TRIGGER, VisGroups } from '../../visualizations/public';
import { VIS_EVENT_TO_TRIGGER, VisGroups, VisParams } from '../../visualizations/public';
import { getDataStart } from './services';
import { INDEXES_SEPARATOR } from '../common/constants';

export const metricsVisDefinition = {
name: 'metrics',
Expand Down Expand Up @@ -84,5 +86,21 @@ export const metricsVisDefinition = {
return [VIS_EVENT_TO_TRIGGER.applyFilter];
},
inspectorAdapters: {},
getUsedIndexPattern: async (params: VisParams) => {
const { indexPatterns } = getDataStart();
const indexes: string = params.index_pattern;

if (indexes) {
const cachedIndexes = await indexPatterns.getIdsWithTitle();
const ids = indexes
.split(INDEXES_SEPARATOR)
.map((title) => cachedIndexes.find((i) => i.title === title)?.id)
.filter((id) => id);

return Promise.all(ids.map((id) => indexPatterns.get(id!)));
}

return [];
},
responseHandler: 'none',
};
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
import { VisualizeEmbeddableFactoryDeps } from './visualize_embeddable_factory';
import { VISUALIZE_ENABLE_LABS_SETTING } from '../../common/constants';
import { SavedVisualizationsLoader } from '../saved_visualizations';
import { IndexPattern } from '../../../data/public';

export const createVisEmbeddableFromObject = (deps: VisualizeEmbeddableFactoryDeps) => async (
vis: Vis,
Expand Down Expand Up @@ -69,8 +70,14 @@ export const createVisEmbeddableFromObject = (deps: VisualizeEmbeddableFactoryDe
return new DisabledLabEmbeddable(vis.title, input);
}

const indexPattern = vis.data.indexPattern;
const indexPatterns = indexPattern ? [indexPattern] : [];
let indexPatterns: IndexPattern[] = [];

if (vis.type.getUsedIndexPattern) {
indexPatterns = await vis.type.getUsedIndexPattern(vis.params);
} else if (vis.data.indexPattern) {
indexPatterns = [vis.data.indexPattern];
}
Comment on lines +75 to +79
Copy link
Member

Choose a reason for hiding this comment

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

One idea that came to mind:

I'm wondering if, instead of having two sources of truth in a vis for the index pattern (either vis.data.indexPattern OR vis.type.getUsedIndexPattern), it would be cleaner to make a change in vis itself so that vis.data.indexPattern always reads from vis.type.getUsedIndexPattern.

In vis.ts we could do something like:

this.data.indexPattern = await this.type.getIndexPattern(this.data, this.params);

Then on the vis type:

await getIndexPattern(data: VisData, params: VisParams) {
  ...etc
}

Then on the vis type, getIndexPattern is optional, and defaults to the logic that's currently in vis.ts:

if (state.data && state.data.searchSource) {
this.data.searchSource = await getSearch().searchSource.create(state.data.searchSource!);
this.data.indexPattern = this.data.searchSource.getField('index');
}
if (state.data && state.data.savedSearchId) {
this.data.savedSearchId = state.data.savedSearchId;
if (this.data.searchSource) {
this.data.searchSource = await getSearchSource(
this.data.searchSource,
this.data.savedSearchId
);
this.data.indexPattern = this.data.searchSource.getField('index');
}
}

WDYT @alexwizp? Do you think that would work / make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty the same idea was implemented in my initial commit(e2e02fe), but after discussing it @ppisljar we decided to don't modify vis.setState method.


const editable = getCapabilities().visualize.save as boolean;

return new VisualizeEmbeddable(
Expand Down
1 change: 0 additions & 1 deletion src/plugins/visualizations/public/vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ export class Vis<TVisParams = VisParams> {
if (state.params || typeChanged) {
this.params = this.getParams(state.params);
}

if (state.data && state.data.searchSource) {
this.data.searchSource = await getSearch().searchSource.create(state.data.searchSource!);
this.data.indexPattern = this.data.searchSource.getField('index');
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/visualizations/public/vis_types/base_vis_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface CommonBaseVisTypeOptions<TVisParams>
| 'editorConfig'
| 'hidden'
| 'stage'
| 'getUsedIndexPattern'
| 'useCustomNoDataScreen'
| 'visConfig'
| 'group'
Expand Down Expand Up @@ -96,6 +97,7 @@ export class BaseVisType<TVisParams = VisParams> implements VisType<TVisParams>
public readonly responseHandler;
public readonly hierarchicalData;
public readonly setup;
public readonly getUsedIndexPattern;
public readonly useCustomNoDataScreen;
public readonly inspectorAdapters;
public readonly toExpressionAst;
Expand Down Expand Up @@ -126,6 +128,7 @@ export class BaseVisType<TVisParams = VisParams> implements VisType<TVisParams>
this.responseHandler = opts.responseHandler ?? 'none';
this.setup = opts.setup;
this.hierarchicalData = opts.hierarchicalData ?? false;
this.getUsedIndexPattern = opts.getUsedIndexPattern;
this.useCustomNoDataScreen = opts.useCustomNoDataScreen ?? false;
this.inspectorAdapters = opts.inspectorAdapters;
this.toExpressionAst = opts.toExpressionAst;
Expand Down
10 changes: 9 additions & 1 deletion src/plugins/visualizations/public/vis_types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
import { IconType } from '@elastic/eui';
import React from 'react';
import { Adapters } from 'src/plugins/inspector';
import { IndexPattern } from 'src/plugins/data/public';
import { VisEditorConstructor } from 'src/plugins/visualize/public';
import { ISchemas } from 'src/plugins/vis_default_editor/public';
import { TriggerContextMapping } from '../../../ui_actions/public';
import { Vis, VisToExpressionAst, VisualizationControllerConstructor } from '../types';
import { Vis, VisParams, VisToExpressionAst, VisualizationControllerConstructor } from '../types';

export interface VisTypeOptions {
showTimePicker: boolean;
Expand Down Expand Up @@ -64,6 +65,13 @@ export interface VisType<TVisParams = unknown> {
* If given, it will return the supported triggers for this vis.
*/
readonly getSupportedTriggers?: () => Array<keyof TriggerContextMapping>;

/**
* Some visualizations are created without SearchSource and may change the used indexes during the visualization configuration.
* Using this method we can rewrite the standard mechanism for getting used indexes
*/
readonly getUsedIndexPattern?: (visParams: VisParams) => IndexPattern[] | Promise<IndexPattern[]>;

readonly isAccessible?: boolean;
readonly requestHandler?: string | unknown;
readonly responseHandler?: string | unknown;
Expand Down