-
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
[RAC] [Metrics UI] Register Inventory rule types with new RAC rules registry #105706
[RAC] [Metrics UI] Register Inventory rule types with new RAC rules registry #105706
Conversation
3094ccb
to
7d81ac8
Compare
7d81ac8
to
9969d88
Compare
5c30733
to
b740326
Compare
b740326
to
43ac40e
Compare
1b47eea
to
352736d
Compare
352736d
to
c9778db
Compare
@elasticmachine merge upstream |
expected head sha didn’t match current head ref. |
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
@weltenwort I checked |
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.
This is definitely going into the right direction, but I left a few questions below.
x-pack/plugins/infra/public/alerting/inventory/rule_data_formatters.ts
Outdated
Show resolved
Hide resolved
.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
Outdated
Show resolved
Hide resolved
.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/public/alerting/inventory/rule_data_formatters.ts
Outdated
Show resolved
Hide resolved
@weltenwort There was a new PR merged e2aacdc#diff-f206eabccdf944b536c967e22ec43adb376f0e7322c92f5c7ee5217bda757f9fR65. I had removed the |
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.
I think we're almost there. Just stumbled across one typing issue which was hidden by an any
.
.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
Outdated
Show resolved
Hide resolved
.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
Outdated
Show resolved
Hide resolved
b136f30
to
193d243
Compare
… info for combined conditions
193d243
to
94955dd
Compare
I think the latest CI failure was caused by the force-push. The current |
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.
Well done, thanks for enduring the back-and-forth. 👏
Let's hope CI agrees 🤞 😉
💚 Build SucceededMetrics [docs]Module Count
Page load bundle
History
To update your PR or re-run it, just comment with: cc @mgiota |
…egistry (elastic#105706) * WIP: register inventory metric threshold as lifecycle rule * fix inventory executor error * save alerts into ES * temp * basic format reason for inventory threshold * clean up, fix i18n error and temporarily remove types * delete serialized params * include group name in the reason * cleanup * link to default metrics page * grab the value and threshold for the inventory item * fix typo * fix check types * remove threshold and currentValue, the reason field will contain this info for combined conditions * remove thereshold and value from the reason, soon will be replaced by indexed reason field * remove unnecessary export Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…egistry (#105706) (#106512) * WIP: register inventory metric threshold as lifecycle rule * fix inventory executor error * save alerts into ES * temp * basic format reason for inventory threshold * clean up, fix i18n error and temporarily remove types * delete serialized params * include group name in the reason * cleanup * link to default metrics page * grab the value and threshold for the inventory item * fix typo * fix check types * remove threshold and currentValue, the reason field will contain this info for combined conditions * remove thereshold and value from the reason, soon will be replaced by indexed reason field * remove unnecessary export Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: mgiota <[email protected]>
This PR Fixes #105746. It
registers Inventory rule type with new RAC rules registry. Alerts created from these registered rules show up in the observability alerting table.
Relates to:
#98360
#105785
Review notes
Current value is X (threshold of Y)
and should be fixed as part of another ticket. Similarly to the log threshold implementation the params need to be serialized into an inventory-threshold-rule-namespaced field in the doc.Inventory alert for {groupName}
. Once the reason filed is indexed we will display a more descriptive message. Relates to [Logs UI] Index reason in log threshold executor #106291