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

[Monitoring] Remove deprecated watcher-based cluster alerts #85047

Merged

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Dec 4, 2020

Relates to #81020

This PR adds logic to call a new API introduced by the ES team (elastic/elasticsearch#50032) to disable and delete legacy watcher-based cluster alerts.

I'm not sure if we should surface a toast to the user if this action fails (the PR currently does do this), and if we do, I'm not sure what it should say.

In a scenario where the API call fails, the user needs to review the error message and most likely manually delete the watches on the affected cluster(s). We should probably supplement this work with an update to our docs to tell users what this is and steps on removing the watches manually.

Curious to thoughts from @igoristic and @ravikesarwani

@chrisronline chrisronline marked this pull request as ready for review December 22, 2020 15:35
@chrisronline chrisronline requested a review from a team December 22, 2020 15:35
@chrisronline chrisronline self-assigned this Dec 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@ravikesarwani
Copy link
Contributor

I am assuming the API is called after we have successfully created replacement Kibana alerts.
If we failed to delete legacy watcher-based cluster alerts we should show a message to the user.
In the message I think 2 thing should be mentioned:

  • Created replacement Kibana alerts.
  • Failed to delete legacy watcher based cluster alerts and some quick glimpse of how they can delete it manually.

@chrisronline
Copy link
Contributor Author

chrisronline commented Jan 4, 2021

Thanks @ravikesarwani!

some quick glimpse of how they can delete it manually.

I don't think we have any docs for this, outside of general watch deletion docs. Should we create new ones, or link to what we have?

@ravikesarwani
Copy link
Contributor

Linking to existing DELETE Watch API is fine.

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

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.

@chrisronline Great job 🥇

We should make a note though to revert this PR if for some reason: #87377 does not get merged into 7.12

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@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
monitoring 979.8KB 981.2KB +1.5KB

History

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

@chrisronline chrisronline merged commit a21ad0d into elastic:master Jan 12, 2021
@chrisronline chrisronline deleted the monitoring/remove_cluster_alerts branch January 12, 2021 16:48
chrisronline added a commit to chrisronline/kibana that referenced this pull request Jan 12, 2021
…85047)

* First draft

* Update to use actual API

* Remove this file

* Update translation key

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
chrisronline added a commit that referenced this pull request Jan 12, 2021
…88043)

* First draft

* Update to use actual API

* Remove this file

* Update translation key

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants