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 multi-target pattern #873

Merged
merged 7 commits into from Oct 30, 2023
Merged

Support multi-target pattern #873

merged 7 commits into from Oct 30, 2023

Conversation

ganzuoni
Copy link
Contributor

Porting of the same implementation to main branch

Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

Hi @ganzuoni, thanks a lot for the PR! I added a few comments, nothing major, mostly refactoring.

BTW while reviewing it I found a few shortcomings in the current implementation and merged #875 and #876. Looks like multi target support is the first time that we look closely at using the Collector interface directly. Thanks a lot for coming up with that use case!

Please rebase to the current main so that you get the latest changes.

Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

Thanks for the update @ganzuoni, I re-opened three conversations, mostly with things I think can be removed :).

Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

Thanks a lot @ganzuoni. We are almost there, just one more comment on the ExtendedMultiCollector thread.

Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!!!

@fstab fstab merged commit cff40dd into prometheus:main Oct 30, 2023
2 checks passed
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.

None yet

2 participants