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

pkg/prom/instance: create a ModalManager for ApplyConfig #427

Merged
merged 5 commits into from
Feb 26, 2021

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Feb 24, 2021

PR Description

ApplyConfig for the pkg/prom ApplyConfig logic will need to be able to mutate the instance grouping mode and other settings. This PR enables that by doing the following:

  1. Create a ModalManager that abstracts away the shared/distinct grouping logic. The ModalManager can change modes at the cost of needing to re-apply all instances.

  2. Remove CountingManager and migrate its logic to ModalManager

  3. Separate out storage of the ModalManager and BasicManager to be able to control both independently

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG updated (internal change, not needed)
  • Documentation added (internal change, not needed)
  • Tests updated

ApplyConfig for the pkg/prom will need to be able to mutate the instance
grouping mode and other settings. This PR enables that by doing the
following:

1. Create a ModalManager that wraps around a GroupManager or a (new)
   DistinctManager. The ModalManager can change modes at the cost of
   needing to re-apply all instances.

2. Remove CountingManager and migrate its logic to ModalManager

3. Separate out storage of the ModalManager and BasicManager to be able
   to control both independently
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.

Looks good. A few non-blocking suggestions and questions.

pkg/prom/agent.go Show resolved Hide resolved
pkg/prom/instance/group_manager.go Outdated Show resolved Hide resolved
pkg/prom/instance/modal_manager.go Outdated Show resolved Hide resolved
pkg/prom/instance/modal_manager.go Outdated Show resolved Hide resolved
pkg/prom/instance/modal_manager.go Outdated Show resolved Hide resolved
@rfratto rfratto merged commit cff0db0 into grafana:main Feb 26, 2021
@rfratto rfratto deleted the modal-manager branch February 26, 2021 17:11
@rfratto rfratto mentioned this pull request Feb 26, 2021
3 tasks
@mattdurham mattdurham mentioned this pull request Sep 7, 2021
3 tasks
mattdurham pushed a commit that referenced this pull request Nov 11, 2021
* pkg/prom/instance: create a ModalManager for ApplyConfig

ApplyConfig for the pkg/prom will need to be able to mutate the instance
grouping mode and other settings. This PR enables that by doing the
following:

1. Create a ModalManager that wraps around a GroupManager or a (new)
   DistinctManager. The ModalManager can change modes at the cost of
   needing to re-apply all instances.

2. Remove CountingManager and migrate its logic to ModalManager

3. Separate out storage of the ModalManager and BasicManager to be able
   to control both independently

* remove DistinctManager, use underlying manager directly

* simplify GroupManager.Stop

* removed unused logger from GroupManager

* document active/wrapped managers a bit better in ModalManager
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Apr 17, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants