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 Alerts] Fix error when a metric reports no data #59810

Merged
merged 3 commits into from
Mar 11, 2020

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Mar 10, 2020

Summary

Fixes #59809

Adds a NO_DATA alert state possibility, and sets this state if the alert can't find data over the specified lookup interval.

@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.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 10, 2020
@Zacqary Zacqary requested a review from a team March 10, 2020 18:22
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@phillipb phillipb left a comment

Choose a reason for hiding this comment

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

TIny flow-control nit.

const { buckets } = aggregations.aggregatedIntervals;
const { value } = buckets[buckets.length - 1].aggregatedValue;
return value;
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why the try/catch here. Why not just explicit if (!buckets.length)? Not a huge deal, but I'm afraid that intent is lost with try/catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, figured we wanted to guard for any error in retrieving the data but I'll see if I can clarify

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@Zacqary Zacqary merged commit db2a92c into elastic:master Mar 11, 2020
@Zacqary Zacqary deleted the 59809-metrics-no-data branch March 11, 2020 13:44
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 11, 2020
* master:
  [Metrics Alerts] Fix error when a metric reports no data (elastic#59810)
  Vislib legend toggle broken (elastic#59736)
  [RFC] Pulse (elastic#57108)
Zacqary added a commit to Zacqary/kibana that referenced this pull request Mar 11, 2020
* [Metrics Alerts] Fix error when a metric reports no data

* Clarify no data case handler, add separate error state

* Throw error state when callCluster fails
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 12, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

Zacqary added a commit that referenced this pull request Mar 12, 2020
…9927)

* [Metrics Alerts] Fix error when a metric reports no data

* Clarify no data case handler, add separate error state

* Throw error state when callCluster fails
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 12, 2020
simianhacker pushed a commit to simianhacker/kibana that referenced this pull request Mar 12, 2020
* [Metrics Alerts] Fix error when a metric reports no data

* Clarify no data case handler, add separate error state

* Throw error state when callCluster fails
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Mar 12, 2020
* [Metrics Alerts] Fix error when a metric reports no data

* Clarify no data case handler, add separate error state

* Throw error state when callCluster fails
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:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metrics Alerts] Fix error when a metric reports no data
4 participants