Add silence logging option

[anarcat]

Past silences are really important to postmortem our alert management operations.
We need to get "when" and "what kind of" create/edit/delete silences.
So this PR added optional function that logging silence change.

Reroll of #2536 to address review item.

Summary by CodeRabbit

• New Features

  • Added a command-line flag (off by default) to enable logging of silence lifecycle events; when enabled, creations, updates, and expirations are logged with IDs, creator, timestamps (RFC3339), and formatted matcher details.

• Tests

  • Added unit tests that verify the silence logging output, timestamp formatting, and matcher encoding and ordering in the emitted logs.

[SuperQ]

Overall LGTM.

[SuperQ]

Needs a rebase.

[anarcat]

i rebased, but the history is not super clean, should i squash?

[TheMeier]

Squashing the commit sounds like a good idea to me. Just a question would it make sense to add a minimal test for the new function?

[anarcat]

probably! not sure where to start with that though

[TheMeier]

How about something like this TheMeier@ada4ea8

[anarcat]

How about something like this TheMeier@ada4ea8

that looks fantastic.

[TheMeier]

So how about you steal it into your branch and then squash everything ;)

[anarcat]

So how about you steal it into your branch and then squash everything ;)

done!

... and rebased.

[anarcat]

so this has conflicted again, but before i start fixing this, perhaps it would be best to get a confirmation it's worth my time... ;) is this go or not? especially in view of log restructuring that's happening in parallel in #5072

[Spaceman1701]

I think this still makes sense in light of #5072 (which in my opinion is mostly for machine consumption).

[anarcat]

conflicts resolved, rebased.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a CLI flag --log.silences and a silence.Options.Logging field; when enabled, the silences subsystem emits Info-level logs for create, update, and expire events via a new Silences.logSilence helper. Includes a unit test validating log formatting.

Changes

Cohort / File(s)	Summary
Silence logging implementation silence/silence.go, silence/silence_test.go	Added exported Options.Logging and internal Silences.logSilence; conditionally log create/update/expire events in Set and expire; added unit test that asserts RFC3339 timestamps and matcher serialization.
Command-line integration cmd/alertmanager/main.go	Added --log.silences boolean flag (default false) and passed its value into silence.Options when initializing silences.

