You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/03/01 12:37:02 UTC
[GitHub] [bookkeeper] hangc0276 opened a new pull request #2633: add flag to control whether basic metrics expose to prometheus or not
hangc0276 opened a new pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633
### Motivation
When using prometheus-metrics-provider lib provided by BookKeeper, it will expose basic metrics such as cpu, jvm, memory metrics etc. However, other systems, eg. Apache Pulsar, may also expose those metrics, which will lead to those metrics register twice, and prometheus metrics provider thread will start failed.
### Changes
1. Add a flag to control whether basic metrics expose to prometheus or not.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633#issuecomment-808863325
> @eolivelli can the names be changed there
>
> https://github.com/apache/bookkeeper/blob/5700641ea509be36a6395f56aae2f60a88fdcaab/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java#L113
>
> otherwise standard metrics can be extended / collect() overridden to provide overridden name https://github.com/prometheus/client_java/blob/7a6b9f1300c5445f477fb0452c5ec7b6f80464a1/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/GarbageCollectorExports.java#L40
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] dlg99 commented on pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633#issuecomment-790948638
@eolivelli can the names be changed there https://github.com/apache/bookkeeper/blob/5700641ea509be36a6395f56aae2f60a88fdcaab/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java#L113
otherwise standard metrics can be extended / collect() overridden to provide overridden name https://github.com/prometheus/client_java/blob/7a6b9f1300c5445f477fb0452c5ec7b6f80464a1/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/GarbageCollectorExports.java#L40
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 removed a comment on pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
hangc0276 removed a comment on pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633#issuecomment-808863325
> @eolivelli can the names be changed there
>
> https://github.com/apache/bookkeeper/blob/5700641ea509be36a6395f56aae2f60a88fdcaab/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java#L113
>
> otherwise standard metrics can be extended / collect() overridden to provide overridden name https://github.com/prometheus/client_java/blob/7a6b9f1300c5445f477fb0452c5ec7b6f80464a1/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/GarbageCollectorExports.java#L40
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 closed pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
hangc0276 closed pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633#issuecomment-808869155
> @eolivelli can the names be changed there
>
> https://github.com/apache/bookkeeper/blob/5700641ea509be36a6395f56aae2f60a88fdcaab/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java#L113
>
> otherwise standard metrics can be extended / collect() overridden to provide overridden name https://github.com/prometheus/client_java/blob/7a6b9f1300c5445f477fb0452c5ec7b6f80464a1/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/GarbageCollectorExports.java#L40
@dlg99 Thanks for you feedback. This method can override all the metrics' name when exposing to http port, but the metics name which registered into `CollectorRegistry` can't be override. Thus it will also lead to the same metric name registered twice and the prometheus metrics provider thread start failed. Thus, we should keep the flag to turn off `standard metrics` register and expose.
I also update the code and support adding metric prefix on metric name for standard metrics and pre-defined metrics. For user registered metric, and metric prefix won't be added.
@eolivelli @dlg99 PTAL thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2633: add flag to control whether basic metrics expose to prometheus or not
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2633:
URL: https://github.com/apache/bookkeeper/pull/2633#issuecomment-790443048
@dlg99
I believe that the standard metrics cannot be renamed
```
registerMetrics(new StandardExports());
registerMetrics(new MemoryPoolsExports());
registerMetrics(new GarbageCollectorExports());
registerMetrics(new ThreadExports());
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org