webhooks/pagerduty: Refactor and make minor improvements.

This commit improves a couple of things:

* All of the message templates are now at the top, a convention
  we follow in a lot of our webhooks.
* Messages are not prefixed with any emojis. We don't do this in
  any of our other webhooks. Plus, the emojis were outdated.
* Remove some superfluous code.
* Use ```quote <quote goes here> ``` style formatting for
  quoted text instead of the `>` character.
This commit is contained in:
Eeshan Garg
2019-01-12 20:11:51 -03:30
committed by Tim Abbott
parent 74f0d32a21
commit a1994122ca
2 changed files with 87 additions and 66 deletions

View File

@@ -8,48 +8,48 @@ class PagerDutyHookTests(WebhookTestCase):
FIXTURE_DIR_NAME = 'pagerduty'
def test_trigger(self) -> None:
expected_message = ':imp: Incident [3](https://zulip-test.pagerduty.com/incidents/P140S4Y) triggered by [Test service](https://zulip-test.pagerduty.com/services/PIL5CUQ) and assigned to [armooo@](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n>foo'
self.send_and_test_stream_message('trigger', u"incident 3", expected_message)
expected_message = 'Incident [3](https://zulip-test.pagerduty.com/incidents/P140S4Y) triggered by [Test service](https://zulip-test.pagerduty.com/services/PIL5CUQ) (assigned to [armooo](https://zulip-test.pagerduty.com/users/POBCFRJ))\n\n``` quote\nfoo\n```'
self.send_and_test_stream_message('trigger', u"Incident 3", expected_message)
def test_trigger_v2(self) -> None:
expected_message = ':imp: Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) triggered by [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75) and assigned to [Laura Haley@](https://webdemo.pagerduty.com/users/P553OPV)\n\n>My new incident'
self.send_and_test_stream_message('trigger_v2', u'incident 33', expected_message)
expected_message = 'Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) triggered by [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75) (assigned to [Laura Haley](https://webdemo.pagerduty.com/users/P553OPV))\n\n``` quote\nMy new incident\n```'
self.send_and_test_stream_message('trigger_v2', u'Incident 33', expected_message)
def test_trigger_without_assignee_v2(self) -> None:
expected_message = ':imp: Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) triggered by [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75) and assigned to [nobody@]()\n\n>My new incident'
self.send_and_test_stream_message('trigger_without_assignee_v2', u'incident 33', expected_message)
expected_message = 'Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) triggered by [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75) (assigned to nobody)\n\n``` quote\nMy new incident\n```'
self.send_and_test_stream_message('trigger_without_assignee_v2', u'Incident 33', expected_message)
def test_unacknowledge(self) -> None:
expected_message = ':imp: Incident [3](https://zulip-test.pagerduty.com/incidents/P140S4Y) unacknowledged by [Test service](https://zulip-test.pagerduty.com/services/PIL5CUQ) and assigned to [armooo@](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n>foo'
self.send_and_test_stream_message('unacknowledge', u"incident 3", expected_message)
expected_message = 'Incident [3](https://zulip-test.pagerduty.com/incidents/P140S4Y) unacknowledged by [Test service](https://zulip-test.pagerduty.com/services/PIL5CUQ) (assigned to [armooo](https://zulip-test.pagerduty.com/users/POBCFRJ))\n\n``` quote\nfoo\n```'
self.send_and_test_stream_message('unacknowledge', u"Incident 3", expected_message)
def test_resolved(self) -> None:
expected_message = ':grinning: Incident [1](https://zulip-test.pagerduty.com/incidents/PO1XIJ5) resolved by [armooo@](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n>It is on fire'
self.send_and_test_stream_message('resolved', u"incident 1", expected_message)
expected_message = 'Incident [1](https://zulip-test.pagerduty.com/incidents/PO1XIJ5) resolved by [armooo](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n``` quote\nIt is on fire\n```'
self.send_and_test_stream_message('resolved', u"Incident 1", expected_message)
def test_resolved_v2(self) -> None:
expected_message = ':grinning: Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) resolved by [Laura Haley@](https://webdemo.pagerduty.com/users/P553OPV)\n\n>My new incident'
self.send_and_test_stream_message('resolve_v2', 'incident 33', expected_message)
expected_message = 'Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) resolved by [Laura Haley](https://webdemo.pagerduty.com/users/P553OPV)\n\n``` quote\nMy new incident\n```'
self.send_and_test_stream_message('resolve_v2', 'Incident 33', expected_message)
def test_auto_resolved(self) -> None:
expected_message = ':grinning: Incident [2](https://zulip-test.pagerduty.com/incidents/PX7K9J2) resolved\n\n>new'
self.send_and_test_stream_message('auto_resolved', u"incident 2", expected_message)
expected_message = 'Incident [2](https://zulip-test.pagerduty.com/incidents/PX7K9J2) resolved\n\n``` quote\nnew\n```'
self.send_and_test_stream_message('auto_resolved', u"Incident 2", expected_message)
def test_acknowledge(self) -> None:
expected_message = ':no_good: Incident [1](https://zulip-test.pagerduty.com/incidents/PO1XIJ5) acknowledged by [armooo@](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n>It is on fire'
self.send_and_test_stream_message('acknowledge', u"incident 1", expected_message)
expected_message = 'Incident [1](https://zulip-test.pagerduty.com/incidents/PO1XIJ5) acknowledged by [armooo](https://zulip-test.pagerduty.com/users/POBCFRJ)\n\n``` quote\nIt is on fire\n```'
self.send_and_test_stream_message('acknowledge', u"Incident 1", expected_message)
def test_acknowledge_v2(self) -> None:
expected_message = ':no_good: Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) acknowledged by [Laura Haley@](https://webdemo.pagerduty.com/users/P553OPV)\n\n>My new incident'
self.send_and_test_stream_message('acknowledge_v2', 'incident 33', expected_message)
expected_message = 'Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) acknowledged by [Laura Haley](https://webdemo.pagerduty.com/users/P553OPV)\n\n``` quote\nMy new incident\n```'
self.send_and_test_stream_message('acknowledge_v2', 'Incident 33', expected_message)
def test_incident_assigned_v2(self) -> None:
expected_message = ':no_good: Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) assigned to [Wiley Jacobson@](https://webdemo.pagerduty.com/users/PFBSJ2Z)\n\n>My new incident'
self.send_and_test_stream_message('assign_v2', 'incident 33', expected_message)
expected_message = 'Incident [33](https://webdemo.pagerduty.com/incidents/PRORDTY) assigned to [Wiley Jacobson](https://webdemo.pagerduty.com/users/PFBSJ2Z)\n\n``` quote\nMy new incident\n```'
self.send_and_test_stream_message('assign_v2', 'Incident 33', expected_message)
def test_no_subject(self) -> None:
expected_message = u':grinning: Incident [48219](https://dropbox.pagerduty.com/incidents/PJKGZF9) resolved\n\n>mp_error_block_down_critical\u2119\u01b4'
self.send_and_test_stream_message('mp_fail', u"incident 48219", expected_message)
expected_message = u'Incident [48219](https://dropbox.pagerduty.com/incidents/PJKGZF9) resolved\n\n``` quote\nmp_error_block_down_critical\u2119\u01b4\n```'
self.send_and_test_stream_message('mp_fail', u"Incident 48219", expected_message)
def test_bad_message(self) -> None:
expected_message = 'Unknown pagerduty message\n```\n{\n "type":"incident.triggered"\n}\n```'