sequenceDiagram
    participant CLI as CLI (`main.go`)
    participant Sil as Silences (`silence/silence.go`)
    participant Log as Logger (slog)

    CLI->>Sil: initialize with Options{Logging: flag}
    Note right of Sil: runtime events (create/update/expire)
    Sil->>Log: logSilence(msg, sil) when Logging=true
    Log-->>Sil: write structured log output

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I tap a flag and start to hum,
Silences whisper, then I drum.
IDs, matchers, timestamps neat —
I log their hop with steady beat.
A rabbit's note: concise and sweet.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description explains the motivation and purpose but lacks required template elements like release notes, test confirmation, and specific issue references needed for a complete submission.	Complete the PR checklist with release notes (e.g., '[FEATURE] Add optional silence logging'), confirm tests were added, and reference the related issue number.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add silence logging option' directly and clearly describes the main change: adding a new optional logging feature for silence status changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@silence/silence_test.go`:
- Around line 2842-2866: The test builds a pb.Silence using time.Time for
StartsAt/EndsAt and sets Matchers directly, but pb.Silence.StartAt/EndsAt are
*timestamppb.Timestamp and production code moves matchers into MatcherSets via
Set()/validateSilence()/postprocessUnmarshalledSilence; update the test to
construct StartsAt and EndsAt using timestamppb.New(...) and to populate
MatcherSets (not Matchers) to mirror real post-processed silence objects, and
also update assertions/log expectations if logSilence was changed to read from
MatcherSets instead of Matchers.

In `@silence/silence.go`:
- Around line 1426-1449: logSilence currently iterates over sil.Matchers which
is cleared by postprocessUnmarshalledSilence after validateSilence, so matcher
info is never logged; update logSilence to iterate over sil.MatcherSets (e.g.,
loop through each MatcherSet and then its Matchers to build listMatchers)
instead of sil.Matchers, preserving the same formatting logic and using the
existing matcherTypeOperator mapping; this change ties into functions Set,
validateSilence and postprocessUnmarshalledSilence where MatcherSets is
populated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e701aca-4089-4f2c-ad9a-b5d5f2d99e4a

📥 Commits

Reviewing files that changed from the base of the PR and between 7aa1b9f and 887ecc0.

📒 Files selected for processing (3)

• cmd/alertmanager/main.go
• silence/silence.go
• silence/silence_test.go

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

silence/silence.go (1)

1434-1438: ⚠️ Potential issue | 🔴 Critical

logSilence reads the wrong matcher field, so matcher logs are effectively empty.

At Line 1434, logSilence iterates sil.Matchers, but normal flows clear that field during postprocessing (see Line 1371) before these log calls run. Use sil.MatcherSets instead (and flatten each set’s Matchers).

🐛 Proposed fix

 func (s *Silences) logSilence(msg string, sil *pb.Silence) {
 	var listMatchers []string
 	matcherTypeOperator := map[string]string{
 		"EQUAL":      "=",
 		"REGEXP":     "=~",
 		"NOT_EQUAL":  "!=",
 		"NOT_REGEXP": "!~",
 	}
-	for _, matcher := range sil.Matchers {
-		ms := []string{matcher.Name, matcherTypeOperator[matcher.Type.String()], matcher.Pattern}
-		m := strings.Join(ms, ``)
-		listMatchers = append(listMatchers, m)
+	for _, matcherSet := range sil.MatcherSets {
+		for _, matcher := range matcherSet.Matchers {
+			ms := []string{matcher.Name, matcherTypeOperator[matcher.Type.String()], matcher.Pattern}
+			m := strings.Join(ms, ``)
+			listMatchers = append(listMatchers, m)
+		}
 	}
 	strMatchers := strings.Join(listMatchers, `,`)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@silence/silence.go` around lines 1434 - 1438, The logSilence code is
