notifications-engine icon indicating copy to clipboard operation
notifications-engine copied to clipboard

feat(services): add NewRelic service

Open saviogl opened this issue 3 years ago • 4 comments
trafficstars

Hi folks,

My company is in the process of rolling out New Relic internally and I saw an opportunity for adding a new notification service for it to integrate with its Deployment Marker functionality.

Not sure if the contributing process involving opening up an issue first, etc, so just let me know if changes are need to be made.

Also, pretty new to the Argoproj codebase, so let me know if I missed any aspect of adding a new notification service, or if there are any other considerations/suggestions. Haven't tested e2e, not sure the best way of doing so within this codebase or integrated with the ArgoCD one.

Issue: #93

saviogl avatar May 21 '22 06:05 saviogl

Codecov Report

Base: 49.38% // Head: 49.84% // Increases project coverage by +0.46% :tada:

Coverage data is based on head (69272e9) compared to base (e096c63). Patch coverage: 61.03% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   49.38%   49.84%   +0.46%     
==========================================
  Files          31       32       +1     
  Lines        1873     1950      +77     
==========================================
+ Hits          925      972      +47     
- Misses        746      764      +18     
- Partials      202      214      +12     
Impacted Files Coverage Δ
pkg/services/services.go 23.77% <28.57%> (+0.24%) :arrow_up:
pkg/services/newrelic.go 64.28% <64.28%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar May 21 '22 06:05 codecov[bot]

One additional comment in regards to deployment markers and integration to ArgoCD. Historically, these markers meant to target directly the revision of the underlying application code rather than the configuration (GitOps) repository, which then gives developers a direct access/relationship between the event and the code revision that rolled out at such time. Changes to environment configuration are also very much relevant to dictate events that can influence production reliability so it also makes sense to post these events as Deployment Markers / Events.

This implementation enforces a GitOps first approach, and will set the Revision based on the GitOps repository commit hash, and I was thinking of leveraging other fields like Description/Changelog and the commit hash / deployment images tag to indicate the underlying application revision so this can be easily inferred from the New Relic's UI.

With that said, I thought that it would be interesting to be able to tell whether or not the underlying application revision has changed with the latest sync and possibly change the Revision field to match either the application revision or the GitOps repository change. It might not work great for applications with multiple deployment objects but it could work well for cleany isolated ones.

I'm curious on the maintainers thought on this topic and the use of these type of events supported by observability tools

saviogl avatar May 24 '22 16:05 saviogl

@alexmt Just making sure I'm following the right approach here

saviogl avatar May 26 '22 20:05 saviogl

@ryota-sakamoto Maybe you can shed some light on the contributing process here?

saviogl avatar Jun 01 '22 23:06 saviogl

Hey @saviogl , i am going to review it

pasha-codefresh avatar Nov 26 '22 21:11 pasha-codefresh

@pasha-codefresh Thanks for getting back to me, I wasn't sure if this repository was still active, or how I could follow up on the contribution process. I had left a few messages back in the day on Slack, but to no avail.

Glad that we are coming around to it, it's been a while so I might need a refresher myself. I ended up using Webhooks Notifications Service to setup the foundation for the company that I worked for but it would be great to have this abstraction as a supported service.

Let me know if you have any questions...

On another note, If I wanted to get more involved with the argo project, where would be the best place to start?

saviogl avatar Nov 27 '22 00:11 saviogl

@saviogl ArgoCD is good candidate, but notification is also part of it, i would say that notification also great place for beginning.

About PR, the code itself looks good to me, I just want run a manual test on it today, and after we can merge

pasha-codefresh avatar Nov 27 '22 09:11 pasha-codefresh

@saviogl i got it working, great job, lets just fix small issues in documentation Снимок экрана 2022-11-27 в 19 34 01

pasha-codefresh avatar Nov 27 '22 17:11 pasha-codefresh

Hey @saviogl , do you need some help with solve comments that i provided?

pasha-codefresh avatar Nov 29 '22 19:11 pasha-codefresh

Hi @pasha-codefresh, I'll be looking at this today, and I'll let you know if I come across any questions? Are you in CNCF Slack workspace?

saviogl avatar Nov 29 '22 19:11 saviogl

Yep @saviogl , messaged you in slack

pasha-codefresh avatar Nov 29 '22 22:11 pasha-codefresh

Great job, thank you!

pasha-codefresh avatar Nov 30 '22 09:11 pasha-codefresh