Skip to content
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

feat(alerting): Added Incident.io alerting provider #972

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

ImTheCurse
Copy link
Contributor

Summary

  • Added Incident.io as an alerting provider.
  • Added testing for Incident.io alerting provider.
  • Added documentation.

Fixes #967

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

@ImTheCurse
Copy link
Contributor Author

@TwiN could you review the PR?

@ImTheCurse ImTheCurse changed the title feat(alerting): Added Incident.io provider feat(alerting): Added Incident.io alerting provider Jan 20, 2025
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
alerting/config.go Outdated Show resolved Hide resolved
@ImTheCurse ImTheCurse requested a review from TwiN January 21, 2025 07:17
README.md Outdated Show resolved Hide resolved
alerting/alert/type.go Outdated Show resolved Hide resolved
ImTheCurse and others added 2 commits January 22, 2025 18:14
Co-authored-by: Maksim Zhylinski <[email protected]>
Co-authored-by: Maksim Zhylinski <[email protected]>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ImTheCurse ImTheCurse marked this pull request as draft January 22, 2025 17:46
Copy link

@limoges limoges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to implement the feature.

I left a few comments. They are all with the intent of improving the default UX of a Gatus user. Let me know what you think.

I'll look at this tomorrow and do some manual testing to see if there are issues (e.g. configure locally, run status, let it resolve alerts).

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
alerting/alert/type.go Outdated Show resolved Hide resolved
alerting/config.go Show resolved Hide resolved
@TwiN
Copy link
Owner

TwiN commented Jan 23, 2025

@limoges Thank you for helping with the review, it's strongly appreciated!

@ImTheCurse ImTheCurse marked this pull request as ready for review January 27, 2025 09:29
@ImTheCurse ImTheCurse requested a review from TwiN January 27, 2025 09:30
@ImTheCurse
Copy link
Contributor Author

@TwiN mind rechecking the PR?

Copy link

@limoges limoges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the source-url and metadata, I did a small manual test with a config and was able to validate the metadata/attributes and source-url.

This is what an alert looks like:
image

LGTM, I think this would deploy without issue for our usecase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Incident.io as Alerting provider
4 participants