Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Alerts] Replaces legacy es client with the ElasticsearchClient for alerts and triggers_actions_ui plugins. #93364

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Mar 3, 2021

Resolve #50247

Did the proper changes for the alerts plugin:

  1. Removed dependencies on the legacy client
  2. Removed callCluster from the list of available services
  3. Replaced usage of callCluster: services.callCluster, with esClient: services.scopedClusterClient, in all alerting dependent plugins.
  4. Fixed tests by adding elasticsearchClientMock.createScopedClusterClient()

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.13.0 labels Mar 3, 2021
@YulNaumenko YulNaumenko self-assigned this Mar 3, 2021
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 3, 2021 03:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@YulNaumenko YulNaumenko changed the title [Alerts] Replaces legasy es client with the ElasticsearchClient [Alerts] Replaces legacy es client with the ElasticsearchClient for alerts plugin Mar 3, 2021
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 3, 2021 19:21
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 3, 2021 19:26
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 5, 2021 17:27
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 6, 2021 02:22
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Mar 6, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a lot of:

Alert "50cb60a0-80d4-11eb-aaaa-a502e7dca037" has resulted in Error: callCluster is not a function

This is because we need to also change any instance of

const response = await callCluster('search', params);`

in monitoring/server/lib/alerts/fetch*queries to:

const response = await callCluster.search(params);

Also, we still need to wrap the response promise (alert fetch queries) in mbSafeQuery in case they also have metricbeat-* indices, since it might break/block the UI under certain conditions

@@ -34,5 +34,5 @@ export const getIndexVersion = async (
return 0;
}
const writeIndexMapping = await readIndex(esClient, writeIndex);
return get(writeIndexMapping, [writeIndex, 'mappings', '_meta', 'version']) ?? 0;
return get(writeIndexMapping, ['body', writeIndex, 'mappings', '_meta', 'version']) ?? 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@YulNaumenko YulNaumenko requested a review from igoristic March 15, 2021 23:55
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 16, 2021 01:07
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Mar 16, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uptime changes LGTM !!

Copy link
Contributor

@dhurley14 dhurley14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for all the work to get the tests passing. Tested out running some rules locally and everything looked good 👍

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stack Monitoring changes look good 👍 Thank you for adding this!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
stackAlerts 682.6KB 682.6KB +3.0B

History

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

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit 21587dc into elastic:master Mar 16, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 16, 2021
…lerts and triggers_actions_ui plugins. (elastic#93364)

* [Alerts] Replaces legasy es client with the ElasticsearchClient

* fixed build

* fixed build

* fixed ci build

* fixed ci build

* fixed infra callCLuster

* fixed infra callCLuster

* fixed infra callCLuster

* fixed ci build

* fixed ci build

* fixed ci build

* fixed infra tests

* fixed security tests

* fixed security tests

* fixed security tests

* fixed tests

* fixed monitoring unit tests

* fixed monitoring unit tests

* fixed type checks

* fixed type checks

* fixed type checks

* migrated lists plugin

* fixed type checks

* fixed tests

* fixed security tests

* fixed type checks

* fixed tests

* fixed type checks

* fixed tests

* fixed tests

* fixed tests

* fixed due to comments

* fixed tests

* fixed comment

* fixed tests

* fixed tests

* fixed searh

* fixed searh

* fixed test

* fixed due to comment

* fixed detections failing test and replaces scopedClusterClient exposure with IScopedClusterClient instead of ElasticsearchClient asCurrentUser

* fixed test

* fixed test

* fixed test

* fixed typecheck

* fixed typecheck

* fixed typecheck

* fixed merge
YulNaumenko added a commit that referenced this pull request Mar 17, 2021
… for alerts and triggers_actions_ui plugins. (#93364) (#94747)

* [Alerts] Replaces legacy es client with the ElasticsearchClient for alerts and triggers_actions_ui plugins. (#93364)

* [Alerts] Replaces legasy es client with the ElasticsearchClient

* fixed build

* fixed build

* fixed ci build

* fixed ci build

* fixed infra callCLuster

* fixed infra callCLuster

* fixed infra callCLuster

* fixed ci build

* fixed ci build

* fixed ci build

* fixed infra tests

* fixed security tests

* fixed security tests

* fixed security tests

* fixed tests

* fixed monitoring unit tests

* fixed monitoring unit tests

* fixed type checks

* fixed type checks

* fixed type checks

* migrated lists plugin

* fixed type checks

* fixed tests

* fixed security tests

* fixed type checks

* fixed tests

* fixed type checks

* fixed tests

* fixed tests

* fixed tests

* fixed due to comments

* fixed tests

* fixed comment

* fixed tests

* fixed tests

* fixed searh

* fixed searh

* fixed test

* fixed due to comment

* fixed detections failing test and replaces scopedClusterClient exposure with IScopedClusterClient instead of ElasticsearchClient asCurrentUser

* fixed test

* fixed test

* fixed test

* fixed typecheck

* fixed typecheck

* fixed typecheck

* fixed merge

* fixed merge issues
jloleysens added a commit that referenced this pull request Mar 17, 2021
…-action

* 'master' of github.com:elastic/kibana: (44 commits)
  Migrate the optimizer mixin to core (#94272)
  Replace EuiCodeBlock with JsonCodeEditor in DiscoverGrid (#92442)
  [ML] Anomaly Detection: Migrate validation messages links to use docLinks. (#94568)
  [Lists][Exceptions] - Adding basic linting, i18n and storybook support (#94772)
  [Fleet] Add test/fix for invalid/missing ids in bulk agent reassign (#94632)
  [Security Solution] [Cases] Move create page components and dependencies to Cases (#94444)
  [ML] Data Frame Analytics accessibility tests: fix flaky outlier creation test (#94735)
  [Security Solutions] Remove commented out old linter rules (#94753)
  [App Search] Role mappings migration part 2 (#94461)
  [CI] Update Backport action inputs to match updated ones (#94721)
  [chore] Remove the infra team from CODEOWNERS (#94740)
  [Connectors UI] Make UI use new connector APIs (#94180)
  [ML] Use indices options in anomaly detection job wizards (#91830)
  Remove `string` as a valid registry var type value (#94174)
  [Alerts] Replaces legacy es client with the ElasticsearchClient for alerts and triggers_actions_ui plugins. (#93364)
  [Reporting-CSV Export] Re-write CSV Export using SearchSource (#88303)
  chore(NA): upgrade bazel rules nodejs to v3.2.2 (#94726)
  [APM] Settings: Update layout and update/add descriptions (#94398)
  skip flaky suite (#94666)
  [XY axis] Integrates legend color picker with the eui palette (#90589)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/cold_phase/cold_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts
#	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/form/configuration_context.tsx
#	x-pack/plugins/index_lifecycle_management/public/shared_imports.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use new elasticsearch-js client everywhere and remove usage of legacy client
9 participants