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

[Uptime] simple monitor status alert fix for page duty and other connectors #87460

Merged
merged 13 commits into from
Jan 14, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jan 6, 2021

Summary

Fixes: #84459

This PR makes sure, that auto generated alert by one click uses correct params for each connectors.

Params validity is made sure by type checking.

Testing:

To test this PR, you will need to create a trial page duty and service now accounts and make sure alert works with those connectors.

After adding connectors, add those in uptime settings for default connectors. And then try creating alert from monitor list for few monitors and see if it get's triggered for down time.

Connect me if you want to test those connectors, i can provide my credentials for those connectors.

Note: i wanted to write functional tests but it turns out mocking them isn't that simple, considiring limited time frame, i have created a follow up issue

elastic/uptime#279

@shahzad31 shahzad31 changed the title simple alert fix [Uptime] simple monitor status alert fix for page duty and other connectors Jan 11, 2021
@shahzad31 shahzad31 marked this pull request as ready for review January 11, 2021 09:23
@shahzad31 shahzad31 requested a review from a team as a code owner January 11, 2021 09:23
@shahzad31 shahzad31 self-assigned this Jan 11, 2021
@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Jan 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM - everything seems to be working ok. Had a few questions about the future of the code we're adding here.

Smoke testing process

  1. I created connectors for PagerDuty, ServiceNow, Slack, and an extra server log connector to use for control purposes.
    image
  2. I defined a local monitor that was up, and allowed data to accumulate.
    image
  3. I killed the process that was running and waited for the alert to fire
    Slack: image
    Server log: image
    PagerDuty: image
    ServiceNow: I must have misconfigured something. I got an error like: server log [13:48:19.750] [warning][actions][actions][plugins] action execution failure: .servicenow:01f234e0-5505-11eb-ab10-f71ddcabade6: service-now: an error occurred while running the action executor: [Action][ServiceNow]: Unable to create incident. Error: Request failed with status code 401
  4. I attempted to create a new user in ServiceNow.
  5. I re-started my service to resolve the current alert instance, and then stopped the service once more.
  6. After re-configuring the ServiceNow connector and testing it in the management portal, it was a "successful" run:
    image

TeamsActionTypeId,
WebhookActionTypeId,
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
} from '../../../../actions/server/builtin_action_types';
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we maybe ask Alerting to export these in the future to avoid busting the linter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah another idea was to make an an uptime endpoint for this.

incident: {
short_description: MonitorStatusTranslations.defaultActionMessage,
description: MonitorStatusTranslations.defaultActionMessage,
impact: '2',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we plan to make these fields configurable for this, Jira, PagerDuty, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah make sense, let's follow up on this.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 568 569 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
uptime 1.1MB 1.1MB +3.8KB

History

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

@shahzad31 shahzad31 merged commit c2fc583 into elastic:master Jan 14, 2021
@shahzad31 shahzad31 deleted the alert-fix branch January 14, 2021 11:01
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Jan 14, 2021
…ectors (elastic#87460)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Jan 14, 2021
…ectors (elastic#87460)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
shahzad31 added a commit that referenced this pull request Jan 15, 2021
…r connectors (#87460) (#88318)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
shahzad31 added a commit that referenced this pull request Jan 15, 2021
…er connectors (#87460) (#88319)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kibana alerting for Uptime monitoring not working properly with Pagerduty
4 participants