elastic / uptime

This project includes resources and general issue tracking for the Elastic Uptime solution
12 stars 3 forks source link

When activating a "status alert" on a monitor, the settings of the notification are missing #294

Open cyrille-leclerc opened 3 years ago

cyrille-leclerc commented 3 years ago

Describe the bug

Uptim v7.10

When activating a "status alert" on a monitor, the settings of the notification are missing.

The alerts created with the toggle "enable status alert" are invalid because the configuration of the notification connector are missing.

To Reproduce

Alt text

See video https://www.youtube.com/watch?v=wEOs2r5EHfI

Expected behavior

Alerts created with the "enable status alert" should be properly configured and fully functional. This would require on "Uptime Settings / Alerts Default / Alert Connectors" to not only choose the desired alert connectors but also to configure the payload of each of them

uptime-settings-alert-conector
andrewvc commented 3 years ago

Thanks for the report @cyrille-leclerc , my first thought here is that we should, for these alerts, have a default message that gets used. Users would be able to edit the alerts later, but this is intended to be a single click feature, and the default message should be in place.

@katrin-freihofner @drewpost @paulb-elastic thoughts on this as a fix?

paulb-elastic commented 3 years ago

@andrewvc do you mean we’d create a default configuration for each type of connector (just populating the mandatory fields)? If so, does this add complexity in needing to define sensible defaults for all the different connector mandatory params?

As per @andrewvc's question, is this toggle meant to be a single click enable? If so, should this control not be disabled, if all the mandatory configurations have not been met (e.g. having no default connector, or the default connector is not configured properly)?

Maybe a tooltip over the disabled toggle button indicates what to do (e.g. “Enable a default connector to enable this feature”, with a click through to the configuration page would be even better).

Then would it be the job of adding the default connector (on the Settings page) that enforces the configuration, similar to when we add them in the flyout?

image

justinkambic commented 3 years ago

I believe this is fixed in 8.x, 7.11, and 7.12.

@shahzad31 configured a trial ServiceNow account and we were able to ship alerts using the connector we created in the Uptime UI. I logged detailed steps in my review of the patch. We also tested PagerDuty, server log, and Slack. I didn't test MS Teams but I believe Shahzad did.

cyrille-leclerc commented 3 years ago

@andrewvc , @paulb-elastic @justinkambic FYI I'm doing some research with @katrin-freihofner on a medium term observability wide solution that would "accidentally" solve this specific problem so let's be cautious before considering an expensive fix.

I believe this is fixed in 8.x, 7.11, and 7.12.

@justinkambic I'm not sure to catch how you could fix this. I tried on edge-oblt but I didn't see how the UX would have changed to solve this problem and when I tried to "enable status alert" with servicenow configured in the uptime settings, I got the following exception:

Error
    at fetch_Fetch.fetchResponse (https://edge-oblt.elastic.dev/40094/bundles/core/core.entry.js:6:33030)
    at async interceptResponse (https://edge-oblt.elastic.dev/40094/bundles/core/core.entry.js:6:28637)
    at async https://edge-oblt.elastic.dev/40094/bundles/core/core.entry.js:6:31117
justinkambic commented 3 years ago

Ok - I will find some time to investigate. I'll try to coordinate with @shahzad31 when he returns.

shahzad31 commented 3 years ago

@cyrille-leclerc that error isn't related to alerting, edge-olbt is broken because of kibana v2 migration.

can you please try it on 7.11 branch, perhaps https://dev-oblt.elastic.dev/app/uptime

the way we fixed it , we are using a default set of params to configure service now and pager duty etc use case.