Expose Elasticsearch from start and deprecate from setup #59886

rudolf · 2020-03-11T11:23:06Z

Summary

As the first step of #55397 we deprecate the CoreSetup.elasticsearch API and expose CoreStart.elasticsearch instead.

We didn't previously feel like it's a high-enough priority to justify working on it, but since we're anyway making a breaking change to the API I thought it might be a good time to unify the admin and data clients as per #49870 and only expose elasticsearch.client and elasticsearch.createClient

Dev Docs

To remove any API that could allow to access/query savedObjects from core setup contract, and move them to the start contract instead. Deprecate the CoreSetup.elasticsearch API and expose CoreStart.elasticsearch instead.

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, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-03-11T11:23:09Z

Pinging @elastic/kibana-platform (Team:Platform)

mshustov · 2020-03-11T14:36:02Z

src/core/server/elasticsearch/elasticsearch_service.ts

+  public async start() {
+    if (typeof this.adminClient === 'undefined' || typeof this.createClient === 'undefined') {
+      throw new Error('ElasticsearchService needs to be setup before calling start');
+    } else {


nit: unnecessary else

mshustov · 2020-03-11T14:41:50Z

src/core/server/elasticsearch/elasticsearch_service.ts

    return {
      legacy: { config$: clients$.pipe(map(clients => clients.config)) },
-
-      adminClient,
-      dataClient,
      esNodesCompatibility$,


~~shouldn't move to start as well?~~ ok, probably after removing adminClient & dataClient

mshustov · 2020-03-11T14:44:17Z

src/core/server/elasticsearch/types.ts

@@ -43,6 +43,7 @@ export interface ElasticsearchServiceSetup {
   * const client = elasticsearch.createCluster('my-app-name', config);
   * const data = await client.callAsInternalUser();
   * ```
+   * @deprecated


could you add a comment what is the blessed way to use cluster from now on?

mshustov · 2020-03-11T14:50:55Z

src/core/server/elasticsearch/elasticsearch_service.ts

+      throw new Error('ElasticsearchService needs to be setup before calling start');
+    } else {
+      return {
+        client: this.adminClient,


What if we reserve these property names for an elasticsearch-js compatible client? #35508
We can group them under legacy namespace, for example.

I agree, this is a good time to prepare for that. It could be confusing if there's a "legacy" client, but we won't provide any alternative until 8.0, but I don't have a better suggestion. It would save a lot of refactoring and risk if we can prevent it, so I think it's worth the possible short term confusion for the long term benefits.

but we won't provide any alternative until 8.0,

why not? I'd expect it to land in v7.8

Ah, I somehow missed that!

mshustov · 2020-03-11T14:55:20Z

src/core/server/internal_types.ts

@@ -49,6 +49,7 @@ export interface InternalCoreSetup {
 */
 export interface InternalCoreStart {
  capabilities: CapabilitiesStart;
+  elasticsearch: ElasticsearchServiceStart;


Shouldn't we migrate the platform code to the new API? I see that SO still uses the deprecated one. It can be done as a follow-up.

And we should update the migration guide accordingly.

Shouldn't we migrate the platform code to the new API? I see that SO still uses the deprecated one. It can be done as a follow-up.

I've added a task to #55397

mshustov

I can see that our migration guide and examples still reference adminClient& dataClient. Added as follow-ups to #55397

kibanamachine · 2020-03-14T20:18:45Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-03-15T20:28:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…o alerting/view-in-app * 'alerting/view-in-app' of github.com:gmmorris/kibana: (33 commits) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) [SIEM] [Case] Insert timeline bugfix and limit 25 cases (elastic#60136) [ML] Client side cut over (elastic#60100) Disable query bar on service map routes (elastic#60118) Move subscribe_with_scope to kibana_legacy (elastic#59781) [Ingest] Agent Config create with sys monitoring (elastic#60111) [Watcher UI] Not possible to edit a watch that was created with the API if the ID contains a dot (elastic#59383) Actually fetch functionbeat fields needed for telemetry (elastic#60054) ...

…0163) * Expose Elasticsearch from start and deprecate from setup * Expose client under legacy namespace, add deprecated note * Update migration guide Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: (40 commits) skips 'config_open.ts' files from linter check (elastic#60248) [Searchprofiler] Spacing between rendered shards (elastic#60238) Add UiSettings validation & Kibana default route redirection (elastic#59694) [SIEM][CASE] Change configuration button (elastic#60229) [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) ...

kibanamachine · 2020-04-10T18:10:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 860e964

Test Failures

Kibana Pipeline / kibana-intake-agent / Jest Integration Tests.packages/kbn-plugin-generator/integration_tests.running the plugin-generator via 'node scripts/generate_plugin.js plugin-name' with default config then running with es instance 'yarn start' should result in the spec plugin being initialized on kibana's stdout

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 4 times on tracked branches: https://github.com/elastic/kibana/issues/54481

Stack Trace

Error: Command failed with exit code 126: ./bin/elasticsearch-keystore create
    at makeError (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/execa/lib/error.js:56:11)
    at handlePromise (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/execa/index.js:114:26)
    at process._tickCallback (internal/process/next_tick.js:68:7)

History

💚 Build #32947 succeeded 860e964

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

* x-pack/watcher: use Elasticsearch from CoreStart * x-pack/upgrade_assistant: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/lens: use Elasticsearch from CoreStart * expressions: use Elasticsearch from CoreStart * x-pack/remote_clusters: remove unused Elasticsearch dependency on CoreSetup * x-pack/oss_telemetry: use Elasticsearch from CoreStart * Cleanup after #59886 * x-pack/watcher: create custom client only once * Revert "x-pack/watcher: create custom client only once" This reverts commit 78fc4d2. * Revert "x-pack/watcher: use Elasticsearch from CoreStart" This reverts commit b621af9. * x-pack/task_manager: use Elasticsearch from CoreStart * x-pack/event_log: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/apm: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * PR Feedback * APM review nits * Remove unused variable * Remove unused variable * x-pack/apm: better typesafety Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* x-pack/watcher: use Elasticsearch from CoreStart * x-pack/upgrade_assistant: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/lens: use Elasticsearch from CoreStart * expressions: use Elasticsearch from CoreStart * x-pack/remote_clusters: remove unused Elasticsearch dependency on CoreSetup * x-pack/oss_telemetry: use Elasticsearch from CoreStart * Cleanup after elastic#59886 * x-pack/watcher: create custom client only once * Revert "x-pack/watcher: create custom client only once" This reverts commit 78fc4d2. * Revert "x-pack/watcher: use Elasticsearch from CoreStart" This reverts commit b621af9. * x-pack/task_manager: use Elasticsearch from CoreStart * x-pack/event_log: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/apm: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * PR Feedback * APM review nits * Remove unused variable * Remove unused variable * x-pack/apm: better typesafety Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

#64329) * Refactor Plugins to access elasticsearch from CoreStart (#59915) * x-pack/watcher: use Elasticsearch from CoreStart * x-pack/upgrade_assistant: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/lens: use Elasticsearch from CoreStart * expressions: use Elasticsearch from CoreStart * x-pack/remote_clusters: remove unused Elasticsearch dependency on CoreSetup * x-pack/oss_telemetry: use Elasticsearch from CoreStart * Cleanup after #59886 * x-pack/watcher: create custom client only once * Revert "x-pack/watcher: create custom client only once" This reverts commit 78fc4d2. * Revert "x-pack/watcher: use Elasticsearch from CoreStart" This reverts commit b621af9. * x-pack/task_manager: use Elasticsearch from CoreStart * x-pack/event_log: use Elasticsearch from CoreStart * x-pack/alerting: use Elasticsearch from CoreStart * x-pack/apm: use Elasticsearch from CoreStart * x-pack/actions: use Elasticsearch from CoreStart * PR Feedback * APM review nits * Remove unused variable * Remove unused variable * x-pack/apm: better typesafety Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Fix event log tests Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Mar 11, 2020

Expose Elasticsearch from start and deprecate from setup

d3426d6

rudolf force-pushed the core-start-elasticsearch branch from f7d9867 to d3426d6 Compare March 11, 2020 11:25

rudolf marked this pull request as ready for review March 11, 2020 11:25

rudolf requested a review from a team as a code owner March 11, 2020 11:25

rudolf added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.7.0 v8.0.0 labels Mar 11, 2020

mshustov reviewed Mar 11, 2020

View reviewed changes

rudolf mentioned this pull request Mar 11, 2020

Refactor Plugins to access elasticsearch from CoreStart #59915

Merged

7 tasks

mshustov reviewed Mar 11, 2020

View reviewed changes

rudolf added 3 commits March 12, 2020 21:05

Merge branch 'master' into core-start-elasticsearch

d2a5b37

Expose client under legacy namespace, add deprecated note

d1e9756

Update migration guide

860e964

rudolf requested a review from mshustov March 13, 2020 09:54

mshustov approved these changes Mar 13, 2020

View reviewed changes

rudolf merged commit 6e6325b into elastic:master Mar 13, 2020

rudolf deleted the core-start-elasticsearch branch March 13, 2020 20:17

rudolf mentioned this pull request Mar 13, 2020

[7.x] Expose Elasticsearch from start and deprecate from setup (#59886) #60163

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 14, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 16, 2020

rudolf added a commit to rudolf/kibana that referenced this pull request Mar 17, 2020

Cleanup after elastic#59886

06c19f5

pgayvallet mentioned this pull request May 26, 2020

Move elasticsearch client creation/access API from core setup to start contract #55397

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Elasticsearch from start and deprecate from setup #59886

Expose Elasticsearch from start and deprecate from setup #59886

rudolf commented Mar 11, 2020 •

edited by jinmu03

Loading

elasticmachine commented Mar 11, 2020

mshustov Mar 11, 2020

mshustov Mar 11, 2020

mshustov Mar 11, 2020

mshustov Mar 11, 2020

rudolf Mar 12, 2020

mshustov Mar 13, 2020 •

edited

Loading

rudolf Mar 13, 2020

mshustov Mar 11, 2020

mshustov Mar 11, 2020

rudolf Mar 12, 2020

mshustov left a comment

kibanamachine commented Mar 14, 2020

kibanamachine commented Mar 15, 2020

kibanamachine commented Apr 10, 2020

Standard Out

Stack Trace

Expose Elasticsearch from start and deprecate from setup #59886

Expose Elasticsearch from start and deprecate from setup #59886

Conversation

rudolf commented Mar 11, 2020 • edited by jinmu03 Loading

Summary

Dev Docs

Checklist

For maintainers

elasticmachine commented Mar 11, 2020

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

mshustov Mar 13, 2020 • 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

Choose a reason for hiding this comment

mshustov left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 14, 2020

kibanamachine commented Mar 15, 2020

kibanamachine commented Apr 10, 2020

💔 Build Failed

Standard Out

Stack Trace

History

rudolf commented Mar 11, 2020 •

edited by jinmu03

Loading

mshustov Mar 13, 2020 •

edited

Loading