iterating sil.Matchers which is cleared earlier, producing empty matcher logs;
change the loop to iterate sil.MatcherSets and for each MatcherSet iterate its
Matchers to flatten them into listMatchers (preserving the existing string
assembly using matcher.Name, matcherTypeOperator[matcher.Type.String()],
matcher.Pattern); update the loop that builds listMatchers so it references
MatcherSet.Matchers instead of sil.Matchers and append each formatted matcher
string as before.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@silence/silence_test.go`:
- Around line 2868-2869: The two assertions use fmt.Printf (which returns (int,
error)) and an incorrect format string; replace fmt.Printf with fmt.Sprintf to
produce a string, use the time package constant time.RFC3339 in now.Format, and
for the EndsAt assertion format the expected timestamp as now.Add(time.Hour).For
example, change the require.Contains checks that reference logOutput and now to
compare against fmt.Sprintf("StartsAt=%s", now.Format(time.RFC3339)) and
fmt.Sprintf("EndsAt=%s", now.Add(time.Hour).Format(time.RFC3339)).

---

Duplicate comments:
In `@silence/silence.go`:
- Around line 1434-1438: The logSilence code is iterating sil.Matchers which is
cleared earlier, producing empty matcher logs; change the loop to iterate
sil.MatcherSets and for each MatcherSet iterate its Matchers to flatten them
into listMatchers (preserving the existing string assembly using matcher.Name,
matcherTypeOperator[matcher.Type.String()], matcher.Pattern); update the loop
that builds listMatchers so it references MatcherSet.Matchers instead of
sil.Matchers and append each formatted matcher string as before.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 425fea03-edfc-436a-b8a8-48a6c11024c3

📥 Commits

Reviewing files that changed from the base of the PR and between 887ecc0 and a64d508.

📒 Files selected for processing (3)

• cmd/alertmanager/main.go
• silence/silence.go
• silence/silence_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/alertmanager/main.go

[coderabbitai]

♻️ Duplicate comments (1)

silence/silence.go (1)

1434-1438: ⚠️ Potential issue | 🔴 Critical

Bug: sil.Matchers is always empty when logSilence is called.

After validateSilence() is called, postprocessUnmarshalledSilence() moves matchers to MatcherSets and sets sil.Matchers = nil (line 1371). Since all call sites invoke logSilence after validation, the matchers will never be logged.

Iterate over sil.MatcherSets instead.

,

🐛 Proposed fix to iterate over MatcherSets

-	for _, matcher := range sil.Matchers {
-		ms := []string{matcher.Name, matcherTypeOperator[matcher.Type.String()], matcher.Pattern}
-		m := strings.Join(ms, ``)
-		listMatchers = append(listMatchers, m)
+	for _, ms := range sil.MatcherSets {
+		for _, matcher := range ms.Matchers {
+			m := matcher.Name + matcherTypeOperator[matcher.Type.String()] + matcher.Pattern
+			listMatchers = append(listMatchers, m)
+		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@silence/silence.go` around lines 1434 - 1438, logSilence currently iterates
sil.Matchers which is cleared by postprocessUnmarshalledSilence; change
logSilence to iterate sil.MatcherSets instead. Specifically, replace the loop
over sil.Matchers with a loop over sil.MatcherSets, and for each MatcherSet
iterate its Matchers to build the ms string (using Matcher.Name,
matcherTypeOperator[Matcher.Type.String()], Matcher.Pattern) and append to
listMatchers; keep the same string-joining and append logic so the output format
is unchanged. Ensure references to sil.Matchers are removed/updated and only
sil.MatcherSets, MatcherSet.Match ers, and the existing matcherTypeOperator
mapping are used.

🧹 Nitpick comments (1)

silence/silence.go (1)

1446-1447: Timestamp format may not be human-readable.

Passing *timestamppb.Timestamp directly to slog will use the protobuf String() method, which produces a debug format like seconds:1234 nanos:0 rather than RFC3339. For readable logs, format explicitly.

♻️ Proposed fix for RFC3339 formatting

 	s.logger.Info(
 		msg,
 		"Id", sil.Id,
 		"CreatedBy", sil.CreatedBy,
 		"Comment", sil.Comment,
-		"StartsAt", sil.StartsAt,
-		"EndsAt", sil.EndsAt,
+		"StartsAt", sil.StartsAt.AsTime().Format(time.RFC3339),
+		"EndsAt", sil.EndsAt.AsTime().Format(time.RFC3339),
 		"Matchers", strMatchers,
 	)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@silence/silence.go` around lines 1446 - 1447, The StartsAt/EndsAt fields are
protobuf *timestamppb.Timestamp and will log as debug protobuf strings; convert
each to a Go time and format using RFC3339 before passing to slog (e.g., use
timestamppb.AsTime or t :=
sil.StartsAt.AsTime()/timestamppb.AsTime(sil.StartsAt) and then
t.Format(time.RFC3339)), also guard for nil (use empty string or zero time) so
the logging call that references "StartsAt" and "EndsAt" on the sil object emits
human-readable RFC3339 timestamps instead of the protobuf String() output.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@silence/silence.go`:
- Around line 1434-1438: logSilence currently iterates sil.Matchers which is
cleared by postprocessUnmarshalledSilence; change logSilence to iterate
sil.MatcherSets instead. Specifically, replace the loop over sil.Matchers with a
loop over sil.MatcherSets, and for each MatcherSet iterate its Matchers to build
the ms string (using Matcher.Name, matcherTypeOperator[Matcher.Type.String()],
Matcher.Pattern) and append to listMatchers; keep the same string-joining and
append logic so the output format is unchanged. Ensure references to
sil.Matchers are removed/updated and only sil.MatcherSets, MatcherSet.Match ers,
and the existing matcherTypeOperator mapping are used.

