From c3a1f54fc2dce975e99787abe0ae3a21c63ea8e0 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 20 Jun 2024 12:02:05 +0100 Subject: [PATCH 1/3] Replace incorrect use of fmt.Errorf (#3883) --- silence/silence.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/silence/silence.go b/silence/silence.go index 67ba97dbb2..51c976ee33 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -44,10 +44,10 @@ import ( ) // ErrNotFound is returned if a silence was not found. -var ErrNotFound = fmt.Errorf("silence not found") +var ErrNotFound = errors.New("silence not found") // ErrInvalidState is returned if the state isn't valid. -var ErrInvalidState = fmt.Errorf("invalid state") +var ErrInvalidState = errors.New("invalid state") type matcherCache map[*pb.Silence]labels.Matchers @@ -338,7 +338,7 @@ type Options struct { func (o *Options) validate() error { if o.SnapshotFile != "" && o.SnapshotReader != nil { - return fmt.Errorf("only one of SnapshotFile and SnapshotReader must be set") + return errors.New("only one of SnapshotFile and SnapshotReader must be set") } return nil } From b7675684eeda6c13bb124c6c37507797f2ad5efd Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 20 Jun 2024 14:50:53 +0100 Subject: [PATCH 2/3] Silence limits as functions (#3885) * Silence limits as functions This commit changes silence limits from a struct of ints to a struct of functions that return individual limits. This allows limits to be lazy-loaded and updated without having to call silences.New(). Signed-off-by: George Robinson * Add explicit test for no limits Signed-off-by: George Robinson * Fix run() Signed-off-by: George Robinson --------- Signed-off-by: George Robinson --- cmd/alertmanager/main.go | 4 ++-- silence/silence.go | 17 ++++++++++------- silence/silence_test.go | 22 ++++++++++++++++++++-- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index 4204e916f2..d14ed4be1f 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -261,8 +261,8 @@ func run() int { SnapshotFile: filepath.Join(*dataDir, "silences"), Retention: *retention, Limits: silence.Limits{ - MaxSilences: *maxSilences, - MaxPerSilenceBytes: *maxPerSilenceBytes, + MaxSilences: func() int { return *maxSilences }, + MaxPerSilenceBytes: func() int { return *maxPerSilenceBytes }, }, Logger: log.With(logger, "component", "silences"), Metrics: prometheus.DefaultRegisterer, diff --git a/silence/silence.go b/silence/silence.go index 51c976ee33..8368d52c88 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -206,10 +206,10 @@ type Silences struct { type Limits struct { // MaxSilences limits the maximum number of silences, including expired // silences. - MaxSilences int + MaxSilences func() int // MaxPerSilenceBytes is the maximum size of an individual silence as // stored on disk. - MaxPerSilenceBytes int + MaxPerSilenceBytes func() int } // MaintenanceFunc represents the function to run as part of the periodic maintenance for silences. @@ -585,8 +585,11 @@ func (s *Silences) setSilence(sil *pb.Silence, now time.Time, skipValidate bool) // Check the limit unless the silence has been expired. This is to avoid // situations where silences cannot be expired after the limit has been // reduced. - if n := msil.Size(); s.limits.MaxPerSilenceBytes > 0 && n > s.limits.MaxPerSilenceBytes && sil.EndsAt.After(now) { - return fmt.Errorf("silence exceeded maximum size: %d bytes (limit: %d bytes)", n, s.limits.MaxPerSilenceBytes) + if s.limits.MaxPerSilenceBytes != nil { + n := msil.Size() + if m := s.limits.MaxPerSilenceBytes(); m > 0 && n > m && sil.EndsAt.After(now) { + return fmt.Errorf("silence exceeded maximum size: %d bytes (limit: %d bytes)", n, m) + } } if s.st.merge(msil, now) { @@ -645,9 +648,9 @@ func (s *Silences) set(sil *pb.Silence) (string, error) { } // If we got here it's either a new silence or a replacing one. - if s.limits.MaxSilences > 0 { - if len(s.st)+1 > s.limits.MaxSilences { - return "", fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), s.limits.MaxSilences) + if s.limits.MaxSilences != nil { + if m := s.limits.MaxSilences(); m > 0 && len(s.st)+1 > m { + return "", fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), m) } } diff --git a/silence/silence_test.go b/silence/silence_test.go index f722260488..11c52823c8 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -462,8 +462,8 @@ func TestSilenceSet(t *testing.T) { func TestSilenceLimits(t *testing.T) { s, err := New(Options{ Limits: Limits{ - MaxSilences: 1, - MaxPerSilenceBytes: 2 << 11, // 4KB + MaxSilences: func() int { return 1 }, + MaxPerSilenceBytes: func() int { return 2 << 11 }, // 4KB }, }) require.NoError(t, err) @@ -535,6 +535,24 @@ func TestSilenceLimits(t *testing.T) { require.Equal(t, "", id3) } +func TestSilenceNoLimits(t *testing.T) { + s, err := New(Options{ + Limits: Limits{}, + }) + require.NoError(t, err) + + // Insert sil should succeed without error. + sil := &pb.Silence{ + Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(5 * time.Minute), + Comment: strings.Repeat("c", 2<<9), + } + id, err := s.Set(sil) + require.NoError(t, err) + require.NotEqual(t, "", id) +} + func TestSilenceUpsert(t *testing.T) { s, err := New(Options{ Retention: time.Hour, From edab848b6cee652faf41a5fc5bdd47dc9d804ec6 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 20 Jun 2024 15:20:52 +0100 Subject: [PATCH 3/3] Rename silence limit to max-silence-size-bytes (#3886) * Rename silence limit to max-silence-size-bytes This commit renames an existing (unreleased) limit from max-per-silence-bytes to max-silence-size-bytes. Signed-off-by: George Robinson * Update help Signed-off-by: George Robinson --------- Signed-off-by: George Robinson --- cmd/alertmanager/main.go | 6 +++--- silence/silence.go | 8 ++++---- silence/silence_test.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index d14ed4be1f..613e26b334 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -146,7 +146,7 @@ func run() int { retention = kingpin.Flag("data.retention", "How long to keep data for.").Default("120h").Duration() maintenanceInterval = kingpin.Flag("data.maintenance-interval", "Interval between garbage collection and snapshotting to disk of the silences and the notification logs.").Default("15m").Duration() maxSilences = kingpin.Flag("silences.max-silences", "Maximum number of silences, including expired silences. If negative or zero, no limit is set.").Default("0").Int() - maxPerSilenceBytes = kingpin.Flag("silences.max-per-silence-bytes", "Maximum per silence size in bytes. If negative or zero, no limit is set.").Default("0").Int() + maxSilenceSizeBytes = kingpin.Flag("silences.max-silence-size-bytes", "Maximum silence size in bytes. If negative or zero, no limit is set.").Default("0").Int() alertGCInterval = kingpin.Flag("alerts.gc-interval", "Interval between alert GC.").Default("30m").Duration() webConfig = webflag.AddFlags(kingpin.CommandLine, ":9093") @@ -261,8 +261,8 @@ func run() int { SnapshotFile: filepath.Join(*dataDir, "silences"), Retention: *retention, Limits: silence.Limits{ - MaxSilences: func() int { return *maxSilences }, - MaxPerSilenceBytes: func() int { return *maxPerSilenceBytes }, + MaxSilences: func() int { return *maxSilences }, + MaxSilenceSizeBytes: func() int { return *maxSilenceSizeBytes }, }, Logger: log.With(logger, "component", "silences"), Metrics: prometheus.DefaultRegisterer, diff --git a/silence/silence.go b/silence/silence.go index 8368d52c88..5c52e853e8 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -207,9 +207,9 @@ type Limits struct { // MaxSilences limits the maximum number of silences, including expired // silences. MaxSilences func() int - // MaxPerSilenceBytes is the maximum size of an individual silence as + // MaxSilenceSizeBytes is the maximum size of an individual silence as // stored on disk. - MaxPerSilenceBytes func() int + MaxSilenceSizeBytes func() int } // MaintenanceFunc represents the function to run as part of the periodic maintenance for silences. @@ -585,9 +585,9 @@ func (s *Silences) setSilence(sil *pb.Silence, now time.Time, skipValidate bool) // Check the limit unless the silence has been expired. This is to avoid // situations where silences cannot be expired after the limit has been // reduced. - if s.limits.MaxPerSilenceBytes != nil { + if s.limits.MaxSilenceSizeBytes != nil { n := msil.Size() - if m := s.limits.MaxPerSilenceBytes(); m > 0 && n > m && sil.EndsAt.After(now) { + if m := s.limits.MaxSilenceSizeBytes(); m > 0 && n > m && sil.EndsAt.After(now) { return fmt.Errorf("silence exceeded maximum size: %d bytes (limit: %d bytes)", n, m) } } diff --git a/silence/silence_test.go b/silence/silence_test.go index 11c52823c8..b632562630 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -462,8 +462,8 @@ func TestSilenceSet(t *testing.T) { func TestSilenceLimits(t *testing.T) { s, err := New(Options{ Limits: Limits{ - MaxSilences: func() int { return 1 }, - MaxPerSilenceBytes: func() int { return 2 << 11 }, // 4KB + MaxSilences: func() int { return 1 }, + MaxSilenceSizeBytes: func() int { return 2 << 11 }, // 4KB }, }) require.NoError(t, err)