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] Get custom metrics working in inventory alerts with limited UI #75073

Merged
merged 7 commits into from
Aug 20, 2020

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Aug 14, 2020

Summary

Part of #65761

Screen Shot 2020-08-14 at 12 35 15 PM

This PR:

  • Fully implements backend capabilities for custom metrics on inventory alerts
  • Allows you to create inventory alerts with custom metrics from the flyout on the Inventory page

In order to create an alert with a custom metric, you need to first add the custom metric using the toolbar on the Inventory page in order to make it available in the alert flyout. There is no UI to create custom metrics from the alert flyout itself, and it will only use the Inventory page's WaffleOptionsState as a source of truth for its list of custom metrics. Therefore, this PR does not enable you to create custom metric alerts on the Alert Management page. You can, however, edit alerts that were previously created with custom metrics.

I think this is a good incremental push, but to fully close this issue we need to figure out what the UI should look like in order to define custom metrics in the alert flyout.

Tagged with release_note:skip because I think we should generate a release note from the final UI implementation.

Checklist

@Zacqary Zacqary added 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 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 14, 2020
@Zacqary Zacqary requested a review from a team as a code owner August 14, 2020 18:15
@elasticmachine
Copy link
Contributor

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

@simianhacker simianhacker self-requested a review August 17, 2020 17:20
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 works as expected. Can you add some tests? I would like to see some stuff around the changes to the following files:

  • public/alerting/inventory/components/expression.tsx
  • server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts

@Zacqary
Copy link
Contributor Author

Zacqary commented Aug 18, 2020

@simianhacker I started trying to add tests for the inventory executor when I pushed this PR but ended up running into a lot of issues with mocking the Snapshot API. Would it be okay if we deferred adding tests to this executor until after we've switched over to the Metrics API? That's probably easier to mock

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
infra 3.5MB +12.9KB 3.5MB

page load bundle size

id value diff baseline
infra 275.6KB +26.0B 275.5KB

History

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

@simianhacker simianhacker self-requested a review August 20, 2020 17:15
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.

Yes... we can wait till that's switched over. Thanks for adding the tests.

@Zacqary Zacqary merged commit bcca933 into elastic:master Aug 20, 2020
@Zacqary Zacqary deleted the 65761-inv-alert-custom-metrics branch August 20, 2020 18:28
Zacqary added a commit to Zacqary/kibana that referenced this pull request Aug 20, 2020
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Aug 21, 2020
Zacqary added a commit that referenced this pull request Aug 24, 2020
…h limited UI (#75073) (#75589)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@Zacqary Zacqary added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Oct 2, 2020
@Zacqary
Copy link
Contributor Author

Zacqary commented Oct 2, 2020

Switching release note to enhancement since the full custom metrics UI isn't making it into 7.10.

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.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants