Use Search API in TSVB #76274

VladLasitsa · 2020-08-31T12:12:16Z

Summary

Use Search API in TSVB.

Important note!

For rollup indexes was leavt old search API because new Search AP uses the elasticsearch.client.transport.request. Using thi endpont the TSVB Table visualization fails to load. The reason is the next elasticsearch error logged:

[error][data][elasticsearch] [illegal_argument_exception]: Unable to translate aggregation tree into Rollup. Aggregation [pivot] is of type [FilterAggregationBuilder] which is currently unsupported.

It seems, we currently can't use the transport request in tsvb rollup indexes (at least for table aggs).

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
This was checked for keyboard-only and screenreader accessibility
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

alexwizp · 2020-08-31T13:20:02Z

x-pack/plugins/rollup/server/plugin.ts

      const { addSearchStrategy } = visTypeTimeseries;
-      registerRollupSearchStrategy(callWithRequestFactoryShim, addSearchStrategy);


I think we should remove all code related to TSVB from the x-pack/plugins/rollup and move all into vis_type_timeseries_enhanced. @stratoula @timroes Any ideas?

It sounds good to me

VladLasitsa · 2020-09-02T13:51:24Z

@elasticmachine merge upstream

src/plugins/vis_type_timeseries/server/index.ts

...gins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts

sulemanof · 2020-09-02T15:16:00Z

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_request.ts

-        this.callWithRequest(SEARCH_METHOD, {
-          body,
-          index,
-          rest_total_hits_as_int: true,
-        })


Since this endpoint was replaced with the elasticsearch.client.transport.request in x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts, the TSVB Table visualization fails to load. The reason is the next elasticsearch error logged:

[error][data][elasticsearch] [illegal_argument_exception]: Unable to translate aggregation tree into Rollup. Aggregation [pivot] is of type [FilterAggregationBuilder] which is currently unsupported.

It seems, we currently can't use the transport request in tsvb rollup indexes (at least for table aggs).
My suggestion is to implement own search method for rollup_search_strategy.ts and use elasticsearch.client.asCurrentUser.rollup.rollupSearch instead.

elasticmachine · 2020-09-10T09:58:16Z

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

VladLasitsa · 2020-09-10T12:34:25Z

@elasticmachine merge upstream

sulemanof

Overall LGTM.
Checked locally, didn't notice any regressions.
Left few minor comments.

Could you please update the PR description?
There is lack of info about left legacy search api in rollup and the reason of it.

src/plugins/vis_type_timeseries/server/index.ts

...gins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts

x-pack/plugins/rollup/server/plugin.ts

src/plugins/vis_type_timeseries/server/lib/vis_data/get_table_data.js

VladLasitsa · 2020-09-14T08:04:29Z

@elasticmachine merge upstream

src/plugins/vis_type_timeseries/server/lib/vis_data/get_table_data.js

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_strategy.ts

stratoula

Code LGTM, tested it on safari, works fine 👍

VladLasitsa · 2020-09-16T10:16:50Z

@elasticmachine merge upstream

VladLasitsa · 2020-09-16T14:43:04Z

@elasticmachine merge upstream

kibanamachine · 2020-09-16T16:21:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c919964

Build metrics

distributable file count

id	value	diff	baseline
default	45903	-5	45908
oss	27724	-4	27728
total		-9

History

💔 Build #74730 failed a74b3fa
💚 Build #74072 succeeded 15062dd
💚 Build #74031 succeeded 331e7cf
💔 Build #73938 failed 685f262
💔 Build #73936 failed 30c436c

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

lukeelmers

App arch changes LGTM

jloleysens

ES UI changes look good to me, tested rollup plugin locally

* Use Search API for TSVB * Fixed ci * Fixed ci * Use constants * Fixed tests * Fixed ci * Fixed ci * Back old rollup search * Fixed test * Fixed tests * Fixed issue with series data * Fixed comments * Fixed comments * Fixed unit test * Deleted unused import * Fixed comments Co-authored-by: Elastic Machine <[email protected]>

* Use Search API for TSVB * Fixed ci * Fixed ci * Use constants * Fixed tests * Fixed ci * Fixed ci * Back old rollup search * Fixed test * Fixed tests * Fixed issue with series data * Fixed comments * Fixed comments * Fixed unit test * Deleted unused import * Fixed comments Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Uladzislau Lasitsa <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

