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

Upgrade alertmanager version to v0.28.0 #6590

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

Conversation

SungJin1212
Copy link
Contributor

@SungJin1212 SungJin1212 commented Feb 14, 2025

This PR upgrades the Prometheus Alertmanager version to v0.28.0.

Notable changes:

The new integrations are added (msteamsv2, jira, and rocketchat)

It switches to use the slog; we can use a GoKitLogToSlog.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@yeya24
Copy link
Contributor

yeya24 commented Feb 16, 2025

@rajagopalanand @rapphil Can you help review this change?

@yeya24
Copy link
Contributor

yeya24 commented Feb 18, 2025

@SungJin1212 Can you please add changelog?

@SungJin1212 SungJin1212 force-pushed the Upgrade-alertmanager-v0.28.0 branch from 1228a89 to 48d8cfb Compare February 19, 2025 01:25
@SungJin1212 SungJin1212 force-pushed the Upgrade-alertmanager-v0.28.0 branch from 48d8cfb to 6ce7ac3 Compare February 19, 2025 04:41
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Looks good. Better to wait for review from @rajagopalanand or @rapphil before merging

@@ -99,7 +103,7 @@ type Alertmanager struct {
persister *statePersister
nflog *nflog.Log
silences *silence.Silences
marker types.Marker
marker *types.MemMarker
Copy link
Contributor

@rapphil rapphil Feb 22, 2025

Choose a reason for hiding this comment

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

Shouldn't we be relying on the interface here instead of the implementation? The
previous Marker interface was renamed to AlertMarker prometheus/alertmanager@d31a249

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I split it into two interfaces.

alertMarker     types.AlertMarker
groupMarker     types.GroupMarker

Copy link
Contributor

@rapphil rapphil left a comment

Choose a reason for hiding this comment

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

Are there any major differences in the log format emitted by AlertManager with this new release? from reading the code It seems not, but did you actually notice anything?

@SungJin1212 SungJin1212 force-pushed the Upgrade-alertmanager-v0.28.0 branch from 7a1a905 to 650d43c Compare February 22, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants