Skip to content

test: document/illustrate notification behavior for muted alerts - #5470

Open
suprjinx wants to merge 5 commits into
prometheus:mainfrom
suprjinx:characterize-muted-alert-notifications
Open

test: document/illustrate notification behavior for muted alerts#5470
suprjinx wants to merge 5 commits into
prometheus:mainfrom
suprjinx:characterize-muted-alert-notifications

Conversation

@suprjinx

@suprjinx suprjinx commented Aug 25, 2026

Copy link
Copy Markdown

This is a tests-only PR that documents and illustrates the current behavior of muted alerts in the notify pipeline. It's expected that these tests will continue to pass when feature-flagged changes are introduced.

  • MuteStage already records muted alert hashes in the context, but the time interval stages do not.
  • MultiStage short-circuits once the mute stage has emptied the group, so the dedup stage never observes "all alerts are muted".
  • A muted alert is dropped from the notification log entry the next time the group is notified about, even though it is still firing.
  • An alert that was notified about and is then muted never produces the matching resolved notification (Send resolved notification for silenced alerts #226), unit tested and end to end.
  • A group with a muted alert is reported as entirely resolved while that alert is still firing, and the alert then opens a new sequence once it is unmuted (Improve Alertmanager Semantics for Partially Muted Alert Groups #5247).

Assisted-by: Claude Code

Pull Request Checklist

Please check all the applicable boxes.

Which user-facing changes does this PR introduce?

The mute stages remove muted alerts from the pipeline before the dedup
stage runs, so muted alerts never reach the notification log and a
group's notification history has no way to represent them.

Pin the resulting semantics before changing them, so that a later change
shows up as an explicit diff of these expectations. The tests assert the
behavior as it is, not as it should be, including the bug reported in

- MuteStage already records muted alert hashes in the context, but the
  time interval stages do not.
- MultiStage short-circuits once the mute stage has emptied the group, so
  the dedup stage never observes "all alerts are muted".
- A muted alert is dropped from the notification log entry the next time
  the group is notified about, even though it is still firing.
- An alert that was notified about and is then muted never produces the
  matching resolved notification (prometheus#226), unit tested and end to end.
- A group with a muted alert is reported as entirely resolved while that
  alert is still firing, and the alert then opens a new sequence once it
  is unmuted (prometheus#5247).

Test only, no mainline code changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Geoff Wilson <geoff@gr-oss.io>
@suprjinx
suprjinx requested a review from a team as a code owner August 25, 2026 14:19
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

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: b71e80da-bb70-4e1a-9bd3-8aaaf3f69b69

📥 Commits

Reviewing files that changed from the base of the PR and between 95c50e3 and f562e7e.

📒 Files selected for processing (1)
  • test/with_api_v2/acceptance/silence_test.go
💤 Files with no reviewable changes (1)
  • test/with_api_v2/acceptance/silence_test.go

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


📝 Walkthrough

Walkthrough

The change adds unit tests for muted-alert handling across notification stages and deduplication. It also adds an API acceptance test that verifies silenced alerts do not generate resolved notifications.

Changes

Muted alert notification behavior

Layer / File(s) Summary
Stage filtering and pipeline setup
notify/muted_alerts_test.go
Adds a pipeline harness and tests mute-stage recording, time-stage behavior, and pipeline short-circuiting when all alerts are muted.
Deduplication and notification-log behavior
notify/muted_alerts_test.go
Tests notification-log removal, missing resolved notifications, and notification-sequence restart after muted alerts are unmuted.
API acceptance coverage
test/with_api_v2/acceptance/silence_test.go
Verifies that silencing a firing alert prevents a resolved notification when send_resolved is enabled.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f562e

This tests-only change documents existing muted-alert behavior and introduces no production behavior changes; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: tests that document notification behavior for muted alerts.
Description check ✅ Passed The description explains the tests-only scope, documents the covered muted-alert behaviors, references related issues, and includes the repository checklist. The empty release-notes block is a minor o…
Docstring Coverage ✅ Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files.
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 explains the tests-only scope, documents the covered muted-alert behaviors, references related issues, and includes the repository checklist. The empty release-notes block is a minor omission because the description states that there are no user-facing changes.

✨ 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.

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

@Spaceman1701 Spaceman1701 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the contribution, these look very good!

Comment on lines +236 to +246
func boolPtr(b bool) *bool {
return &b
}

func stringPtr(s string) *string {
return &s
}

func dateTimePtr(t strfmt.DateTime) *strfmt.DateTime {
return &t
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like these are unused. (this is the source of the linter failures in CI).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

will remove!

Signed-off-by: Geoff Wilson <geoff@gr-oss.io>
…suprjinx/alertmanager into characterize-muted-alert-notifications
@TheMeier

Copy link
Copy Markdown
Contributor

/workflow-approve

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.

4 participants