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

[Metrics UI] Add warning severity to Metric Alerts #90070

Merged
merged 10 commits into from
Feb 9, 2021

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Feb 2, 2021

Summary

Closes #88591
Closes #89912
Should also fix #89807

Screenshots are from the Metric Threshold alert type, but the Warning threshold is also available on the Inventory alert type. Test them both during PR review.

106316537-87bb0680-6232-11eb-9d55-ec415a7d2b20

106329655-77615680-6247-11eb-8fff-0a44a7e50fc2

Screen Shot 2021-02-01 at 5 01 04 PM

Checklist

@Zacqary Zacqary added release_note:enhancement Feature:Metrics UI Metrics UI feature v8.0.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.12.0 labels Feb 2, 2021
@Zacqary Zacqary requested a review from a team as a code owner February 2, 2021 19:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@Zacqary Zacqary changed the title [Metrics UI] Add warning severity to Metric Threshold Alert [Metrics UI] Add warning severity to Metric Alerts Feb 3, 2021
…vels

# Conflicts:
#	x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx
@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
infra 1.9MB 1.9MB +16.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
infra 172.5KB 173.8KB +1.4KB

History

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

Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

I have 10 hosts reporting an average around 12-20%, then I have one host that is pushing 100% (which averages out to about 50% (ish). When I set the threshold to 50% I would expect the preview to say something like

There were 60 instances that satisfied the conditions of this alert across 1 kubernetes.pod.uids in the last hour.

but instead it says

There were 60 instances that satisfied the conditions of this alert across 10 kubernetes.pod.uids in the last hour.

It feels like we are counting too many UUIDs for the match. When the alert runs, it only matches on one UUID.

image

@Zacqary
Copy link
Contributor Author

Zacqary commented Feb 9, 2021

@simianhacker That's unrelated to this PR, we can file an issue for that. It's currently the expected behavior (telling the user that the alert examined 10 pods total) but we can revisit the language again since it's ambiguous.

Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.12.0 v8.0.0
Projects
None yet
4 participants