support setting integration name for alert

[alexandermarston]

We'd like to start pushing some of our PrometheusRule evaluations through Coralogix, but at the moment we can't seem to natively setup the notification integration without using a Coralogix specific Kubernetes resource.

It would be nice if we could add an additional annotation to our already existing PrometheusRules to specify the name of the Notification Integration we would like to use.

Looking for some feedback on this approach.

[CLAassistant]

All committers have signed the CLA.

[OrNovo]

Hi @alexandermarston.
Thank you for your contribution. Please take into account that this may create a conflict with future support of alert-manger (ie alert-manager controller will override the logic you suggested).
Please resolve the conflict and I'll approve your PR.

[alexandermarston]

Hi @alexandermarston. Thank you for your contribution. Please take into account that this may create a conflict with future support of alert-manger (ie alert-manager controller will override the logic you suggested). Please resolve the conflict and I'll approve your PR.

Thank you for the feedback.

Would ignoring the cxNotificationName annotation if the PrometheusRule is labelled with coralogix.com/managed-by-alertmanger-config be enough?

Just as some added context, the Alert Manager integration looks neat but as it currently stands it wouldn't be compatible with our Alert Manager implementation. We use the Alertmanager resource, but define our routes and receivers in a configuration secret not in the resource itself.

[OrNovo]

Hi @alexandermarston. Thank you for your contribution. Please take into account that this may create a conflict with future support of alert-manger (ie alert-manager controller will override the logic you suggested). Please resolve the conflict and I'll approve your PR.

Thank you for the feedback.

Would ignoring the cxNotificationName annotation if the PrometheusRule is labelled with coralogix.com/managed-by-alertmanger-config be enough?

Just as some added context, the Alert Manager integration looks neat but as it currently stands it wouldn't be compatible with our Alert Manager implementation. We use the Alertmanager resource, but define our routes and receivers in a configuration secret not in the resource itself.

Yes. But we will deal with it separately.

Got it. Guess that will be handled separately when we get all the necessary information.

[alexandermarston]

Thanks for the approval @OrNovo . I am still happy to add the additional check now, if you'd like?

I can see some tests are failing, but I don't believe they're related to my change.

[assafad1]

@OrNovo regarding failing tests, I see on the e2e-test logs:

2024/10/13 15:21:42 Warning: teamLevelAPIKey was not provided. Some functionality will not be available.
2024/10/13 15:21:42 Warning: userLevelAPIKey was not provided. Some functionality will not be available.

I think there is no access to the repo secrets, because the tests run from @alexandermarston's fork. I think we can merge this PR.

[assafad1]

@alexandermarston can you please sign your commit?

[alexandermarston]

@assafad1 is there any chance for a review?

[assafad1]

@alexandermarston thanks for your contribution!