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.

Sign up for GitHub

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

Jump to bottom

[Metrics UI] Prefill alerts from the global dropdown #68967

Merged
merged 20 commits into from
Jun 25, 2020
Merged

[Metrics UI] Prefill alerts from the global dropdown #68967

merged 20 commits into from
Jun 25, 2020

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Jun 11, 2020 •
edited
Loading

Summary

Closes #60659

Metric Threshold and Inventory alerts will now pre-populate based on your current filters when you create an alert from the Alerts dropdown:
Screen Shot 2020-06-11 at 4 47 29 PM

When you see this Inventory screen

Screen Shot 2020-06-11 at 4 43 46 PM

The alert prefills with this

Screen Shot 2020-06-11 at 4 43 51 PM

From this Metrics Explorer

Screen Shot 2020-06-11 at 4 45 50 PM

To this alert

Screen Shot 2020-06-11 at 4 46 12 PM

@Zacqary Zacqary added release_note:enhancement Feature:Alerting 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.9.0 labels Jun 11, 2020
@Zacqary Zacqary requested a review from a team as a code owner June 11, 2020 21:50
@elasticmachine
Copy link
Contributor

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

@Zacqary
Copy link
Contributor Author

Zacqary commented Jun 17, 2020

@elasticmachine merge upstream

@simianhacker simianhacker self-requested a review June 17, 2020 19:34
simianhacker
simianhacker requested changes Jun 17, 2020
View reviewed changes
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.

Everything looks really good.

I want to start making an effort to start filling in some of the test coverage for our app. As a team (I'm probably the most guilty) we've been really laxed about testing, I want to work on changing that.

In that spirit, I added some comments about updating a test and adding tests for some of the hooks that don't have them. For some of the hooks that don't have tests where we've added the useAlertPrefillContext() it's ok to set up the minimum to just test the new code. If you have the extra time and want to fill in test for the rest of the functionality, that would really be appreciated but it's not required since technically it's out of the scope of this PR. Eventually, we will need to fill those in but that can be done later as "tech debt" cleanup.

...ck/plugins/infra/public/pages/metrics/metrics_explorer/hooks/use_metrics_explorer_options.ts Show resolved Hide resolved
x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_waffle_options.ts Show resolved Hide resolved
x-pack/plugins/infra/public/pages/metrics/inventory_view/hooks/use_waffle_filters.ts Show resolved Hide resolved
x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx Show resolved Hide resolved
@Zacqary Zacqary requested a review from a team as a code owner June 18, 2020 22:03
mikecote
mikecote approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Triggers actions UI code LGTM. Great seeing UX enhancements like this :)

@Zacqary
Copy link
Contributor Author

Zacqary commented Jun 23, 2020

@elasticmachine merge upstream

@Zacqary
Copy link
Contributor Author

Zacqary commented Jun 23, 2020

@simianhacker Added all the tests you requested. New test files are just testing the new prefill functionality in this PR, but there's a framework to fill them out when we cover tech debt

@simianhacker
Copy link
Member

For some reason this PR shows two headers:

image

and on the Metrics Explorer page I'm getting this error:

image

@Zacqary
Copy link
Contributor Author

Zacqary commented Jun 23, 2020

@simianhacker Looks like a bad merge conf resolution, I'll fix

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

simianhacker
simianhacker approved these changes Jun 24, 2020
View reviewed changes
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

@Zacqary Zacqary merged commit 71ea1a0 into elastic:master Jun 25, 2020
@Zacqary Zacqary deleted the 60659-alert-prefill branch June 25, 2020 17:01
@Zacqary Zacqary mentioned this pull request Jun 25, 2020
Merged
Zacqary added a commit to Zacqary/kibana that referenced this pull request Jun 25, 2020
@Zacqary @elasticmachine
[Metrics UI] Prefill alerts from the global dropdown (elastic#68967)
87642a5 
Co-authored-by: Elastic Machine <[email protected]>
@Zacqary Zacqary mentioned this pull request Jun 25, 2020
Merged
Zacqary added a commit that referenced this pull request Jun 25, 2020
@Zacqary @elasticmachine
[7.x] [Metrics UI] Prefill alerts from the global dropdown (#68967) (#…
35fde53 
…69981)

* [Metrics UI] Prefill alerts from the global dropdown (#68967)

Co-authored-by: Elastic Machine <[email protected]>

* Fix uncaught typecheck merge conflict

Co-authored-by: Elastic Machine <[email protected]>
@sorantis
Copy link

Conditions disappear from the flyout if I remove the queried metrics.
image

@sorantis sorantis mentioned this pull request Jul 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikecote mikecote mikecote approved these changes

@simianhacker simianhacker simianhacker approved these changes

Assignees
No one assigned
Labels
Feature:Alerting 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.9.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Metrics Alerts] Add ability to pre-populate alert from the Metrics Explorer toolbar
6 participants
@Zacqary @elasticmachine @simianhacker @kibanamachine @sorantis @mikecote