-
Notifications
You must be signed in to change notification settings - Fork 164
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
PagerDuty Service Integration - Incident Creation API #89
Comments
hey @josephmcasey , good catch! Do you want to contribute this fix or do you prefer somebody else take it? I think suggestion with move to another API is good one |
@pasha-codefresh , I can definitely add this functionality if you don't mind guiding the contribution:
|
Argocd notification is part of argocd today, so you need to read this and just replace argo notification in go mod with your local folder In my opinion use PagerDuty v2 Events API is good option for solve this problem |
I ran into the same issue, and didn't see any movement on a PR so I went ahead and took a shot at refactoring the integration to use the v2 Events API instead: master...EricTendian:notifications-engine:master Not quite yet ready for a PR (need to do some more testing) but wanted to update folks here. Right now this is a breaking change (since it will not be compatible with the old PagerDuty integration configuration) so if that's a problem, we can discuss how backwards compatibility should work. |
Not sure what i am missing here but I am using the latest helm chart 5.36.3, but still getting
Using below configuration:
Project Yaml
|
after some debugging found that Argocd 's Go.mod is still pointing to the older version of notification-engine. I've raised an issue 14127 to use the latest revision. |
Problem Statement:
It appears the pagerduty.go service implementation recently used a rate-limited synchronous API intended for human initiated incidents.
notifications-engine/pkg/services/pagerduty.go
Line 98 in 7b9b5d3
Root Cause Analysis
The Notification Engine utilizes CreateIncidentWithContext from the PagerDuty Go SDK v1.5.0. Upon inspection of the implementation of
CreateIncidentWithContext
, it can be determined that the API Path/incident
corresponds to PagerDuty's Incident Creation API. This has concerning implications when utilizing the API in correspondence with Machine Events due to the following exerpts from the provided API documentation:Git Blame:
The changeset introducing this code was authored by @RaviHari and merged by @pasha-codefresh . Additional discussion is necessary to identify whether there is consensus from the original author and approver on the problem statement and recommended solution.
Recommendation
Add
Remove
Alternative & Additional Solutioning
CreateIncidentWithContext
as intended by the API.The text was updated successfully, but these errors were encountered: