-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Infrastructure UI][Rules] Change bucket_scripts to use params for thresholds #133214
[Infrastructure UI][Rules] Change bucket_scripts to use params for thresholds #133214
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simianhacker so this looks like it's pulling two items off an existing array and storing them as numbered values? Can you help me understand what this actually changes?
@jasonrhodes This changes the format of the script so instead of writing the values as Painless constants, Functionality wise, it's the same but without the Painless casting error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, thanks for the explanation. These changes LGTM in general. I'll defer to you and @fkanout for the more complete testing but from our end,
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Summary
This PR fixes #133210 by changing the Metric Threshold Rule and Inventory Threshold Rule to use params for the thresholds instead of hard coding them.
Before:
After:
Checklist
Note to Reviewers
I'm aware that there is duplicate code between these two rules, specifically the
create_condition_script.ts
. When these two rules were written, they were developed simultaneously; both PRs sat in review for a while. The plan is to refactor both of these rules to remove the duplication while implementing #131205 Please consider "deduplicating code" out of scope for this PR.