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

[RAC][Observability] preserve lifecycle alert changes for active alerts #110124

Merged
merged 5 commits into from
Aug 31, 2021
Merged

[RAC][Observability] preserve lifecycle alert changes for active alerts #110124

merged 5 commits into from
Aug 31, 2021

Conversation

mgiota
Copy link
Contributor

@mgiota mgiota commented Aug 25, 2021 •
edited
Loading

Fixes #108670

@mgiota mgiota added v7.15.0 v7.16.0 v8.0.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Aug 25, 2021
@elasticmachine
Copy link
Contributor

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

@mgiota mgiota added auto-backport Deprecated - use backport:version if exact versions are needed Theme: rac label obsolete release_note:skip Skip the PR/issue when compiling release notes labels Aug 25, 2021
@mgiota mgiota marked this pull request as draft August 25, 2021 17:57
@afgomez afgomez requested a review from a team August 26, 2021 13:23
@mgiota mgiota marked this pull request as ready for review August 26, 2021 15:01
@mgiota mgiota requested review from afgomez and removed request for a team August 26, 2021 15:46
@mgiota mgiota self-assigned this Aug 30, 2021
afgomez
afgomez approved these changes Aug 30, 2021
View reviewed changes
Copy link
Contributor

@afgomez afgomez left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

@mgiota
Copy link
Contributor Author

mgiota commented Aug 30, 2021

@elasticmachine merge upstream

@mgiota
Copy link
Contributor Author

mgiota commented Aug 30, 2021 •
edited
Loading

@afgomez I was wondering about this line of code that calculates new alert ids.

kibana/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts

Line 181 in f7436e7

const newAlertIds = currentAlertIds.filter((alertId) => !trackedAlertIds.includes(alertId));

Shouldn't it be like this?

const newAlertIds = trackedAlertIds.filter((alertId) => !currentAlertIds.includes(alertId));

newAlertIds is not used anywhere in the logic other than logging purposes.

UPDATE
It should be fine as it is. I got a bit confused with the wording trackedAlertIds (previous alerts) vs currentAlertIds.

let trackedAlertIds = [ 'opbeans-java', 'opbeans-node' ]
let currentAlertIds = [ 'opbeans-java' ]
let newAlertIds = currentAlertIds.filter((alertId) => !trackedAlertIds.includes(alertId)); // gives [] as it should

@mgiota
Copy link
Contributor Author

mgiota commented Aug 31, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @mgiota

@mgiota mgiota merged commit b7ad426 into elastic:master Aug 31, 2021
@kibanamachine kibanamachine mentioned this pull request Aug 31, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 31, 2021
@mgiota @kibanamachine
[RAC][Observability] preserve lifecycle alert changes for active aler…
0d7c5e3 
…ts (elastic#110124)

* preserve lifecycle changes for active alerts

* fix failing tests

* fix failing lifecycle executor tests

Co-authored-by: Kibana Machine <[email protected]>
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 31, 2021
@mgiota @kibanamachine
[RAC][Observability] preserve lifecycle alert changes for active aler…
6fda7b4 
…ts (elastic#110124)

* preserve lifecycle changes for active alerts

* fix failing tests

* fix failing lifecycle executor tests

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Aug 31, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.15
✅ 7.x

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 31, 2021
@kibanamachine @mgiota
[RAC][Observability] preserve lifecycle alert changes for active aler…
eecc41d 
…ts (#110124) (#110576)

* preserve lifecycle changes for active alerts

* fix failing tests

* fix failing lifecycle executor tests

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

Co-authored-by: mgiota <[email protected]>
kibanamachine added a commit that referenced this pull request Aug 31, 2021
@kibanamachine @mgiota
[RAC][Observability] preserve lifecycle alert changes for active aler…
325bba9 
…ts (#110124) (#110577)

* preserve lifecycle changes for active alerts

* fix failing tests

* fix failing lifecycle executor tests

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

Co-authored-by: mgiota <[email protected]>
@mgiota mgiota deleted the 108670_preserve_changes_for_active_alerts branch January 4, 2022 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afgomez afgomez afgomez approved these changes

Assignees

@mgiota mgiota

Labels
auto-backport Deprecated - use backport:version if exact versions are needed 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 Theme: rac label obsolete v7.15.0 v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RAC] Preserve lifecycle alert document changes for active alerts
4 participants
@mgiota @elasticmachine @kibanamachine @afgomez