You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/11/07 08:21:59 UTC

[GitHub] [zookeeper] maoling opened a new pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

maoling opened a new pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528


   - Apply this patch, we can export ZooKeeper version as prometheus metric. For example:
   ```
   leader_unavailable_time_sum 0.0
   # HELP zk_version_info ZooKeeper version info
   # TYPE zk_version_info gauge
   zk_version_info{version="3.7.0-SNAPSHOT",revision_hash="b761b77a3bf8cd02edaad88a33bafbc8b6c65be4",built_on="2020-11-07 08:03 UTC",} 1.0
   # HELP ensemble_auth_success ensemble_auth_success
   ```
   
   - more details in the [ZOOKEEPER-3982](https://issues.apache.org/jira/browse/ZOOKEEPER-3982)


----------------------------------------------------------------
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] [zookeeper] maoling commented on a change in pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#discussion_r528193191



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/VersionInfoExports.java
##########
@@ -0,0 +1,44 @@
+package org.apache.zookeeper.metrics.prometheus;

Review comment:
       Oops, I will fix it tomorrow. A little surprising, let me re-push this patch and check whether the CI or checkstyle could detect this license missing issue.




----------------------------------------------------------------
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] [zookeeper] maoling commented on a change in pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#discussion_r669588617



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
##########
@@ -70,6 +70,7 @@
     private Server server;
     private final MetricsServletImpl servlet = new MetricsServletImpl();
     private final Context rootContext = new Context();
+    public final CustomizedExports customizedExports = CustomizedExports.instance();

Review comment:
       I need to clean up customizedExports in the PrometheusMetricsProviderTest#setup manually because customizedExports is the text information which will pollute all the existing UTs in the PrometheusMetricsProviderTest




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

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] pkuwm commented on a change in pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#discussion_r528013978



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/VersionInfoExports.java
##########
@@ -0,0 +1,44 @@
+package org.apache.zookeeper.metrics.prometheus;

Review comment:
       Hey @maoling, maybe apache license is required?




----------------------------------------------------------------
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] [zookeeper] maoling commented on pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#issuecomment-723439199


   > Why do you want to expose this value only on Prometheus?
   
   @eolivelli Could you please point out what place else I should expose? `DefaultMetricsProvider`?


----------------------------------------------------------------
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] [zookeeper] maoling commented on pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#issuecomment-869513158


   @eolivelli  PTAL. Cc @huizhilu 


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

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling closed pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling closed pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528


   


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

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] eolivelli commented on a change in pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#discussion_r661484410



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
##########
@@ -70,6 +70,7 @@
     private Server server;
     private final MetricsServletImpl servlet = new MetricsServletImpl();
     private final Context rootContext = new Context();
+    public final CustomizedExports customizedExports = CustomizedExports.instance();

Review comment:
       why "public" ?
   it is initialised in `configure` and you can stop it in the close method
   
   no need to do something special in the tests 




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

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling commented on pull request #1528: ZOOKEEPER-3982: export ZooKeeper version as prometheus metric

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1528:
URL: https://github.com/apache/zookeeper/pull/1528#issuecomment-879870905


   > I am not sure we have to write the version to Prometheus. why do you want do to so ?
   
   @eolivelli 
   - The version info is a valuable metric for users to diagnose/report issues although we can find this from 
   other ways(e.g., log, JMX, zkServer.sh version).
   - A mature software should have this function  in its metric system. Let me give two example:
   
   ```
   Example A:
   JDK exposes its version metric like this:
   jvm_info{version="1.8.0_191-b12",vendor="Oracle Corporation",runtime="Java(TM) SE Runtime Environment",} 1.0
   
   Example B:
   Etcd exposes its version metric like this:
   curl -L http://localhost:2379/metrics
   # HELP etcd_server_version Which version is running. 1 for 'server_version' label with current version.
   # TYPE etcd_server_version gauge
   etcd_server_version{server_version="3.3.13"} 1
   ```


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

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org