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 2021/04/28 23:08:43 UTC

[GitHub] [zookeeper] li4wang opened a new pull request #1688: ZOOKEEPER-4287: Upgrade prometheus client library version to 0.10.0

li4wang opened a new pull request #1688:
URL: https://github.com/apache/zookeeper/pull/1688


   Author: Li Wang <li...@gmail.com>


-- 
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] eolivelli commented on a change in pull request #1688: ZOOKEEPER-4287: Upgrade prometheus client library version to 0.10.0

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



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java
##########
@@ -76,31 +77,51 @@ public void testCounters() throws Exception {
         counter.add(10);
         int[] count = {0};
         provider.dump((k, v) -> {
-            assertEquals("cc", k);
-            assertEquals(10, ((Number) v).intValue());
             count[0]++;
+            int value = ((Number) v).intValue();
+            switch (k) {
+                case "cc_total":

Review comment:
       so with this upgrade we are changing the metrics that we are providing ?
   
   can you please explain in the description of the PR and/or in JIRA ?
   
   this may be a significant impact for people who created downstream processing dashboards/alerts




-- 
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] li4wang commented on a change in pull request #1688: ZOOKEEPER-4287: Upgrade prometheus client library version to 0.10.0

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



##########
File path: zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java
##########
@@ -76,31 +77,51 @@ public void testCounters() throws Exception {
         counter.add(10);
         int[] count = {0};
         provider.dump((k, v) -> {
-            assertEquals("cc", k);
-            assertEquals(10, ((Number) v).intValue());
             count[0]++;
+            int value = ((Number) v).intValue();
+            switch (k) {
+                case "cc_total":

Review comment:
       Yes, Prometheus switched to OpenMetrics data model in 0.10.0 release. These means the following. 
   
   1.  all Counter samples now have a _total suffix
   2. Counter, Summary, and Histogram metrics now expose a _created sample with when their child was created
   
   More details can be found in the Prometheus change logs https://github.com/prometheus/client_java/releases
   
   Yes, the downstream processing dashboard/alerts need to make the change accordingly when they upgrade ZK version. 
   




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