-
Notifications
You must be signed in to change notification settings - Fork 290
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
Address changes to PagerDuty integration #5980
Comments
This issue is stale because it has been open 90 days with no activity. |
Initial checks indicate that this is used in hundreds of location (pagerduty_service_integration) just within modernisation-platform repo |
Moved to blocked until Steve returns |
Amended the pagerduty code in modernisation-platform to pick set up sprinkler for testing purposes. Unfortunately this has also created many other items in pagerduty so we may need to consider removing those that are not needed later. The plan is to test changes against sprinkler to see if the proposed changes will work as required. This will, hopefully, stop other systems being amended/hit with the same changes. |
It looks like the manual run, to add new items to pagerduty worked, or there were no errors send to pagerduty. I have also amended the PR so that it contains the correct location for Sprinkler. This was also run manually so it will point to #modernisation-platform-low-priority-alarms. Testing will not commence on the changes required to correct the code to point at event orchestration. |
A different approach was taken Added orchestration using PR #7912 which, again, was successful. Now I am looking to see if the settings in aws.tf (e.g. corporate-staff-rostering-preproduction = pagerduty_service_integration.integrations["corporate-staff-rostering-preproduction"].integration_key) can be amended to also use pagerduty_event_orchestration. At the moment I am struggling to get sprinkler set up "as-is" in here. |
Checked in chatgpt and gemini and neither suggested the integration was being removed. |
A change has been made to the code to set the PagerDuty code to point to orchestration from the original for cdpt_chaps_cloudwatch. This is being moved under #7976 to test the change works and does not cause issues. The aws.tf code mentioned above will be amended as I go along (later) |
The latest version was pushed but Dave noticed some items were missing compared to main. Had a look and copied those back in. Hopefully the PR (#8044 ) is correct now. |
Various issues were popping up with the name I had used in 8048. This was reverted and the new code put in. It was simply the "name" needed to be something not used elsewhere and, as the one I was using was used in many places, we got an error. It was amended to |
Re-ran the code for just cdpt again, removing the changes I'd made for delius non prod. |
Talked to Steve to workout a handover. Change has been made to one team, @markgov was part of this. Sprinkler and cdpt have been done which can be found here - Delius is next, the code needs to be uncommented. https://github.com/ministryofjustice/modernisation-platform/pull/8158/files Will catch up with Mark about this more tomorrow. Alistair Curtis is testing CDPT IFS, need to catch up with him to see if its what is expected before deploying to other teams. Last contact from Alistair
|
Changes had broken a teams non prod alarms, I fixed this but we need to investigate |
Integrations modernisation-platform-orchestration See instructions in how to integrate with any system. The integration key must be part of the API v2 payload. API v2 payload = Changes to the payload :( |
We need to edit this module - https://github.com/ministryofjustice/modernisation-platform-terraform-pagerduty-integration/tree/main Instead of associating SNS topics with PagerDuty services, we need to associate them with orchestration integrations I believe. |
Ive discussed with Dave Elliot and David Sibley. https://support.pagerduty.com/main/docs/amazon-cloudwatch-integration-guide#integrate-with-rulesets Although this worried us PagerDuty/terraform-provider-pagerduty#775 Above, you can see the deprecation is about integration with rulesets, rather than service integration in what we do. I cant see any deprecation warnings here - https://github.com/ministryofjustice/modernisation-platform/actions/workflows/terraform-pagerduty.yml So have been advised to close the ticket, if our code was going to be deprecated, we would see it in that workflows output. |
Have reviewed. Moving to done. |
User Story
As a Modernisation Platform Engineer
I need to look at how we use PagerDuty integration keys with SNS / other AWS code
So that we're not caught out by PagerDuty's deprecation of Rulesets and Event Rules at the end of 2024
Value / Purpose
From PagerDuty's documentation, it's recommended that users switch from Rulesets / Event Rules to Event Orchestration.
We can see that the Terraform PagerDuty provider is currently warning us about the deprecated use of the
integration_key
argument which I believe is linked to this PagerDuty provider change..We should take steps, if needed, to update our PagerDuty integration to make use of Event Orchestration before this argument is removed from the PagerDuty provider / any integration with PagerDuty itself is removed
Useful Contacts
No response
Additional Information
No response
Proposal / Unknowns
No response
Definition of Done
The text was updated successfully, but these errors were encountered: