Adjust New Relic Integration for the "Workflows and Destinations" update

[PieterCK]

This is issue was opened to revive efforts to update New Relic integration in PR #27779. However, further investigations show that the scope of what should be updated in the New Relic integration is wider than what PR #27779 has attempted to fix. This Issue aims to report what is outdated in the current Zulip's New Relic integration and what are the new changes from New Relic

New Relic Update Discussion: Plan to upgrade Alert Notification Channels to Workflows and Destinations

In short, New Relic has deployed a new feature(UI) called "Workflows and Destinations" to replace the old "Alert notification channels" UI alongside upgrading their webhook payload.

Outdated Parts in Zulip New Relic Integration

Thankfully, the previous contributor to the New Relic integration has anticipated the "Workflow and Destination" update and thus has started implementing code blocks that handle some of the new updates. New Relic's migration guide has also outlined on how to adjust for the new changes. That said, here are a few outdated parts that I was able to find:

New Relic Icon

The current integration still uses the old New Relic icon

New Relic Icon in the current integration:

New Relic Icon from the official media asset:

The integration doc

the old documentation still navigates and shows the user to the old "Alert notification channels" UI. It's also missing a step of configuring the webhook payload in the new "Workflow" UI.

• current doc
  

• updated New Relic UI
  

Test fixtures & Payload

Some of the obvious changes to the payload was the current_state field that now uses different terminologies. Full changes to the payload are in the migration guide referenced above

• Example of the updated webhook payload:

• The most recently updated fixtures are missing a set of new fields. e.g alertConditionNames mentioned in PR webhooks: Newrelic extract alert condition names from payload. #27779, priority, title, totalIncidents, impactedEntities, etc. The most recent New Relic payload looks like this:

{
	"id": "d1b1f3fd-995a-4066-88ab-8ce4f6960654",
	"issueUrl": "https://radar-api.service.newrelic.com/accounts/1/issues/0ea2df1c-adab-45d2-aae0-042b609d2322?notifier=SLACK",
	"title": "Memory Used % > 90 for at least 2 minutes on 'Some-Entity'",
	"priority": "CRITICAL",
	"impactedEntities": ["logs.itg.cloud","MonitorTTFB query"],
	"totalIncidents": 42,
	"state": "ACTIVATED",
	"trigger": "INCIDENT_ADDED",
	"isCorrelated": false,
	"createdAt": 1617881246260,
	"updatedAt": 1617881246260,
	"sources": ["newrelic"],
	"alertPolicyNames": ["Policy1","Policy2"],
	"alertConditionNames": ["condition1","condition2"],
	"workflowName": "DBA Team workflow"
}

Whereas, the most recently updated test fixtures looks like this:

# newrelic/fixtures/incident_active_new.json
{
    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
    "id": "f0d98b28-bf9d-49e7-b9d0-ac7cbb52e73a",
    "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"
}

Test suites

To be expected from changes to the fixtures, the current tests are also outdated.

Legacy codes

As highlighted by the previous contributor, some parts of the code are now outdated and needs to be deleted. Some part of the code could also use the UnsupportedWebhookEventTypeError exception.

• Comments from previous contributor highlighting legacy codes after New Relic updated their webhook
  
  
  

Notification messages

As per mentioned in PR #27779, the current integration doesn't handle and display new alert conditions. With a plethora of new information available in the updated payload, a new notification format might also be needed.

• Outdated notification

PR #27779 has added conditions to adjust the code to support the new update. Notification rendered from the same payload:

Considering the payload supports multiple alert condition, I propose restructuring the notification message and moving it to the "details" section of the notification.

[PieterCK]

@zulipbot claim

[zulipbot]

Hello @PieterCK!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

[zulipbot]

Hello @zulip/server-integrations members, this issue was labeled with the "area: integrations" label, so you may want to check it out!

[PieterCK]

Update: New discussion in this CZO for the new notification message design

[PieterCK]

Update: #29730 is ready for review!