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

alexwizp · 2020-11-09T15:24:23Z

Summary

For dashboard's filter bar to work visualisations communicate its index-patterns.
Some visualisations don't do that (#19408)
But according to (#19408 (comment)) TSVB should.

This is causing bugs and confusions with how filter bar is working on a dashboard when TSVB is used.
Especially filter bar is almost non usable for dashboards that have only TSVB visualisations and use non-default index patterns.

Steps to reproduce:

Create a TSVB with non default index pattern
Create a dashboard with single TSVB lib
Try to use a filter bar and notice that there is no way to use pick a filter for used index pattern

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests 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)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Closes: elastic#81476

elasticmachine · 2020-11-10T08:59:50Z

Pinging @elastic/kibana-app (Team:KibanaApp)

stratoula

Code LGTM and it works fine, now I can create filters even if I use a non default index pattern

I have also tested it with multiple index patterns and all of them are communicated to our filter components and can be used.
I am fine with this approach if the arch team agrees

alexwizp · 2020-11-11T15:52:47Z

@elasticmachine merge upstream

lukeelmers · 2020-11-11T17:14:03Z

src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts

+    if (vis.type.getUsedIndexPattern) {
+      indexPatterns = await vis.type.getUsedIndexPattern(vis.params);
+    } else if (vis.data.indexPattern) {
+      indexPatterns = [vis.data.indexPattern];
+    }


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:

kibana/src/plugins/visualizations/public/vis.ts

Lines 140 to 153 in a17dcad

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?

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.

lukeelmers

Code here LGTM. I still think that it would feel a bit cleaner to modify setState to call this new method from vis.type, but I'll defer to @ppisljar's original feedback on that as I'm sure he had reasons for not doing so.

alexwizp · 2020-11-12T12:48:17Z

@elasticmachine merge upstream

alexwizp · 2020-11-12T14:34:24Z

@ppisljar it wouldn't be right to merge/fix it without your feedback

alexwizp · 2020-11-15T18:24:03Z

@elasticmachine merge upstream

