nflog: add muted_alerts to the notification log entry by suprjinx · Pull Request #5474 · prometheus/alertmanager

suprjinx · 2026-08-26T15:24:27Z

Entry currently records the alerts that were firing and resolved at the time of the last notification, but not the ones that were muted. The mute stages drop muted alerts before the dedup stage runs, so they are absent from both lists and indistinguishable from alerts that stopped firing.

Add a muted_alerts field alongside them, an IsMutedSubset helper mirroring IsFiringSubset and IsResolvedSubset, and a mutedAlerts parameter on Log(). Entries that predate this field decode with MutedAlerts unset.

Nothing populates the field yet: SetNotifiesStage passes nil, and the dedup stage does not read it. This is the schema and write path only, so that the behavior change can be reviewed on its own.

Generated code regenerated with scripts/genproto.sh.

Assisted-by: Claude Code

Pull Request Checklist

Please check all the applicable boxes.

Which user-facing changes does this PR introduce?

Entry records the alerts that were firing and resolved at the time of the last notification, but not the ones that were muted. The mute stages drop muted alerts before the dedup stage runs, so they are absent from both lists and indistinguishable from alerts that stopped firing. Add a muted_alerts field alongside them, an IsMutedSubset helper mirroring IsFiringSubset and IsResolvedSubset, and a mutedAlerts parameter on Log(). The field number 9 is new; existing field numbers are untouched, so entries gossiped by peers that predate this field decode with MutedAlerts unset, and entries this version writes are readable by those peers. Nothing populates the field yet: SetNotifiesStage passes nil, and the dedup stage does not read it. This is the schema and write path only, so that the behavior change can be reviewed on its own. Generated code regenerated with scripts/genproto.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqRsyADYBfpUxp1JpzQsW8

coderabbitai · 2026-08-26T15:27:47Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0f7a80e4-aea0-4d01-a954-cbd81d0b0fe6

📥 Commits

Reviewing files that changed from the base of the PR and between 8012469 and 742448e.

⛔ Files ignored due to path filters (1)

nflog/nflogpb/nflog.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (8)

nflog/nflog.go
nflog/nflog_test.go
nflog/nflogpb/nflog.proto
nflog/nflogpb/set.go
nflog/nflogpb/set_test.go
notify/notify.go
notify/notify_test.go
notify/set_notifies_stage.go

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Walkthrough

The notification log now accepts muted alert IDs, stores them in protobuf entries, exposes subset checks, and validates serialization, querying, timestamp-based merging, and notification integrations.

Changes

Muted alert persistence

Layer / File(s)	Summary
Muted alert contract and subset checks `nflog/nflogpb/nflog.proto`, `nflog/nflogpb/set.go`, `nflog/nflogpb/set_test.go`	`Entry` now contains repeated `muted_alerts` hashes. `IsMutedSubset` checks requested IDs and supports entries without the field.
Logging API and entry persistence `nflog/nflog.go`, `notify/notify.go`, `notify/set_notifies_stage.go`	`Log` and `NotificationLog.Log` accept muted alert IDs. New entries store them in `MutedAlerts`. Notification-stage calls pass the new argument.
Serialization, merge, and integration validation `nflog/nflog_test.go`, `notify/notify_test.go`	Tests cover query results, wire-format compatibility, timestamp-based replacement, callback forwarding, and nil muted-alert arguments.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 74244

This adds muted-alert information to notification log entries without changing existing alert processing behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding muted_alerts to nflog notification entries.
Description check	✅ Passed	The description clearly explains the schema, API, compatibility, tests, and intentionally deferred behavior. The feature and compatibility checklist items remain unchecked, and the release-notes block…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description clearly explains the schema, API, compatibility, tests, and intentionally deferred behavior. The feature and compatibility checklist items remain unchecked, and the release-notes block is empty, but the description is otherwise mostly complete and directly related to the changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Spaceman1701

Thanks for the contribution, generally looks good!

Spaceman1701 · 2026-08-27T09:25:52Z


+// IsMutedSubset returns whether the given subset is a subset of the alerts
+// that were muted at the time of the last notification.
+func (m *Entry) IsMutedSubset(subset map[uint64]struct{}) bool {


I'm not sure we'll end up needing this function, I think we should leave it out until it's clear that it'll be used.

…ted-alerts-to-proto

Signed-off-by: Geoff Wilson <geoff@gr-oss.io>

suprjinx requested a review from a team as a code owner August 26, 2026 15:24

Spaceman1701 reviewed Aug 27, 2026

View reviewed changes

suprjinx added 2 commits August 27, 2026 15:08

Merge branch 'main' of github.com:prometheus/alertmanager into add-mu…

920bf1a

…ted-alerts-to-proto

Remove IsMutedSubset func and tests

23fbcb5

Signed-off-by: Geoff Wilson <geoff@gr-oss.io>

Navigation Menu

Uh oh!

nflog: add muted_alerts to the notification log entry - #5474

nflog: add muted_alerts to the notification log entry#5474
suprjinx wants to merge 3 commits into
prometheus:mainfrom
suprjinx:add-muted-alerts-to-proto

suprjinx commented Aug 26, 2026

Uh oh!

coderabbitai Bot commented Aug 26, 2026

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Spaceman1701 left a comment

Uh oh!

Spaceman1701 Aug 27, 2026

Uh oh!

suprjinx Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

suprjinx commented Aug 26, 2026

Pull Request Checklist

Which user-facing changes does this PR introduce?

Uh oh!

coderabbitai Bot commented Aug 26, 2026

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Spaceman1701 left a comment

Choose a reason for hiding this comment

Uh oh!

Spaceman1701 Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

suprjinx Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants