Do not apply search source data for tsvb #75137

flash1293 · 2020-08-17T10:05:41Z

In some cases, TSVB visualizations have outdated data in their serialized search source JSON. As TSVB doesn't support the query bar, it should never use these, but as it's sharing infrastructure with Visualize, it will use it if it contains a query or filters.

This PR only puts the query and filters from search source on the expression if it's not writing a TSVB expression. Mid-term TSVB should build its complete expression on its own using a separate toExpression function - when this happens, this workaround can be removed and the logic for honoring search source can simply be omitted from the TSVB specific logic.

Release note

In some old TSVB visualization saved objects, queries and filters can be stored. This is not possible anymore for a while and there is no way to edit them besides changing the JSON of the saved object, but they were still applied to the rendered output. In 7.10, these leftover queries and filters will be removed automatically from the saved object. In almost all cases, no change is necessary. If a visualization contained these local queries and filters deliberately, they should be converted to panel filters in the "Panel options" of the TSVB interface.

elasticmachine · 2020-08-17T12:10:34Z

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

elasticmachine · 2020-08-17T12:10:34Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

flash1293 · 2020-08-19T13:02:06Z

@elasticmachine merge upstream

ppisljar · 2020-08-19T13:04:25Z

src/plugins/visualizations/public/legacy/build_pipeline.ts

  }
 ) => {
  const { indexPattern, searchSource } = vis.data;
-  const query = searchSource!.getField('query');


this is used for the kibana_context function, and should be irrelevant to the visualizaiton. tsvb function can ignore those if it wishes to, but shouldnt in my opinion, as we want it to work outside of visualize context as well.

so i think the actual problem is why is there a searchSource query set for tsvb saved objects, if it shouldn't be as there is no query bar in tsvb application ?

we should probably try to address that problem directly.

this is used for the kibana_context function, and should be irrelevant to the visualizaiton

In case of TSVB, the visualization is using the kibana_context passed in as input to do its data fetching. It needs to do so to get filter and query of the current dashboard and time range, but it shouldn't use the search source of the saved object, because it should be empty anyway.

I agree this is not a super clean fix, but it's meant as a temporary measure to fix the referenced bug till we can remove build_pipeline completely and make tsvb code responsible for its complete expression. When we do this, we can just not use kibana_context at all and choose to not deserialize the saved search source (right now this is happening automatically for all visualizations).

so i think the actual problem is why is there a searchSource query set for tsvb saved objects, if it shouldn't be as there is no query bar in tsvb application ?

I agree it shouldn't, but for now it has to have one because that's how the visualization saved object is structured and in some exotic cases (multiple updates, auto generated saved objects) actual data ends up in there, causing the referenced bug.

current plan was to not give tsvb control over that part of expression, visualize frame/embeddable would handle that internally.

any idea why we have those queries on the saved objects ? (why its not empty for tsvb) ?

lets make sure we fix that (so new tsvb panels no longer have that problem)

we can write a simple migration script (or migrate on the fly inside update_vis_state) to remove those if we know that none of saved tsvbs should have those set.

…ject

This reverts commit 50bf1c2.

flash1293 · 2020-08-20T13:08:50Z

@ppisljar I changed the approach and added a migration removing the searchSourceJSON for tsvb visualizations. This should have the same effect.

ppisljar

code LGTM, can you please also update the description of the PR ?

thanks!

…ject

kibanamachine · 2020-09-07T12:12:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0c67d6e

Build metrics

✅ unchanged

History

💚 Build #69379 succeeded 7769b16
💛 Build #69063 was flaky d47c12e
💚 Build #68378 succeeded 50bf1c2

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

…' of github.com:jloleysens/kibana into ilm/fix/pre-existing-policy-with-no-existing-repository * 'ilm/fix/pre-existing-policy-with-no-existing-repository' of github.com:jloleysens/kibana: fix empty string in selected indices (elastic#76855) [Security Solution] Refactor OverviewHost and OverviewNetwork to use Search Strategy (elastic#76409) Use Search API in Timelion (sync) (elastic#75115) [telemetry] expose getIsOptedIn function in plugin start contract (elastic#75143) [ILM] Clean up remaining js files and any typings (elastic#76803) [Logs UI] Shared `<LogStream />` component (elastic#76262) [Security Solution] Add unit test for all hosts (elastic#76752) [Security Solution] Add unit test for authentications search strategy (elastic#76665) Do not apply search source data for tsvb (elastic#75137) [Security Solution] Refactor NetworkDns to use Search Strategy (elastic#76250) [SECURITY SOLUTION] Adds 'cypress:open-as-ci' command (elastic#76125) [Logs UI] Update alert executor tests (elastic#75764) [Functional] Unskip vega tests and fix flakiness (elastic#76600) [Data] Query String Input accepts classname prop (elastic#76848) [ML] Swim lane pagination for viewing by job id (elastic#76847) [Security Solution] Refactor MatrixHistogram to use Search Strategy (elastic#76603) [APM] Use the outcome field to calculate the transaction error rate chart (elastic#75528) [APM] Use observer.hostname instead of observer.name (elastic#76074) Legacy logging: fix remoteAddress being duplicated in userAgent field (elastic#76751)

…s-for-710 * 'master' of github.com:elastic/kibana: [Search] Use async es client endpoints (elastic#76872) fix empty string in selected indices (elastic#76855) [Security Solution] Refactor OverviewHost and OverviewNetwork to use Search Strategy (elastic#76409) Use Search API in Timelion (sync) (elastic#75115) [telemetry] expose getIsOptedIn function in plugin start contract (elastic#75143) [ILM] Clean up remaining js files and any typings (elastic#76803) [Logs UI] Shared `<LogStream />` component (elastic#76262) [Security Solution] Add unit test for all hosts (elastic#76752) [Security Solution] Add unit test for authentications search strategy (elastic#76665) Do not apply search source data for tsvb (elastic#75137) [Security Solution] Refactor NetworkDns to use Search Strategy (elastic#76250) [SECURITY SOLUTION] Adds 'cypress:open-as-ci' command (elastic#76125) [Logs UI] Update alert executor tests (elastic#75764) [Functional] Unskip vega tests and fix flakiness (elastic#76600) [Data] Query String Input accepts classname prop (elastic#76848) [ML] Swim lane pagination for viewing by job id (elastic#76847) [Security Solution] Refactor MatrixHistogram to use Search Strategy (elastic#76603) [APM] Use the outcome field to calculate the transaction error rate chart (elastic#75528) [APM] Use observer.hostname instead of observer.name (elastic#76074) Legacy logging: fix remoteAddress being duplicated in userAgent field (elastic#76751) # Conflicts: # x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx # x-pack/plugins/index_lifecycle_management/common/types/index.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/services/api.ts

do not apply search source data for tsvb

50bf1c2

flash1293 added release_note:fix v7.10.0 v8.0.0 labels Aug 17, 2020

flash1293 marked this pull request as ready for review August 17, 2020 12:10

flash1293 requested a review from a team as a code owner August 17, 2020 12:10

flash1293 added Feature:TSVB TSVB (Time Series Visual Builder) Team:AppArch Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Aug 17, 2020

Merge branch 'master' into fix-tsvb-query-object

d47c12e

ppisljar requested changes Aug 19, 2020

View reviewed changes

flash1293 added 3 commits August 20, 2020 14:38

Merge remote-tracking branch 'upstream/master' into fix-tsvb-query-ob…

ab78f64

…ject

Revert "do not apply search source data for tsvb"

5d82514

This reverts commit 50bf1c2.

remove search source in migration

7769b16

ppisljar approved these changes Aug 25, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into fix-tsvb-query-ob…

0c67d6e

…ject

flash1293 merged commit 8705ab8 into elastic:master Sep 7, 2020

flash1293 mentioned this pull request Sep 7, 2020

[7.x] Do not apply search source data for tsvb (#75137) #76875

Merged

flash1293 added a commit to flash1293/kibana that referenced this pull request Sep 7, 2020

Do not apply search source data for tsvb (elastic#75137)

4a5eed9

flash1293 added a commit that referenced this pull request Sep 7, 2020

Do not apply search source data for tsvb (#75137) (#76875)

324760e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Do not apply search source data for tsvb #75137

Do not apply search source data for tsvb #75137

Uh oh!

flash1293 commented Aug 17, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

flash1293 commented Aug 19, 2020

Uh oh!

ppisljar Aug 19, 2020

Uh oh!

flash1293 Aug 19, 2020

Uh oh!

ppisljar Aug 19, 2020

Uh oh!

flash1293 commented Aug 20, 2020

Uh oh!

ppisljar left a comment

Uh oh!

kibanamachine commented Sep 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Do not apply search source data for tsvb #75137

Do not apply search source data for tsvb #75137

Uh oh!

Conversation

flash1293 commented Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release note

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

elasticmachine commented Aug 17, 2020

Uh oh!

flash1293 commented Aug 19, 2020

Uh oh!

ppisljar Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

flash1293 Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

ppisljar Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

flash1293 commented Aug 20, 2020

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Sep 7, 2020

💚 Build Succeeded

Build metrics

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

flash1293 commented Aug 17, 2020 •

edited

Loading