kibanamachine · 2020-11-15T20:06:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 422a5cd

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimeseries`	132.8KB	134.5KB	+1.7KB
`visualizations`	169.7KB	169.9KB	+286.0B
total			+1.9KB

History

💚 Build #87598 succeeded ec1fb8a
💚 Build #87371 succeeded a17dcad
💚 Build #87072 succeeded 49623c9
💔 Build #87055 failed 8d3878d
💔 Build #87038 failed fbec631

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

ppisljar

code LGTM, not putting this on vis.data as that's specific to our 'classic' visualizations and should not try to cover all the custom vis types (like tsvb)

) * TSVB doesn't communicate it's index-patterns to dashboard Closes: elastic#81476 * useCustomSearchSource -> getUsedIndexPattern * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…83395) * TSVB doesn't communicate it's index-patterns to dashboard Closes: #81476 * useCustomSearchSource -> getUsedIndexPattern * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: Migrate `/translations` route to core (elastic#83280) [APM] Ensure APM jest script can run (elastic#83398) [Uptime] Monitor status alert use url as instance (elastic#81736) [ML] Add basic license test run details to ML+Transform READMEs (elastic#83259) TSVB doesn't communicate it's index-patterns to dashboard (elastic#82964) [Alerting UI] Added ability to assign alert actions to resolved action group in UI (elastic#83139) Skips Vega test skip flaky suite (elastic#79967) [bundle optimization] Update to semver 7.x to get tree-shaking (elastic#83020) Added ability to fire actions when an alert instance is resolved (elastic#82799) [ML] Adds functional tests for the index data visualizer card contents (elastic#83174)

… into add-logs-to-node-details * 'add-logs-to-node-details' of github.com:phillipb/kibana: fix tall vislib charts in visualize (elastic#83340) [Lens] Avoid unnecessary data fetching on dimension flyout open (elastic#82957) [Security Solution][Case] Change case connector minimum required license to basic (elastic#83401) fix logstash central pipeline management test (elastic#83281) [Search] Send to background UI (elastic#81793) Migrate `/translations` route to core (elastic#83280) [APM] Ensure APM jest script can run (elastic#83398) [Uptime] Monitor status alert use url as instance (elastic#81736) [ML] Add basic license test run details to ML+Transform READMEs (elastic#83259) TSVB doesn't communicate it's index-patterns to dashboard (elastic#82964) [Alerting UI] Added ability to assign alert actions to resolved action group in UI (elastic#83139) Skips Vega test skip flaky suite (elastic#79967) [bundle optimization] Update to semver 7.x to get tree-shaking (elastic#83020) Added ability to fire actions when an alert instance is resolved (elastic#82799)

TheRiffRafi · 2020-12-17T19:24:31Z

Hello Folks,
Does this fix cover Timelion as well? Timelion can especify a particular index in it's expression but that also is not communicated to the dashboard filter it seems, I just tested on 7.10.

stratoula · 2020-12-18T08:21:29Z

@TheRiffRafi this fix covers Vega and TSVB, not timelion. I have created an issue to track it #86418. It is a little bit more complicated for Timelion but it is doable.

) * TSVB doesn't communicate it's index-patterns to dashboard Closes: elastic#81476 * useCustomSearchSource -> getUsedIndexPattern * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/vis_type_timeseries/public/metrics_type.ts # src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts # src/plugins/visualizations/public/vis_types/base_vis_type.ts # src/plugins/visualizations/public/vis_types/types.ts

alexwizp · 2020-12-18T13:18:57Z

@stratoula this PR cover only TSVB related issues. For Vega we have a separate PR #84090. Please let me know if we also want to backport it into 7.10

stratoula · 2020-12-18T13:23:14Z

@alexwizp yes I meant that we did the fix for TSVB and vega for 7.11. Yes let's also backport the Vega PR ❤️

) (#86448) * TSVB doesn't communicate it's index-patterns to dashboard (#82964) * TSVB doesn't communicate it's index-patterns to dashboard Closes: #81476 * useCustomSearchSource -> getUsedIndexPattern * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/vis_type_timeseries/public/metrics_type.ts # src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts # src/plugins/visualizations/public/vis_types/base_vis_type.ts # src/plugins/visualizations/public/vis_types/types.ts * fix CI

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

e2e02fe

Closes: elastic#81476

alexwizp force-pushed the 81476 branch from 4cab0fa to e2e02fe Compare November 9, 2020 15:54

alexwizp requested a review from stratoula November 10, 2020 08:11

alexwizp self-assigned this Nov 10, 2020

alexwizp requested a review from ppisljar November 10, 2020 08:50

alexwizp added v7.11 v8.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix labels Nov 10, 2020

alexwizp marked this pull request as ready for review November 10, 2020 08:59

alexwizp requested a review from a team November 10, 2020 08:59

useCustomSearchSource -> getUsedIndexPattern

fbec631

alexwizp force-pushed the 81476 branch 2 times, most recently from 8d3878d to bd50777 Compare November 10, 2020 13:25

fix CI

49623c9

alexwizp force-pushed the 81476 branch from bd50777 to 49623c9 Compare November 10, 2020 13:28

stratoula approved these changes Nov 11, 2020

View reviewed changes

Merge branch 'master' into 81476

a17dcad

alexwizp requested a review from lukeelmers November 11, 2020 15:54

lukeelmers reviewed Nov 11, 2020

View reviewed changes

lukeelmers approved these changes Nov 11, 2020

View reviewed changes

Merge branch 'master' into 81476

ec1fb8a

spalger added v7.11.0 and removed v7.11 labels Nov 12, 2020

Merge branch 'master' into 81476

422a5cd

ppisljar approved these changes Nov 16, 2020

View reviewed changes

alexwizp merged commit 253495b into elastic:master Nov 16, 2020

alexwizp mentioned this pull request Nov 16, 2020

[7.x] TSVB doesn't communicate it's index-patterns to dashboard (#82964) #83395

Merged

alexwizp mentioned this pull request Nov 27, 2020

[Vega] Filter bar in Vega is not usable with non default index pattern. #84090

Merged

4 tasks

alexwizp mentioned this pull request Dec 18, 2020

[7.10] TSVB doesn't communicate it's index-patterns to dashboard (#82964) #86448

Merged

alexwizp deleted the 81476 branch January 16, 2021 15:27

alexwizp mentioned this pull request Apr 16, 2021

[TSVB] Improve dashboard performance #97371

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

alexwizp commented Nov 9, 2020 •

edited

Loading

elasticmachine commented Nov 10, 2020

stratoula left a comment

alexwizp commented Nov 11, 2020

lukeelmers Nov 11, 2020

alexwizp Nov 11, 2020

lukeelmers left a comment

alexwizp commented Nov 12, 2020

alexwizp commented Nov 12, 2020

alexwizp commented Nov 15, 2020

kibanamachine commented Nov 15, 2020

ppisljar left a comment

TheRiffRafi commented Dec 17, 2020

stratoula commented Dec 18, 2020

alexwizp commented Dec 18, 2020

stratoula commented Dec 18, 2020

	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');
	}
	}

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

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

Conversation

alexwizp commented Nov 9, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Nov 10, 2020

stratoula left a comment

Choose a reason for hiding this comment

alexwizp commented Nov 11, 2020

lukeelmers Nov 11, 2020

Choose a reason for hiding this comment

alexwizp Nov 11, 2020

Choose a reason for hiding this comment

lukeelmers left a comment

Choose a reason for hiding this comment

alexwizp commented Nov 12, 2020

alexwizp commented Nov 12, 2020

alexwizp commented Nov 15, 2020

kibanamachine commented Nov 15, 2020

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

ppisljar left a comment

Choose a reason for hiding this comment

TheRiffRafi commented Dec 17, 2020

stratoula commented Dec 18, 2020

alexwizp commented Dec 18, 2020

stratoula commented Dec 18, 2020

alexwizp commented Nov 9, 2020 •

edited

Loading