Phase 1 of search services #46742

stacey-gammon · 2019-09-26T21:17:34Z

Summary

Introduction of the new pluggable search service. There are no actual users of this system yet except for the sample plugin which showcases the api's, and is functionally tested.

Fixes #43371

Jest test coverage

Further exploration

The full exploration PR can be seen at #45058. This is just the first phase - the new client and server search extension points, as well as the ability to search Elasticsearch in a synchronous manner.

The async version has been tested in the fuller exploration PR.

Overview

Search service provides:

the ability for plugins to register new search strategies via it's setup contract
the ability for applications to use the search functionality via application mount context.
the ability for routes to use the search functionality via route mount context.

Client side interaction

Client side + server side

Client and server side api's differ slightly. On the client side, the search function returns an observable to support progress reporting. On the server side it is always a promise since we don't have a use case yet for a server side component wanting to handle progress, and client side progress strategy will require the server to always immediately return, to the client, all partial results as they come in.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] 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
- Documentation in the form of "living documentation" inside the search explorer sample plugin. Run via yarn start --plugin-path test/plugin_functional/plugins/demo_search/ --plugin-path test/plugin_functional/plugins/search_explorer/ and navigate to the search explorer app to view documentation, links and working demos using the search services.
Unit or functional tests were updated or added to match the most common scenarios
- TODO
  ~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
- this is net new
This includes a feature addition or change that requires a release note and was labeled appropriately
- Skipping release notes as this API is too new to be documented or highlighted. It is still extremely volatile, and will be until we test it out with some real usage.

elasticmachine · 2019-09-26T22:27:31Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-30T17:27:01Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

NP search cleanup

elasticmachine · 2019-10-01T13:44:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-10-01T19:16:11Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-10-02T14:44:00Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

lukasolson · 2019-10-02T15:16:36Z

test/plugin_functional/test_suites/search/index.ts

+      await esArchiver.loadIfNeeded('../functional/fixtures/es_archiver/dashboard/current/data');
+      await esArchiver.load('../functional/fixtures/es_archiver/dashboard/current/kibana');
+      await kibanaServer.uiSettings.replace({
+        'dateFormat:tz': 'Australia/North',
+        defaultIndex: 'logstash-*',
+      });


Do we need to revert these in an after?

yuuupppp, added another commit (and fixed the ts error that was killing ci)

actually I reverted the data that was loaded. I don't think we need to change anything with the uiSettings, haven't seen that being "undone" before. Most tests set it explicitly.

…-search-api

Adding unit tests

…te, at least not yet.

elasticmachine · 2019-10-03T15:48:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 83eab32

…-search-api

stacey-gammon

LGTM

@lukasolson and I pair programmed a bit on this so giving my official LGTM, and I think we only need one more in additional to Lukas's.

elasticmachine · 2019-10-07T20:16:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2c9b24c

spalger

Jest config changes LGTM

ppisljar

can we add some documentation either to the README or to the pr description devdocs section ?
i see we have a demo showing how to register search strategy, but i am wondering how can i use this service if i just want to perform search ?

ppisljar · 2019-10-08T07:00:25Z

src/plugins/data/public/plugin.ts

@@ -20,16 +20,22 @@
 import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from '../../../core/public';
 import { AutocompleteProviderRegister } from './autocomplete_provider';
 import { DataPublicPluginSetup, DataPublicPluginStart } from './types';
+import { searchService } from './search';
+import { SearchService } from './search/search_service';


why import both, or why even have searchService function, all it does is new SearchService(params) ?

Copied the plugin way of doing things (this was its own separate plugin so this was the index.ts file, at the plugin level, this is export plugin..., here I renamed it export searchService ...). Not necessary except for conformity. It is pretty silly when you take it outside of the plugin level. I can remove.

ppisljar · 2019-10-08T07:00:52Z

src/plugins/data/public/plugin.ts


-  constructor(initializerContext: PluginInitializerContext) {}
+  constructor(initializerContext: PluginInitializerContext) {
+    this.searchService = searchService(initializerContext);


do we need to create this in the constructor or could it be postponed till setup() ?

We could but then we'd need to store the initializerContext and make searchService possibly undefined. Is there any benefit to postponing? It was my impression that constructor and setup are all run before Kibana loads up so it doesn't make a difference which it's in. If I'm wrong I can move it.

ppisljar · 2019-10-08T07:03:26Z

src/plugins/data/public/search/search_service.ts

+    return api;
+  }
+
+  public start(core: CoreStart) {}


hmm, how does this work ? setup() contract includes registerSearchStrategyContext and registerSearchStrategyProvider ... that makes sense

what about start method ? (i was expecting search function here)

search functionality is passed down via AppMountContext, or RouteHandlerContext. In NP land, I can't think of a situation they will need to be access via the start contract. We exposed it in Setup just for legacy land. We think we could expose in start too (or instead of). I don't think it really matters at that point.

exposing search in setup method means that who ever uses it has no idea if searchStrategies have been registered yet.
not exposing search in start: how can i pass search to any other plugin then ? my plugin would want to register esaggs function to expressions function registry, esaggs requires search, so we kind of need it in the contracts.

