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

Fix alerting health API to consider rules in all spaces #100879

Merged

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented May 28, 2021

Resolves #99930.

In this PR, I'm fixing the alerting health APIs to consider rules in all spaces instead of the default space only.

@mikecote mikecote added release_note:fix Feature:Alerting v8.0.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.14.0 labels May 28, 2021
@mikecote mikecote self-assigned this May 28, 2021
Comment on lines +82 to +84
expect(health.alerting_framework_heath.decryption_health.status).to.eql('ok');
expect(health.alerting_framework_heath.execution_health.status).to.eql('ok');
expect(health.alerting_framework_heath.read_health.status).to.eql('ok');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is potential for flakiness here if ever a test from another file forgets to cleanup a rule that is in an error state or something along those lines.

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote mikecote marked this pull request as ready for review May 31, 2021 15:31
@mikecote mikecote requested a review from a team as a code owner May 31, 2021 15:31
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM! Verified by creating rules in different spaces and then forcing an execution error in some of the rules. Verified that the health reflected these errors instead of always saying ok. Also verified it worked with spaces disabled.

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM! Works as expected.

@mikecote
Copy link
Contributor Author

mikecote commented Jun 2, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @mikecote

@mikecote mikecote added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 2, 2021
@mikecote mikecote merged commit e607b58 into elastic:master Jun 2, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 2, 2021
* Initial commit

* Expand tests

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

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 2, 2021
…01193)

* Initial commit

* Expand tests

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

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alerting health API only considers rules in the default space
5 participants