---

Nitpick comments:
In `@silence/silence.go`:
- Around line 1446-1447: The StartsAt/EndsAt fields are protobuf
*timestamppb.Timestamp and will log as debug protobuf strings; convert each to a
Go time and format using RFC3339 before passing to slog (e.g., use
timestamppb.AsTime or t :=
sil.StartsAt.AsTime()/timestamppb.AsTime(sil.StartsAt) and then
t.Format(time.RFC3339)), also guard for nil (use empty string or zero time) so
the logging call that references "StartsAt" and "EndsAt" on the sil object emits
human-readable RFC3339 timestamps instead of the protobuf String() output.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 71a525f1-eb5d-4fc6-baff-2bc073295289

📥 Commits

Reviewing files that changed from the base of the PR and between a64d508 and 9ace411.

📒 Files selected for processing (3)

• cmd/alertmanager/main.go
• silence/silence.go
• silence/silence_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• cmd/alertmanager/main.go
• silence/silence_test.go

[coderabbitai]

🧹 Nitpick comments (1)

silence/silence.go (1)

1442-1450: Consider using snake_case for log keys to match codebase conventions.

Other logging in this file uses snake_case keys (e.g., "silence_id" at line 791, "err" throughout). Using PascalCase here creates inconsistency.

♻️ Suggested change for consistency

 	s.logger.Info(
 		msg,
-		"Id", sil.Id,
-		"CreatedBy", sil.CreatedBy,
-		"Comment", sil.Comment,
-		"StartsAt", sil.StartsAt.AsTime().Format(time.RFC3339),
-		"EndsAt", sil.EndsAt.AsTime().Format(time.RFC3339),
-		"Matchers", strMatchers,
+		"id", sil.Id,
+		"created_by", sil.CreatedBy,
+		"comment", sil.Comment,
+		"starts_at", sil.StartsAt.AsTime().Format(time.RFC3339),
+		"ends_at", sil.EndsAt.AsTime().Format(time.RFC3339),
+		"matchers", strMatchers,
 	)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@silence/silence.go` around lines 1442 - 1450, The log call using
s.logger.Info with keys "Id", "CreatedBy", "Comment", "StartsAt", "EndsAt",
"Matchers" is inconsistent with the codebase snake_case convention; update the
keys to snake_case (e.g., "silence_id" or "id" for sil.Id, "created_by" for
sil.CreatedBy, "comment", "starts_at", "ends_at", "matchers") while keeping the
same values (msg, sil.Id, sil.CreatedBy, sil.Comment,
sil.StartsAt.AsTime().Format(...), sil.EndsAt.AsTime().Format(...), strMatchers)
to match existing logging style used elsewhere in this file and preserve context
from s.logger.Info.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@silence/silence.go`:
- Around line 1442-1450: The log call using s.logger.Info with keys "Id",
"CreatedBy", "Comment", "StartsAt", "EndsAt", "Matchers" is inconsistent with
the codebase snake_case convention; update the keys to snake_case (e.g.,
"silence_id" or "id" for sil.Id, "created_by" for sil.CreatedBy, "comment",
"starts_at", "ends_at", "matchers") while keeping the same values (msg, sil.Id,
sil.CreatedBy, sil.Comment, sil.StartsAt.AsTime().Format(...),
sil.EndsAt.AsTime().Format(...), strMatchers) to match existing logging style
used elsewhere in this file and preserve context from s.logger.Info.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a40b560b-0877-43b0-bca7-eb22233f79d3

📥 Commits

Reviewing files that changed from the base of the PR and between 9ace411 and 6617ff3.

📒 Files selected for processing (3)

• cmd/alertmanager/main.go
• silence/silence.go
• silence/silence_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• cmd/alertmanager/main.go
• silence/silence_test.go

[anarcat]

whoohoo! thanks! :)