Skip to content

Support multi-target pattern#873

Merged
fstab merged 7 commits intoprometheus:mainfrom
ganzuoni:main
Oct 30, 2023
Merged

Support multi-target pattern#873
fstab merged 7 commits intoprometheus:mainfrom
ganzuoni:main

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
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.

2 participants