[Uptime] Ml anomaly alert edit

[shahzad31]

Summary

Fixes: #75080

Enabled requireAppContext true for uptime anomaly alert, because it doesn't make sense to enable/disable it outside uptime app.

ML job needs to be enabled for this alert to work. User enable/edit/delete job via popover alongside ml job.

[elasticmachine]

Pinging @elastic/uptime (Team:uptime)

[shahzad31]

@elasticmachine merge upstream

[shahzad31]

@elasticmachine merge upstream

[shahzad31]

@justinkambic please review this when you get chance

[shahzad31]

@elasticmachine merge upstream

[andrewvc]

@drewpost @paulb-elastic WDYT about this change? This makes the alert ONLY editable from the app, not from the global alerts page. I'm not sure I like this solution, but I'd be curious to hear another thought.

[shahzad31]

@andrewvc do you have an alternative solution in mind?

[andrewvc]

In the issue #75080 I mentioned making it editable from the alerts manager. I'd still like to hear feedback from @drewpost an @paulb-elastic on this one.

[shahzad31]

In the issue #75080 I mentioned making it editable from the alerts manager. I'd still like to hear feedback from @drewpost an @paulb-elastic on this one.

Yeah biggest problem with that is that it needs context if job is enabled or not, also allowing editing from alert management will also allow to create alert from alerting app. which doesn't make sense at all.

I can enable editing the threshold part from alert app, but like i said, it also will allow user to create an alert.

[shahzad31]

@elasticmachine merge upstream

[paulb-elastic]

I would think that having the alert be created in Uptime, but then not be available (even listed?) in the central Alerts and Actions section, would be confusing to a user (they may think it is a bug, where has it gone?).

Having the ability to define which monitor it relates to also seems necessary (so it doesn't treat all monitors the same, and trigger on any of them, e.g. a dev or staging environment).

Being able to do this by selecting the monitor in the WHEN clause as mentioned by @andrewvc seems useful (this will allow one or more monitors to be defined). Perhaps a future version could have a UI to help the user manage this more easily).

[andrewvc]

@paulb-elastic given our discussion about priorities today, I've actually reversed my thinking on this. I'm inclined to accept this patch as is given that selecting the monitor ID in the alert dialog doesn't make much sense, since we'd also need to validate that it has ML properly processing. I think we can improve this flow, but the effort is high, but the benefit not commensurate.

Your thoughts?

[shahzad31]

@elasticmachine merge upstream

[shahzad31]

@andrewvc please give this another look when you get chance :)

[shahzad31]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 9bfb88b

History

• 💚 Build #76891 succeeded 4934f04
• 💚 Build #76229 succeeded 7212b6d
• 💚 Build #75171 succeeded 6c338b5
• 💚 Build #74060 succeeded 72351b5
• 💚 Build #73465 succeeded 1c9e87c

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

[andrewvc]

LGTM

[andrewvc]

@shahzad31 can you check with @paulb-elastic before merging ? I don't think he has any objections now but it'd be good to be sure

[shahzad31]

I talked with @paulb-elastic offline, he gave go ahead, so merging it.