…rok/new-patterns-component-use-array * 'master' of github.com:elastic/kibana: (140 commits) Add telemetry as an automatic privilege grant (elastic#77390) [Security Solutions][Cases] Cases Redesign (elastic#73247) Use Search API in TSVB (elastic#76274) [Mappings editor] Add support for constant_keyword field type (elastic#76564) [ML] Adds ML modules for Metrics UI Integration (elastic#76460) [Drilldowns] {{event.points}} in URL drilldown for VALUE_CLICK_TRIGGER (elastic#76771) Migrate status & stats APIs to KP + remove legacy status lib (elastic#76054) use App updater API instead of deprecated chrome.navLinks.update (elastic#77708) [CSM Dashboard] Remove points from line chart (elastic#77617) [APM] Trace timeline: Replace multi-fold function icons with new EuiIcon glyphs (elastic#77470) [Observability] Overview: Alerts section style improvements (elastic#77670) Bump the Node.js version used by Docker in CI (elastic#77714) Upgrade all minimist (sub)dependencies to version ^1.2.5 (elastic#60284) Remove unneeded forced package resolutions (elastic#77467) [ML] Add metrics app to check made for internal custom URLs (elastic#77627) Functional tests - add supertest for test_user (elastic#77584) [ML] Adding option to create AD jobs without starting the datafeed (elastic#77484) Bump node-fetch to 2.6.1 (elastic#77445) Bump sharkdown from v0.1.0 to v0.1.1 (elastic#77607) [APM]fixing y axis on transaction error rate to 100% (elastic#77609) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.container.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/text_editor.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/processors/grok.test.tsx

Use Search API for TSVB

f9d4730

VladLasitsa requested a review from alexwizp August 31, 2020 12:12

Fixed ci

3d8e131

alexwizp reviewed Aug 31, 2020

View reviewed changes

VladLasitsa added 3 commits August 31, 2020 16:52

Fixed ci

261ed7f

Use constants

8e5fdef

Fixed tests

a0fd855

alexwizp requested a review from sulemanof September 1, 2020 08:57

VladLasitsa added 2 commits September 1, 2020 13:48

Fixed ci

23048fc

Fixed ci

ad6f51e

Merge branch 'master' into elastic#67587

4d08f9d

sulemanof suggested changes Sep 2, 2020

View reviewed changes

VladLasitsa added 6 commits September 8, 2020 11:00

Merge remote-tracking branch 'upstream/master' into elastic#67587

7185669

Back old rollup search

208b9ca

Fixed test

2732d26

Fixed tests

e988883

Fixed issue with series data

3d70a9b

Fixed comments

77e3608

VladLasitsa marked this pull request as ready for review September 10, 2020 09:57

VladLasitsa requested a review from a team as a code owner September 10, 2020 09:57

VladLasitsa requested a review from a team September 10, 2020 09:57

VladLasitsa requested a review from a team as a code owner September 10, 2020 09:57

VladLasitsa self-assigned this Sep 10, 2020

VladLasitsa added release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 labels Sep 10, 2020

VladLasitsa added the v8.0.0 label Sep 10, 2020

Merge branch 'master' into elastic#67587

1bcb4c6

sulemanof approved these changes Sep 11, 2020

View reviewed changes

VladLasitsa added 3 commits September 11, 2020 17:37

Fixed comments

30c436c

Fixed unit test

685f262

Deleted unused import

1549d39

Merge branch 'master' into elastic#67587

331e7cf

stratoula reviewed Sep 14, 2020

View reviewed changes

src/plugins/vis_type_timeseries/server/lib/vis_data/get_table_data.js Outdated Show resolved Hide resolved

stratoula reviewed Sep 14, 2020

View reviewed changes

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_strategy.ts Outdated Show resolved Hide resolved

Fixed comments

15062dd

stratoula approved these changes Sep 14, 2020

View reviewed changes

Merge branch 'master' into elastic#67587

a74b3fa

Merge branch 'master' into elastic#67587

c919964

lukeelmers approved these changes Sep 16, 2020

View reviewed changes

jloleysens self-requested a review September 17, 2020 14:08

jloleysens approved these changes Sep 17, 2020

View reviewed changes

stratoula merged commit 75a1459 into elastic:master Sep 17, 2020

stratoula mentioned this pull request Sep 17, 2020

[7.x] Use Search API in TSVB (#76274) #77767

Merged

alexwizp mentioned this pull request Sep 29, 2020

[TSVB] No data in visualizations with annotations #78789

Closed

alexwizp mentioned this pull request Nov 5, 2020

[TSVB] use new Search API for rollup search #82710

Closed

alexwizp mentioned this pull request Nov 16, 2020

[TSVB] use new Search API for rollup search #83275

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Search API in TSVB #76274

Use Search API in TSVB #76274

VladLasitsa commented Aug 31, 2020 •

edited

Loading

alexwizp Aug 31, 2020

stratoula Sep 14, 2020

VladLasitsa commented Sep 2, 2020

sulemanof Sep 2, 2020

elasticmachine commented Sep 10, 2020

VladLasitsa commented Sep 10, 2020

sulemanof left a comment

VladLasitsa commented Sep 14, 2020

stratoula left a comment

VladLasitsa commented Sep 16, 2020

VladLasitsa commented Sep 16, 2020

kibanamachine commented Sep 16, 2020

lukeelmers left a comment

jloleysens left a comment

		const { addSearchStrategy } = visTypeTimeseries;
		registerRollupSearchStrategy(callWithRequestFactoryShim, addSearchStrategy);

Use Search API in TSVB #76274

Use Search API in TSVB #76274

Conversation

VladLasitsa commented Aug 31, 2020 • edited Loading

Summary

Important note!

Checklist

For maintainers

alexwizp Aug 31, 2020

Choose a reason for hiding this comment

stratoula Sep 14, 2020

Choose a reason for hiding this comment

VladLasitsa commented Sep 2, 2020

sulemanof Sep 2, 2020

Choose a reason for hiding this comment

elasticmachine commented Sep 10, 2020

VladLasitsa commented Sep 10, 2020

sulemanof left a comment

Choose a reason for hiding this comment

VladLasitsa commented Sep 14, 2020

stratoula left a comment

Choose a reason for hiding this comment

VladLasitsa commented Sep 16, 2020

VladLasitsa commented Sep 16, 2020

kibanamachine commented Sep 16, 2020

💚 Build Succeeded

Build metrics

distributable file count

History

lukeelmers left a comment

Choose a reason for hiding this comment

jloleysens left a comment

Choose a reason for hiding this comment

VladLasitsa commented Aug 31, 2020 •

edited

Loading