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

[Actions][ServiceNow] Allow to close serviceNow incident when alert resolves #171760

Merged
merged 28 commits into from Dec 1, 2023

Conversation

js-jankisalvi
Copy link
Contributor

@js-jankisalvi js-jankisalvi commented Nov 22, 2023

Summary

Fixes: #170522

This PR allows to close service now incident when alert is recovered

SN connector form shows only correlation_id field as it is mandatory field to close an incident.

Screenshot 2023-11-27 at 11 52 36

How to test:

  • Create a rule and select serviceNow ITSM action with Run when option as Recovered
  • Verify that it closes an incident in SN when Alert is recovered

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release notes

Auto close ServiceNow incidents when alerts are resolved

@js-jankisalvi js-jankisalvi added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Nov 22, 2023
@js-jankisalvi js-jankisalvi self-assigned this Nov 22, 2023
@@ -8,7 +8,7 @@
import { RecoveredActionGroup } from './builtin_action_groups';

const DisabledActionGroupsByActionType: Record<string, string[]> = {
[RecoveredActionGroup.id]: ['.jira', '.servicenow', '.resilient'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To allow Recovered option in Run when dropdown

Copy link
Member

Choose a reason for hiding this comment

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

@elastic/response-ops-execution FYI, it seems that some connectors are missing from this list like the .servicenow-secops connector.

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

I tested and everything is working as expected 🚀. I managed to create an incident and close it when the alert recovers. I left some comments.

@@ -8,7 +8,7 @@
import { RecoveredActionGroup } from './builtin_action_groups';

const DisabledActionGroupsByActionType: Record<string, string[]> = {
[RecoveredActionGroup.id]: ['.jira', '.servicenow', '.resilient'],
Copy link
Member

Choose a reason for hiding this comment

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

@elastic/response-ops-execution FYI, it seems that some connectors are missing from this list like the .servicenow-secops connector.

@@ -39,6 +39,18 @@ describe('servicenow action params validation', () => {
});
});

test(`${SERVICENOW_ITSM_CONNECTOR_TYPE_ID}: action params validation succeeds when no short_description provided and subAction is closeIncident`, async () => {
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about testing that there are no errors at all? This will make the test fail if someone adds a new required field and forgets the recovered feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added test in d627b86, is this what you meant?

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my feedback! Some comments 🙂.

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

LGMT! I tested by creating a rule with a SN action that creates and incident and a SN action that close the incident when the alert recovers. Everything is working as expected :rocket. I found two small bugs:

  1. if you try to close the incident through the actions API by using the externalId you get an error. It seems that we need to also provide the close_notes: 'Closed by Caller'. In the ServiceNow UI it is a required field when you try to close an incident.
  2. If the incident is already closed the warning message is not logged.

@cnasikas
Copy link
Member

cnasikas commented Dec 1, 2023

Ok, I found out about the second bug, we are doing incidentToBeClosed.state === 7 { // log warn } but the state is a string not an integer. It should be incidentToBeClosed.state === "7" 🙂.

@js-jankisalvi
Copy link
Contributor Author

  1. if you try to close the incident through the actions API by using the externalId you get an error. It seems that we need to also provide the close_notes: 'Closed by Caller'. In the ServiceNow UI it is a required field when you try to close an incident.
  2. If the incident is already closed the warning message is not logged.

Fixed both bugs and added tests in e3b43c7

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Thanks for your patience with the review 🚀 . I tested and everything is working as expected.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #44 / security APIs - Session Concurrent Limit Session Concurrent Limit cleanup should properly clean up sessions that exceeded concurrent session limit even for multiple providers

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
triggersActionsUi 557 558 +1

Async chunks

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

id before after diff
stackConnectors 542.2KB 543.6KB +1.5KB
triggersActionsUi 1.4MB 1.4MB +44.0B
total +1.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
alerting 23.3KB 23.3KB -14.0B
stackConnectors 39.9KB 40.5KB +595.0B
total +581.0B
Unknown metric groups

API count

id before after diff
triggersActionsUi 583 584 +1

ESLint disabled line counts

id before after diff
stackConnectors 103 102 -1

Total ESLint disabled count

id before after diff
stackConnectors 108 107 -1

History

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

cc @js-jankisalvi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Actions release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Actions][ServiceNow] Close issue when an alerts resolves
5 participants