View File

@@ -30,10 +30,46 @@ PAGER_DUTY_EVENT_NAMES_V2 = {
'incident.assign': 'assigned',
}
def build_pagerduty_formatdict(message: Dict[str, Any]) -> Dict[str, Any]:
# Normalize the message dict, after this all keys will exist. I would
# rather some strange looking messages than dropping pages.
ASSIGNEE_TEMPLATE = '[{username}]({url})'
INCIDENT_WITH_SERVICE_AND_ASSIGNEE = (
'Incident [{incident_num}]({incident_url}) {action} by [{service_name}]'
'({service_url}) (assigned to {assignee_info})\n\n``` quote\n{trigger_message}\n```'
)
INCIDENT_WITH_ASSIGNEE = """
Incident [{incident_num}]({incident_url}) {action} by {assignee_info}
``` quote
{trigger_message}
```
""".strip()
INCIDENT_ASSIGNED = """
Incident [{incident_num}]({incident_url}) {action} to {assignee_info}
``` quote
{trigger_message}
```
""".strip()
INCIDENT_RESOLVED_WITH_AGENT = """
Incident [{incident_num}]({incident_url}) resolved by {resolving_agent_info}
``` quote
{trigger_message}
```
""".strip()
INCIDENT_RESOLVED = """
Incident [{incident_num}]({incident_url}) resolved
``` quote
{trigger_message}
```
""".strip()
def build_pagerduty_formatdict(message: Dict[str, Any]) -> Dict[str, Any]:
format_dict = {} # type: Dict[str, Any]
format_dict['action'] = PAGER_DUTY_EVENT_NAMES[message['type']]
@@ -44,27 +80,21 @@ def build_pagerduty_formatdict(message: Dict[str, Any]) -> Dict[str, Any]:
format_dict['service_name'] = message['data']['incident']['service']['name']
format_dict['service_url'] = message['data']['incident']['service']['html_url']
# This key can be missing on null
if message['data']['incident'].get('assigned_to_user', None):
assigned_to_user = message['data']['incident']['assigned_to_user']
format_dict['assigned_to_email'] = assigned_to_user['email']
format_dict['assigned_to_username'] = assigned_to_user['email'].split('@')[0]
format_dict['assigned_to_url'] = assigned_to_user['html_url']
format_dict['assignee_info'] = ASSIGNEE_TEMPLATE.format(
username=assigned_to_user['email'].split('@')[0],
url=assigned_to_user['html_url'],
)
else:
format_dict['assigned_to_email'] = 'nobody'
format_dict['assigned_to_username'] = 'nobody'
format_dict['assigned_to_url'] = ''
format_dict['assignee_info'] = 'nobody'
# This key can be missing on null
if message['data']['incident'].get('resolved_by_user', None):
resolved_by_user = message['data']['incident']['resolved_by_user']
format_dict['resolved_by_email'] = resolved_by_user['email']
format_dict['resolved_by_username'] = resolved_by_user['email'].split('@')[0]
format_dict['resolved_by_url'] = resolved_by_user['html_url']
else:
format_dict['resolved_by_email'] = 'nobody'
format_dict['resolved_by_username'] = 'nobody'
format_dict['resolved_by_url'] = ''
format_dict['resolving_agent_info'] = ASSIGNEE_TEMPLATE.format(
username=resolved_by_user['email'].split('@')[0],
url=resolved_by_user['html_url'],
)
trigger_message = []
trigger_subject = message['data']['incident']['trigger_summary_data'].get('subject', '')
@@ -90,16 +120,17 @@ def build_pagerduty_formatdict_v2(message: Dict[str, Any]) -> Dict[str, Any]:
assignments = message['incident']['assignments']
if assignments:
assignee = assignments[0]['assignee']
format_dict['assigned_to_username'] = assignee['summary']
format_dict['assigned_to_url'] = assignee['html_url']
format_dict['assignee_info'] = ASSIGNEE_TEMPLATE.format(
username=assignee['summary'], url=assignee['html_url'])
else:
format_dict['assigned_to_username'] = 'nobody'
format_dict['assigned_to_url'] = ''
format_dict['assignee_info'] = 'nobody'
last_status_change_by = message['incident'].get('last_status_change_by')
if last_status_change_by is not None:
format_dict['resolved_by_username'] = last_status_change_by['summary']
format_dict['resolved_by_url'] = last_status_change_by['html_url']
format_dict['resolving_agent_info'] = ASSIGNEE_TEMPLATE.format(
username=last_status_change_by['summary'],
url=last_status_change_by['html_url'],
)
trigger_description = message['incident'].get('description')
if trigger_description is not None:
@@ -123,28 +154,18 @@ def send_formated_pagerduty(request: HttpRequest,
message_type: str,
format_dict: Dict[str, Any]) -> None:
if message_type in ('incident.trigger', 'incident.unacknowledge'):
template = (u':imp: Incident '
u'[{incident_num}]({incident_url}) {action} by '
u'[{service_name}]({service_url}) and assigned to '
u'[{assigned_to_username}@]({assigned_to_url})\n\n>{trigger_message}')
elif message_type == 'incident.resolve' and format_dict['resolved_by_url']:
template = (u':grinning: Incident '
u'[{incident_num}]({incident_url}) resolved by '
u'[{resolved_by_username}@]({resolved_by_url})\n\n>{trigger_message}')
elif message_type == 'incident.resolve' and not format_dict['resolved_by_url']:
template = (u':grinning: Incident '
u'[{incident_num}]({incident_url}) resolved\n\n>{trigger_message}')
template = INCIDENT_WITH_SERVICE_AND_ASSIGNEE
elif message_type == 'incident.resolve' and format_dict.get('resolving_agent_info') is not None:
template = INCIDENT_RESOLVED_WITH_AGENT
elif message_type == 'incident.resolve' and format_dict.get('resolving_agent_info') is None:
template = INCIDENT_RESOLVED
elif message_type == 'incident.assign':
template = (u':no_good: Incident [{incident_num}]({incident_url}) '
u'{action} to [{assigned_to_username}@]({assigned_to_url})\n\n>{trigger_message}')
template = INCIDENT_ASSIGNED
else:
template = (u':no_good: Incident [{incident_num}]({incident_url}) '
u'{action} by [{assigned_to_username}@]({assigned_to_url})\n\n>{trigger_message}')
template = INCIDENT_WITH_ASSIGNEE
subject = u'incident {incident_num}'.format(**format_dict)
subject = u'Incident {incident_num}'.format(**format_dict)
body = template.format(**format_dict)
check_send_webhook_message(request, user_profile, subject, body)
@@ -160,7 +181,7 @@ def api_pagerduty_webhook(
# If the message has no "type" key, then this payload came from a
# Pagerduty Webhook V2.
if message_type is None:
continue
break
if message_type not in PAGER_DUTY_EVENT_NAMES:
send_raw_pagerduty_json(request, user_profile, message)
@@ -178,7 +199,7 @@ def api_pagerduty_webhook(
# If the message has no "event" key, then this payload came from a
# Pagerduty Webhook V1.
if event is None:
continue
break
if event not in PAGER_DUTY_EVENT_NAMES_V2:
raise UnexpectedWebhookEventType('Pagerduty', event)