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] Metric Threshold rule type should clear previous group cache on KQL query change #115468

Closed
Zacqary opened this issue Oct 18, 2021 · 1 comment · Fixed by #116205
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@Zacqary
Copy link
Contributor

Zacqary commented Oct 18, 2021

Metric Threshold rules store the previous list of detected groups in state.groups, in order to detect when groups unexpectedly disappear. To make sure we don't continue to alert on them when we do expect them to disappear, i.e. when the user edits the rule definition, we check to see if the groupBy (alert per) value has changed. But sometimes, changing the rule's KQL filter can also result in some groups disappearing.

See logic in https://github.com/elastic/kibana/blob/master/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts#L96-L98

const previousGroupBy = state.groupBy;
const prevGroups =
      alertOnGroupDisappear && isEqual(previousGroupBy, params.groupBy)
        ? // Filter out the * key from the previous groups, only include it if it's one of
          // the current groups. In case of a groupBy alert that starts out with no data and no
          // groups, we don't want to persist the existence of the * alert instance
          state.groups?.filter((g) => g !== UNGROUPED_FACTORY_KEY) ?? []
        : [];

Note that we are only checking if the groupBy value has changed — if it has changed, prevGroups evaluates as an empty array instead of the stored state.groups. This resets the cache, as prevGroups gets merged with the currently detected groups to get saved to state.groups.

Acceptance criteria

  • Store the rule definition's filterQuery in rule state
  • In addition to checking if groupBy has changed before building prevGroups, also check to see if the filterQuery has changed
@Zacqary Zacqary added bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Oct 18, 2021
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants