-
Notifications
You must be signed in to change notification settings - Fork 808
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
Conversation
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
There was a problem hiding this 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.
...pserver/src/main/java/io/prometheus/metrics/examples/httpserver/SampleExtendedCollector.java
Outdated
Show resolved
Hide resolved
...rter-httpserver/src/main/java/io/prometheus/metrics/examples/httpserver/MainMultiTarget.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/io/prometheus/metrics/examples/httpserver/SampleExtendedMultiCollector.java
Outdated
Show resolved
Hide resolved
...metrics-model/src/main/java/io/prometheus/metrics/model/registry/ExtendedMultiCollector.java
Outdated
Show resolved
Hide resolved
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java
Outdated
Show resolved
Hide resolved
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java
Outdated
Show resolved
Hide resolved
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java
Outdated
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
Signed-off-by: Guido Anzuoni <ganzuoni@gmail.com>
There was a problem hiding this 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>
There was a problem hiding this 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>
There was a problem hiding this 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!!!
Porting of the same implementation to main branch