also ... it doesn't seem we expose it in setup currently (at least not on the client side)

Yea, I honestly couldn't think of a reason to expose it, but you just gave one example. It will be interesting though because esaggs will need to use it bound to the current user.

Mind if we punt on that for this PR and then we can work on adding it in for real to replace esaggs and make the adjustments needed that way?

ppisljar · 2019-10-08T07:05:45Z

test/plugin_functional/plugins/demo_search/public/plugin.ts

+  data: DataPublicPluginSetup;
+}
+
+declare module '../../../../../src/plugins/data/public' {


whats this for ?

adding a comment to the file:

/** * Add the typescript mappings for our search strategy to the request and * response types. This allows typescript to require the right shapes if * making the call: * const response = context.search.search(request, {}, DEMO_SEARCH_STRATEGY); * * If the caller does not pass in the right `request` shape, typescript will * complain. The caller will also get a typed response. */

elasticmachine · 2019-10-08T14:43:21Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 25ec030

…-search-api

stacey-gammon · 2019-10-08T14:50:16Z

can we add some documentation either to the README or to the pr description devdocs section ?
i see we have a demo showing how to register search strategy, but i am wondering how can i use this service if i just want to perform search ?

I don't really want to call this service out yet in devdocs given that it will likely go through so much change.

The demos do have code for searching both the ES_SEARCH_STRATEGY and the DEMO_SEARCH_STRATEGY (they do actually perform the searches). I called out the specific sections in this presentation: https://docs.google.com/presentation/d/1_nKsMnRYRTDDz6n_U7XRMfKrSEYXiNjYLMJD5ttR78o/edit#slide=id.g61fddd3178_0_3217 starting on slide 25, Hands on: using our search. The legacy, server side version is not tested, but described. The NP way of using search services is tested in the demo. lmk if you think that is sufficient.

…earch service

elasticmachine · 2019-10-08T16:06:59Z

💚 Build Succeeded

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

ppisljar · 2019-10-09T06:25:30Z

a demo of how can i use it in my plugin would be nice (so a plugin that does not register an application)
also a demo of how (or a slide) of how i can use it on client side in the old platform. (i was looking for npStart.plugins.search.search) something that can be used in SearchSource (which is still a legacy service)

stacey-gammon · 2019-10-09T14:29:23Z

a demo of how can i use it in my plugin would be nice (so a plugin that does not register an application)

Yea, this would be good, do you mind if I punt on this for this PR though? I think I'm going to hit the context issues described here: #46483. And probably require some syncing with platform team. I actually would hit it when migrating to https://github.com/elastic/kibana/pull/47337/files ... since then the search_explorer would be registering a section to the living documentation plugin, which would be similar to what you are asking. Also it'd be cool to add some documentation for the interpreter using this system, and we can use it that way too.

also a demo of how (or a slide) of how i can use it on client side in the old platform. (i was looking for npStart.plugins.search.search) something that can be used in SearchSource (which is still a legacy service)

This one I'm not sure is worth pointing out in the demo plugin since we are moving away from legacy. I just pushed another commit that exposes via start contract and confirmed it worked by using it inside default_search_strategy Courier:

elasticmachine · 2019-10-09T15:36:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 1678aec

elasticmachine · 2019-10-09T17:06:38Z

💚 Build Succeeded

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

ppisljar

LGTM

* Phase 1 of search services * First review feedback * Start on tests * Add functional tests for search explorer * Add unload and fix ts error * Add index.test.ts files for coverage completeness * Adding unit tests * use internal route terminology. No reason this should be a public route, at least not yet. * Move search service into data plugin * App mount search context needs to be optional * Add more unit tests for server stuff * wip types fix * fix types for new context container stuff * put back all jest test coverage paths * address review comments * delete the two test files that just tested the instantiation of the search service * expose search fn on StartContract... tested locally only * update mocks to account for new startcontract

elasticmachine · 2019-10-21T18:10:30Z

💚 Build Succeeded

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

stacey-gammon added the release_note:skip Skip the PR/issue when compiling release notes label Sep 26, 2019

stacey-gammon force-pushed the 2019-09-25-np-search-api branch from 1ab0729 to 098cd69 Compare September 30, 2019 16:21

stacey-gammon added v7.5.0 v8.0.0 labels Sep 30, 2019

stacey-gammon mentioned this pull request Sep 30, 2019

Data Access Search API: MVP/Phase 1 #43371

Closed

stacey-gammon force-pushed the 2019-09-25-np-search-api branch from 098cd69 to ddfed3f Compare September 30, 2019 18:25

Phase 1 of search services

e9d62d2

stacey-gammon force-pushed the 2019-09-25-np-search-api branch from ddfed3f to e9d62d2 Compare September 30, 2019 18:26

lukasolson and others added 2 commits September 30, 2019 17:04

First review feedback

1da04ee

Merge pull request #11 from lukasolson/np-search-api

d2c6787

NP search cleanup

Start on tests

c0d3070

stacey-gammon requested a review from a team as a code owner October 1, 2019 17:59

Add functional tests for search explorer

6c9a09a

lukasolson reviewed Oct 2, 2019

View reviewed changes

stacey-gammon and others added 6 commits October 2, 2019 11:48

Add unload and fix ts error

83806cc

Merge branch 'master' of github.com:elastic/kibana into 2019-09-25-np…

606dcc0

…-search-api

Add index.test.ts files for coverage completeness

00a22d6

Adding unit tests

ba50e95

Merge pull request #12 from lukasolson/np-search-api

83eab32

Adding unit tests

use internal route terminology. No reason this should be a public rou…

d75b3e9

…te, at least not yet.

Move search service into data plugin

1617ffa

stacey-gammon requested a review from a team as a code owner October 3, 2019 18:02

stacey-gammon added 2 commits October 3, 2019 14:11

Merge branch 'master' of github.com:elastic/kibana into 2019-09-25-np…

95bde79

…-search-api

App mount search context needs to be optional

ce571b5

stacey-gammon requested a review from ppisljar October 7, 2019 19:00

stacey-gammon commented Oct 7, 2019

View reviewed changes

stacey-gammon added the review label Oct 7, 2019

lukasolson approved these changes Oct 7, 2019

View reviewed changes

spalger approved these changes Oct 7, 2019

View reviewed changes

ppisljar reviewed Oct 8, 2019

View reviewed changes

address review comments

25ec030

Merge branch 'master' of github.com:elastic/kibana into 2019-09-25-np…

82db4e7

…-search-api

delete the two test files that just tested the instantiation of the s…

8621708

…earch service

expose search fn on StartContract... tested locally only

1678aec

update mocks to account for new startcontract

ce0a93f

ppisljar approved these changes Oct 10, 2019

View reviewed changes

stacey-gammon merged commit 3cb74bd into elastic:master Oct 10, 2019

stacey-gammon mentioned this pull request Oct 10, 2019

[7.x] Phase 1 of search services (#46742) #47819

Merged

This was referenced Oct 21, 2019

[Search service] Add async search strategy #48811

Closed

[Search service] Use outside application context #48850

Closed

[Search service] Add support for ES request preference #49424

Merged

lukasolson mentioned this pull request Nov 18, 2019

Default search params for ignore_unavailable and rest_total_hits_as_int #50953

Merged

1 task

lukasolson mentioned this pull request Jan 6, 2020

[Search service] Add support for frozen indices and rollups #54077

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phase 1 of search services #46742

Phase 1 of search services #46742

stacey-gammon commented Sep 26, 2019 •

edited by lukasolson

Loading

elasticmachine commented Sep 26, 2019

elasticmachine commented Sep 30, 2019

elasticmachine commented Oct 1, 2019

elasticmachine commented Oct 1, 2019

elasticmachine commented Oct 2, 2019

lukasolson Oct 2, 2019

stacey-gammon Oct 2, 2019

stacey-gammon Oct 2, 2019

elasticmachine commented Oct 3, 2019

stacey-gammon left a comment

elasticmachine commented Oct 7, 2019

spalger left a comment

ppisljar left a comment

ppisljar Oct 8, 2019

stacey-gammon Oct 8, 2019

ppisljar Oct 8, 2019

stacey-gammon Oct 8, 2019

ppisljar Oct 8, 2019

stacey-gammon Oct 8, 2019

ppisljar Oct 9, 2019

ppisljar Oct 9, 2019 •

edited

Loading

stacey-gammon Oct 9, 2019

ppisljar Oct 8, 2019

stacey-gammon Oct 8, 2019

elasticmachine commented Oct 8, 2019

stacey-gammon commented Oct 8, 2019

elasticmachine commented Oct 8, 2019

ppisljar commented Oct 9, 2019

stacey-gammon commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

ppisljar left a comment

elasticmachine commented Oct 21, 2019

Phase 1 of search services #46742

Phase 1 of search services #46742

Conversation

stacey-gammon commented Sep 26, 2019 • edited by lukasolson Loading

Summary

Jest test coverage

Further exploration

Overview

Client side interaction

Client side + server side

Checklist

For maintainers

elasticmachine commented Sep 26, 2019

💔 Build Failed

elasticmachine commented Sep 30, 2019

💔 Build Failed

elasticmachine commented Oct 1, 2019

💔 Build Failed

elasticmachine commented Oct 1, 2019

💔 Build Failed

elasticmachine commented Oct 2, 2019

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 3, 2019

💚 Build Succeeded

stacey-gammon left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 7, 2019

💚 Build Succeeded

spalger left a comment

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppisljar Oct 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 8, 2019

💔 Build Failed

stacey-gammon commented Oct 8, 2019

elasticmachine commented Oct 8, 2019

💚 Build Succeeded

ppisljar commented Oct 9, 2019

stacey-gammon commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

💔 Build Failed

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

ppisljar left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 21, 2019

💚 Build Succeeded

stacey-gammon commented Sep 26, 2019 •

edited by lukasolson

Loading

ppisljar Oct 9, 2019 •

edited

Loading