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

Dispatcher groups limits #4254

Merged
merged 6 commits into from
Jun 9, 2021

Conversation

pstibrany
Copy link
Contributor

@pstibrany pstibrany commented Jun 3, 2021

What this PR does: This PR exposes new alertmanager limit: number of active dispatcher groups in Alertmanager.

This PR builds on top of #4237, and will be rebased once #4237 is merged.

Checklist

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

@pstibrany
Copy link
Contributor Author

This PR builds on top of #4237, marking as draft until it is merged.

@pstibrany pstibrany marked this pull request as draft June 3, 2021 14:50
@pstibrany pstibrany force-pushed the dispatcher-groups-limits branch from 4b93c22 to 908e04c Compare June 3, 2021 16:53
@pull-request-size pull-request-size bot added size/L and removed size/XL labels Jun 3, 2021
@pstibrany
Copy link
Contributor Author

Rebased on top of master now, ready for review.

@pstibrany pstibrany marked this pull request as ready for review June 3, 2021 16:53
Copy link
Contributor

@stevesg stevesg left a comment

Choose a reason for hiding this comment

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

LGTM. Can't see anything wrong here!

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Good job! LGTM (modulo a comment about config name which I think we can make a bit more consistent with the rest of the config).

Should we mark these new limits as experimental?

@pstibrany
Copy link
Contributor Author

Thanks for review.

Should we mark these new limits as experimental?

I've marked all recently added Alertmanager limits as experimental for now.

pstibrany and others added 5 commits June 8, 2021 21:15
@pstibrany pstibrany force-pushed the dispatcher-groups-limits branch from 0dd19e5 to 4618a92 Compare June 8, 2021 19:16
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM (modulo a fix to the CHANGELOG entry). Thanks!

Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany pstibrany merged commit 65fbad2 into cortexproject:master Jun 9, 2021
@pstibrany pstibrany deleted the dispatcher-groups-limits branch June 9, 2021 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants