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

[Alerting] [Discuss] Modifying alert params within the executor as a migration tool #66608

Closed
Zacqary opened this issue May 14, 2020 · 4 comments
Labels
discuss Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@Zacqary
Copy link
Contributor

Zacqary commented May 14, 2020

Alerts are in beta right now, so we might be pushing a large amount of breaking changes to them. Required alert params might change between minor versions, and so alerts created in a previous version would immediately break.

Can we fix this by including a migration assistant service in alert executors? For example:

  • Metric alert types have an interval (timeSize and timeUnit) inside each alert criteria (an array of alert conditions), but these are supposed to be the same for each of them. It's redundant and error-prone to repeat the same value inside each array member.
params: {
  criteria: [{
     timeSize: 's' | 'm' | 'h' | 'd',
     timeUnit: number,
     ...rest
  }, 
  ...additionalConditions
  ]
}
  • In the next minor version, we might move timeSize and timeUnit outside of the criteria array and into the top-level alert params.
params: {
  criteria: [...arrayOfConditions],
  timeSize,
  timeUnit
}
  • When the alert executor runs, it checks to see if timeSize and timeUnit are inside of the criteria. If they are, it will move them into the top level of params and update the SavedObject of the alert to reflect this change. This code can then be deprecated in the next minor version, assuming that all alerts probably migrated before then.

Is something like this already possible with the available savedObjects service in alerts, or would a new one be needed? Is this even a good idea? I'd like some opinions.

@Zacqary Zacqary added discuss Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels May 14, 2020
@elasticmachine
Copy link
Contributor

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

@mikecote
Copy link
Contributor

Relates to #50216.

@pmuellr
Copy link
Member

pmuellr commented May 15, 2020

I've seen a few notes about future directions of saved object migrations, we should be sure we're aligned with that. This sort of migrate-when-needed is an interesting approach.

Also potentially relevant: #50213 - I think migrate-when-needed would imply we can't have mappings for alert params. But presumably if we did have mappings for alert params, then we'd be doing standard SO migrations at that point (in terms of when and how they happened).

@mikecote
Copy link
Contributor

Closing as duplicate of #50216 and will mention this issue / comments in the other one.

@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

No branches or pull requests

5 participants