diff --git a/zerver/webhooks/newrelic/fixtures/incident_acknowledged_new.json b/zerver/webhooks/newrelic/fixtures/incident_acknowledged_new.json new file mode 100644 index 0000000000..b1ee9bed12 --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_acknowledged_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "state": "acknowledged", + "owner": "Alice", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_acknowledged.json b/zerver/webhooks/newrelic/fixtures/incident_acknowledged_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_acknowledged.json rename to zerver/webhooks/newrelic/fixtures/incident_acknowledged_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_active_new.json b/zerver/webhooks/newrelic/fixtures/incident_active_new.json new file mode 100644 index 0000000000..766aeda8ad --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_active_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "state": "activated", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_closed_new.json b/zerver/webhooks/newrelic/fixtures/incident_closed_new.json new file mode 100644 index 0000000000..3412e9ff6c --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_closed_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "state": "closed", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_closed.json b/zerver/webhooks/newrelic/fixtures/incident_closed_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_closed.json rename to zerver/webhooks/newrelic/fixtures/incident_closed_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_created_new.json b/zerver/webhooks/newrelic/fixtures/incident_created_new.json new file mode 100644 index 0000000000..b9aa02c36b --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_created_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "state": "created", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_default_fields_new.json b/zerver/webhooks/newrelic/fixtures/incident_default_fields_new.json new file mode 100644 index 0000000000..82db1eb44c --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_default_fields_new.json @@ -0,0 +1,5 @@ +{ + "id": 1234, + "createdAt": 1605133931151, + "state": "activated" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_default_fields.json b/zerver/webhooks/newrelic/fixtures/incident_default_fields_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_default_fields.json rename to zerver/webhooks/newrelic/fixtures/incident_default_fields_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_malformatted_time_new.json b/zerver/webhooks/newrelic/fixtures/incident_malformatted_time_new.json new file mode 100644 index 0000000000..60f2d44ed4 --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_malformatted_time_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": "1969-12-31 23:59:55", + "state": "acknowledged", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_malformatted_time.json b/zerver/webhooks/newrelic/fixtures/incident_malformatted_time_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_malformatted_time.json rename to zerver/webhooks/newrelic/fixtures/incident_malformatted_time_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_missing_current_state.json b/zerver/webhooks/newrelic/fixtures/incident_missing_current_state_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_missing_current_state.json rename to zerver/webhooks/newrelic/fixtures/incident_missing_current_state_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_missing_state_new.json b/zerver/webhooks/newrelic/fixtures/incident_missing_state_new.json new file mode 100644 index 0000000000..c86cad125d --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_missing_state_new.json @@ -0,0 +1,10 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "owner": "Alice", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_new.json b/zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_new.json new file mode 100644 index 0000000000..8ca464ebc1 --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_new.json @@ -0,0 +1,10 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "state": "acknowledged", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_missing_timestamp.json b/zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_missing_timestamp.json rename to zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_opened.json b/zerver/webhooks/newrelic/fixtures/incident_opened_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_opened.json rename to zerver/webhooks/newrelic/fixtures/incident_opened_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_new.json b/zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_new.json new file mode 100644 index 0000000000..81c7ad3256 --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 1605133931151, + "state": "hello world", + "owner": "Alice", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_state_not_recognized.json b/zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_state_not_recognized.json rename to zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_old.json diff --git a/zerver/webhooks/newrelic/fixtures/incident_time_too_large_new.json b/zerver/webhooks/newrelic/fixtures/incident_time_too_large_new.json new file mode 100644 index 0000000000..cb56618011 --- /dev/null +++ b/zerver/webhooks/newrelic/fixtures/incident_time_too_large_new.json @@ -0,0 +1,11 @@ +{ + "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge", + "id": 1234, + "details": "Violation description test.", + "alertPolicyNames": ["Test policy name"], + "condition_name": "Server Down", + "createdAt": 160513393115100000, + "state": "open", + "owner": "", + "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234" +} diff --git a/zerver/webhooks/newrelic/fixtures/incident_time_too_large.json b/zerver/webhooks/newrelic/fixtures/incident_time_too_large_old.json similarity index 100% rename from zerver/webhooks/newrelic/fixtures/incident_time_too_large.json rename to zerver/webhooks/newrelic/fixtures/incident_time_too_large_old.json diff --git a/zerver/webhooks/newrelic/tests.py b/zerver/webhooks/newrelic/tests.py index 567cf4091e..369527dbf0 100644 --- a/zerver/webhooks/newrelic/tests.py +++ b/zerver/webhooks/newrelic/tests.py @@ -6,7 +6,11 @@ class NewRelicHookTests(WebhookTestCase): URL_TEMPLATE = "/api/v1/external/newrelic?stream={stream}&api_key={api_key}" WEBHOOK_DIR_NAME = "newrelic" - def test_open(self) -> None: + # The following 9 unit tests are for the old format + # corresponding json fixtures were renamed to have the "_old" trailing + # These tests and fixtures are to be deleted when old notifications EOLed + + def test_open_old(self) -> None: expected_topic = "Test policy name (1234)" expected_message = """ [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **opened** for condition: **Server Down** at @@ -16,42 +20,42 @@ Violation description test. """.strip() self.check_webhook( - "incident_opened", + "incident_opened_old", expected_topic, expected_message, content_type="application/json", ) - def test_closed(self) -> None: + def test_closed_old(self) -> None: expected_topic = "Test policy name (1234)" expected_message = """ [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **closed** for condition: **Server Down** """.strip() self.check_webhook( - "incident_closed", + "incident_closed_old", expected_topic, expected_message, content_type="application/json", ) - def test_acknowledged(self) -> None: + def test_acknowledged_old(self) -> None: expected_topic = "Test policy name (1234)" expected_message = """ [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **acknowledged** by **Alice** for condition: **Server Down** """.strip() self.check_webhook( - "incident_acknowledged", + "incident_acknowledged_old", expected_topic, expected_message, content_type="application/json", ) - def test_not_recognized(self) -> None: + def test_not_recognized_old(self) -> None: with self.assertRaises(AssertionError) as e: self.check_webhook( - "incident_state_not_recognized", + "incident_state_not_recognized_old", "", "", content_type="application/json", @@ -61,7 +65,7 @@ Violation description test. e.exception.args[0], ) - def test_missing_fields(self) -> None: + def test_missing_fields_old(self) -> None: expected_topic = "Unknown Policy (Unknown ID)" expected_message = """ [Incident](https://alerts.newrelic.com) **opened** for condition: **Unknown condition** at @@ -71,16 +75,16 @@ No details. """.strip() self.check_webhook( - "incident_default_fields", + "incident_default_fields_old", expected_topic, expected_message, content_type="application/json", ) - def test_missing_current_state(self) -> None: + def test_missing_current_state_old(self) -> None: with self.assertRaises(AssertionError) as e: self.check_webhook( - "incident_missing_current_state", + "incident_missing_current_state_old", "", "", content_type="application/json", @@ -90,10 +94,10 @@ No details. e.exception.args[0], ) - def test_missing_timestamp(self) -> None: + def test_missing_timestamp_old(self) -> None: with self.assertRaises(AssertionError) as e: self.check_webhook( - "incident_missing_timestamp", + "incident_missing_timestamp_old", "", "", content_type="application/json", @@ -102,20 +106,153 @@ No details. "The newrelic webhook requires timestamp in milliseconds", e.exception.args[0] ) - def test_malformatted_time(self) -> None: + def test_malformatted_time_old(self) -> None: with self.assertRaises(AssertionError) as e: self.check_webhook( - "incident_malformatted_time", + "incident_malformatted_time_old", "", "", content_type="application/json", ) self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0]) - def test_time_too_large(self) -> None: + def test_time_too_large_old(self) -> None: with self.assertRaises(AssertionError) as e: self.check_webhook( - "incident_time_too_large", + "incident_time_too_large_old", + "", + "", + content_type="application/json", + ) + self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0]) + + # The following 10 unit tests are for the new format + # One more test than the old format as we have 4 states instead of 3 in the old + # corresponding json fixtures have "_new" trailing in the name + + def test_activated_new(self) -> None: + expected_topic = "Test policy name (1234)" + expected_message = """ +[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **active** for condition: **Server Down** at +``` quote +Violation description test. +``` +""".strip() + + self.check_webhook( + "incident_active_new", + expected_topic, + expected_message, + content_type="application/json", + ) + + def test_created_new(self) -> None: + expected_topic = "Test policy name (1234)" + expected_message = """ +[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **created** for condition: **Server Down** +""".strip() + + self.check_webhook( + "incident_created_new", + expected_topic, + expected_message, + content_type="application/json", + ) + + def test_closed_new(self) -> None: + expected_topic = "Test policy name (1234)" + expected_message = """ +[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **closed** for condition: **Server Down** +""".strip() + + self.check_webhook( + "incident_closed_new", + expected_topic, + expected_message, + content_type="application/json", + ) + + def test_acknowledged_new(self) -> None: + expected_topic = "Test policy name (1234)" + expected_message = """ +[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **acknowledged** by **Alice** for condition: **Server Down** +""".strip() + + self.check_webhook( + "incident_acknowledged_new", + expected_topic, + expected_message, + content_type="application/json", + ) + + def test_not_recognized_new(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "incident_state_not_recognized_new", + "", + "", + content_type="application/json", + ) + self.assertIn( + "The newrelic webhook requires state be in [created|activated|acknowledged|closed]", + e.exception.args[0], + ) + + def test_missing_fields_new(self) -> None: + expected_topic = "Unknown Policy (1234)" + expected_message = """ +[Incident](https://alerts.newrelic.com) **active** for condition: **Unknown condition** at +``` quote +No details. +``` +""".strip() + + self.check_webhook( + "incident_default_fields_new", + expected_topic, + expected_message, + content_type="application/json", + ) + + def test_missing_state_new(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "incident_missing_state_new", + "", + "", + content_type="application/json", + ) + self.assertIn( + "The newrelic webhook requires state be in [created|activated|acknowledged|closed]", + e.exception.args[0], + ) + + def test_missing_timestamp_new(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "incident_missing_timestamp_new", + "", + "", + content_type="application/json", + ) + self.assertIn( + "The newrelic webhook requires timestamp in milliseconds", e.exception.args[0] + ) + + def test_malformatted_time_new(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "incident_malformatted_time_new", + "", + "", + content_type="application/json", + ) + self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0]) + + def test_time_too_large_new(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "incident_time_too_large_new", "", "", content_type="application/json", diff --git a/zerver/webhooks/newrelic/view.py b/zerver/webhooks/newrelic/view.py index 560a53da92..b0730029c7 100644 --- a/zerver/webhooks/newrelic/view.py +++ b/zerver/webhooks/newrelic/view.py @@ -11,6 +11,12 @@ from zerver.lib.response import json_success from zerver.lib.webhooks.common import check_send_webhook_message, unix_milliseconds_to_timestamp from zerver.models import UserProfile +# Newrelic planned to upgrade Alert Notification Channels to Workflows and Destinations +# https://discuss.newrelic.com/t/plan-to-upgrade-alert-notification-channels-to-workflows-and-destinations/188205 +# This view will handle both old and new format but will keep it easy to delete the old code +# once it is EOLed by the end of June, 2023 + +# Once old is EOLed, delete the OPEN_TEMPLATE OPEN_TEMPLATE = """ [Incident]({incident_url}) **opened** for condition: **{condition_name}** at ``` quote @@ -18,13 +24,23 @@ OPEN_TEMPLATE = """ ``` """.strip() +ACTIVE_TEMPLATE = """ +[Incident]({incident_url}) **active** for condition: **{condition_name}** at +``` quote +{details} +``` +""".strip() + DEFAULT_TEMPLATE = ( """[Incident]({incident_url}) **{status}** {owner}for condition: **{condition_name}**""".strip() ) TOPIC_TEMPLATE = """{policy_name} ({incident_id})""".strip() -ALL_EVENT_TYPES = ["closed", "acknowledged", "open"] +# Once old is EOLed, delete old and keep new +OLD_EVENT_TYPES = ["closed", "acknowledged", "open"] +NEW_EVENT_TYPES = ["created", "activated", "acknowledged", "closed"] +ALL_EVENT_TYPES = list(set(OLD_EVENT_TYPES).union(set(NEW_EVENT_TYPES))) @webhook_view("NewRelic", all_event_types=ALL_EVENT_TYPES) @@ -35,45 +51,102 @@ def api_newrelic_webhook( payload: Dict[str, Any] = REQ(argument_type="body"), ) -> HttpResponse: - info = { - "condition_name": payload.get("condition_name", "Unknown condition"), - "details": payload.get("details", "No details."), - "incident_url": payload.get("incident_url", "https://alerts.newrelic.com"), - "incident_acknowledge_url": payload.get( - "incident_acknowledge_url", "https://alerts.newrelic.com" - ), - "status": payload.get("current_state", "None"), - "iso_timestamp": "", - "owner": payload.get("owner", ""), - } + # Handle old format + # Once old is EOLed, delete if block and keep else block + if not payload.get("id"): + info = { + "condition_name": payload.get("condition_name", "Unknown condition"), + "details": payload.get("details", "No details."), + "incident_url": payload.get("incident_url", "https://alerts.newrelic.com"), + "incident_acknowledge_url": payload.get( + "incident_acknowledge_url", "https://alerts.newrelic.com" + ), + "status": payload.get("current_state", "None"), + "iso_timestamp": "", + "owner": payload.get("owner", ""), + } - unix_time = payload.get("timestamp", None) - if unix_time is None: - raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds")) + unix_time = payload.get("timestamp", None) + if unix_time is None: + raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds")) - info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic") + info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic") - # Add formatting to the owner field if owner is present - if info["owner"] != "": - info["owner"] = "by **{}** ".format(info["owner"]) + # Add formatting to the owner field if owner is present + if info["owner"] != "": + info["owner"] = "by **{}** ".format(info["owner"]) - # These are the three promised current_state values - if "open" in info["status"]: - content = OPEN_TEMPLATE.format(**info) - elif "acknowledged" in info["status"]: - content = DEFAULT_TEMPLATE.format(**info) - elif "closed" in info["status"]: - content = DEFAULT_TEMPLATE.format(**info) + # These are the three promised current_state values + if info["status"].lower() == "open": + content = OPEN_TEMPLATE.format(**info) + elif info["status"].lower() == "acknowledged": + content = DEFAULT_TEMPLATE.format(**info) + elif info["status"].lower() == "closed": + content = DEFAULT_TEMPLATE.format(**info) + else: + raise JsonableError( + _("The newrelic webhook requires current_state be in [open|acknowledged|closed]") + ) + + topic_info = { + "policy_name": payload.get("policy_name", "Unknown Policy"), + "incident_id": payload.get("incident_id", "Unknown ID"), + } + topic = TOPIC_TEMPLATE.format(**topic_info) + + check_send_webhook_message(request, user_profile, topic, content, info["status"]) + return json_success(request) + + # Handle new format else: - raise JsonableError( - _("The newrelic webhook requires current_state be in [open|acknowledged|closed]") - ) + info = { + "condition_name": payload.get("condition_name", "Unknown condition"), + "details": payload.get("details", "No details."), + "incident_url": payload.get("issueUrl", "https://alerts.newrelic.com"), + "incident_acknowledge_url": payload.get( + "incident_acknowledge_url", "https://alerts.newrelic.com" + ), + "status": payload.get("state", "None"), + "iso_timestamp": "", + "owner": payload.get("owner", ""), + } - topic_info = { - "policy_name": payload.get("policy_name", "Unknown Policy"), - "incident_id": payload.get("incident_id", "Unknown ID"), - } - topic = TOPIC_TEMPLATE.format(**topic_info) + unix_time = payload.get("createdAt", None) + if unix_time is None: + raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds")) - check_send_webhook_message(request, user_profile, topic, content, info["status"]) - return json_success(request) + info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic") + + # Add formatting to the owner field if owner is present + if info["owner"] != "": + info["owner"] = "by **{}** ".format(info["owner"]) + + # These are the three promised state values + if info["status"].lower() == "activated": + content = ACTIVE_TEMPLATE.format(**info) + elif info["status"].lower() == "acknowledged": + content = DEFAULT_TEMPLATE.format(**info) + elif info["status"].lower() == "closed": + content = DEFAULT_TEMPLATE.format(**info) + elif info["status"].lower() == "created": + content = DEFAULT_TEMPLATE.format(**info) + else: + raise JsonableError( + _( + "The newrelic webhook requires state be in [created|activated|acknowledged|closed]" + ) + ) + + policy_names_list = payload.get("alertPolicyNames", []) + if policy_names_list: + policy_names_str = ",".join(policy_names_list) + else: + policy_names_str = "Unknown Policy" + topic_info = { + "policy_name": policy_names_str, + "incident_id": payload.get("id", "Unknown ID"), + } + topic = TOPIC_TEMPLATE.format(**topic_info) + + check_send_webhook_message(request, user_profile, topic, content, info["status"]) + return json_success(request)