Skip to content

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

Merged
chrisronline merged 8 commits intoelastic:masterfrom
chrisronline:monitoring/remove_cluster_alerts
Jan 12, 2021
Merged

[Monitoring] Remove deprecated watcher-based cluster alerts#85047
chrisronline merged 8 commits intoelastic:masterfrom
chrisronline:monitoring/remove_cluster_alerts

Conversation

@chrisronline
Copy link
Copy Markdown
Contributor

@chrisronline chrisronline commented Dec 4, 2020 •
edited
Loading

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
Copy Markdown
Contributor

elasticmachine commented Dec 22, 2020

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

@chrisronline chrisronline mentioned this pull request Dec 22, 2020
Closed
3 tasks
@ravikesarwani
Copy link
Copy Markdown
Contributor

ravikesarwani commented Jan 4, 2021

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
Copy Markdown
Contributor Author

chrisronline commented Jan 4, 2021 •
edited
Loading

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
Copy Markdown
Contributor

ravikesarwani commented Jan 4, 2021

Linking to existing DELETE Watch API is fine.

@chrisronline chrisronline mentioned this pull request Jan 5, 2021
Merged
@chrisronline
Copy link
Copy Markdown
Contributor Author

chrisronline commented Jan 6, 2021

@elasticmachine merge upstream

@chrisronline chrisronline mentioned this pull request Jan 7, 2021
Closed
23 tasks
igoristic
igoristic approved these changes Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

chrisronline commented Jan 12, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jan 12, 2021

💚 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
@chrisronline @kibanamachine
[Monitoring] Remove deprecated watcher-based cluster alerts (elastic#…
d4e2f5f 
…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 chrisronline mentioned this pull request Jan 12, 2021
Merged
chrisronline added a commit that referenced this pull request Jan 12, 2021
@chrisronline @kibanamachine
[Monitoring] Remove deprecated watcher-based cluster alerts (#85047) (#…
2951a6e 
…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

Reviewers

1 more reviewer

@igoristic igoristic igoristic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@chrisronline chrisronline

Labels

release_note:enhancement review Team:Monitoring Stack Monitoring team v7.12.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chrisronline @elasticmachine @ravikesarwani @kibanamachine @igoristic