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

Support limits for silences #8241

Merged
merged 6 commits into from
May 31, 2024

Conversation

grobinson-grafana
Copy link
Contributor

@grobinson-grafana grobinson-grafana commented May 31, 2024

What this PR does

This pull request adds support for silence limits prometheus/alertmanager#3852.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

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

pkg/util/validation/limits.go Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
@grobinson-grafana grobinson-grafana force-pushed the grobinson/support-silence-limits-in-mimir branch 4 times, most recently from 7e78842 to f2ae89c Compare May 31, 2024 19:43
@grobinson-grafana grobinson-grafana force-pushed the grobinson/support-silence-limits-in-mimir branch from 3126d2d to 7d7a12c Compare May 31, 2024 20:02
@grobinson-grafana grobinson-grafana marked this pull request as ready for review May 31, 2024 20:02
@grobinson-grafana grobinson-grafana requested review from grafanabot and a team as code owners May 31, 2024 20:02
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM with one non-blocking comment

Metrics: am.registry,
Limits: silence.Limits{
MaxSilences: cfg.Limits.AlertmanagerMaxSilencesCount(cfg.UserID),
MaxPerSilenceBytes: cfg.Limits.AlertmanagerMaxSilenceSizeBytes(cfg.UserID),
Copy link
Contributor

@56quarters 56quarters May 31, 2024

Choose a reason for hiding this comment

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

max-per-silence-bytes makes this limit a lot easier to understand (compared to max-silence-size-bytes). WDYT about using max per silence bytes everywhere? OK too if you'd like to keep it as-is for consistency with other limits.

EDIT: lol, I see @alexweav suggested the opposite. Ignore me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to keep it consistent with other limits 🙂

@grobinson-grafana grobinson-grafana merged commit 2ead4da into main May 31, 2024
29 checks passed
@grobinson-grafana grobinson-grafana deleted the grobinson/support-silence-limits-in-mimir branch May 31, 2024 21:51
duricanikolic added a commit that referenced this pull request Jun 2, 2024
narqo pushed a commit to narqo/grafana-mimir that referenced this pull request Jun 6, 2024
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.

3 participants