bookkeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mme...@apache.org
Subject [bookkeeper] branch master updated: Issue #377: Make Prometheus stats logger registration idempotent
Date Thu, 03 Aug 2017 21:00:17 GMT
This is an automated email from the ASF dual-hosted git repository.

mmerli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 398d332  Issue #377: Make Prometheus stats logger registration idempotent
398d332 is described below

commit 398d33218eb2d9a4932fa69b9b3a29f02b2dce02
Author: Matteo Merli <mmerli@apache.org>
AuthorDate: Thu Aug 3 14:00:09 2017 -0700

    Issue #377: Make Prometheus stats logger registration idempotent
    
    Make sure the metric are only registered once on Prometheus client lib.
    
    Discusses in #377.
    
    Author: Matteo Merli <mmerli@apache.org>
    
    Reviewers: Enrico Olivelli <None>, Sijie Guo <None>
    
    This closes #378 from merlimat/fix-metrics-double-registration, closes #377
---
 .../apache/bookkeeper/stats/PrometheusCounter.java |  3 +-
 .../bookkeeper/stats/PrometheusOpStatsLogger.java  | 23 ++++----
 .../bookkeeper/stats/PrometheusStatsLogger.java    |  6 +-
 .../apache/bookkeeper/stats/PrometheusUtil.java    | 67 ++++++++++++++++++++++
 4 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusCounter.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusCounter.java
index 60e19bd..1e80087 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusCounter.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusCounter.java
@@ -28,7 +28,8 @@ public class PrometheusCounter implements Counter {
     private final Gauge gauge;
 
     public PrometheusCounter(CollectorRegistry registry, String name) {
-        this.gauge = Gauge.build().name(Collector.sanitizeMetricName(name)).help("-").create().register(registry);
+        this.gauge = PrometheusUtil.safeRegister(registry,
+                Gauge.build().name(Collector.sanitizeMetricName(name)).help("-").create());
     }
 
     @Override
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusOpStatsLogger.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusOpStatsLogger.java
index c0b61ef..94ad9b0 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusOpStatsLogger.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusOpStatsLogger.java
@@ -30,17 +30,18 @@ public class PrometheusOpStatsLogger implements OpStatsLogger {
     private final Summary.Child fail;
 
     public PrometheusOpStatsLogger(CollectorRegistry registry, String name) {
-        this.summary = Summary.build().name(name).help("-") //
-                .quantile(0.50, 0.01) //
-                .quantile(0.75, 0.01) //
-                .quantile(0.95, 0.01) //
-                .quantile(0.99, 0.01) //
-                .quantile(0.999, 0.01) //
-                .quantile(0.9999, 0.01) //
-                .quantile(1.0, 0.01) //
-                .maxAgeSeconds(60) //
-                .labelNames("success") //
-                .create().register(registry);
+        this.summary = PrometheusUtil.safeRegister(registry,
+                Summary.build().name(name).help("-") //
+                        .quantile(0.50, 0.01) //
+                        .quantile(0.75, 0.01) //
+                        .quantile(0.95, 0.01) //
+                        .quantile(0.99, 0.01) //
+                        .quantile(0.999, 0.01) //
+                        .quantile(0.9999, 0.01) //
+                        .quantile(1.0, 0.01) //
+                        .maxAgeSeconds(60) //
+                        .labelNames("success") //
+                        .create());
 
         this.success = summary.labels("true");
         this.fail = summary.labels("false");
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusStatsLogger.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusStatsLogger.java
index 6c3e4f1..5e39159 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusStatsLogger.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusStatsLogger.java
@@ -45,8 +45,8 @@ public class PrometheusStatsLogger implements StatsLogger {
 
     @Override
     public <T extends Number> void registerGauge(String name, Gauge<T> gauge)
{
-        io.prometheus.client.Gauge.build().name(completeName(name)).help("-").create()
-                .setChild(new io.prometheus.client.Gauge.Child() {
+        PrometheusUtil.safeRegister(registry, io.prometheus.client.Gauge.build().name(completeName(name)).help("-")
+                .create().setChild(new io.prometheus.client.Gauge.Child() {
                     @Override
                     public double get() {
                         Number value = null;
@@ -61,7 +61,7 @@ public class PrometheusStatsLogger implements StatsLogger {
                         }
                         return value.doubleValue();
                     }
-                }).register(registry);
+                }));
     }
 
     @Override
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusUtil.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusUtil.java
new file mode 100644
index 0000000..6ccc645
--- /dev/null
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/PrometheusUtil.java
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.bookkeeper.stats;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.CollectorRegistry;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Container for Prometheus utility methods.
+ *
+ */
+public class PrometheusUtil {
+
+    private static final Field collectorsMapField;
+    private static final Method collectorsNamesMethod;
+
+    static {
+        try {
+            collectorsMapField = CollectorRegistry.class.getDeclaredField("namesToCollectors");
+            collectorsMapField.setAccessible(true);
+
+            collectorsNamesMethod = CollectorRegistry.class.getDeclaredMethod("collectorNames",
String.class);
+            collectorsNamesMethod.setAccessible(true);
+
+        } catch (NoSuchFieldException | SecurityException | NoSuchMethodException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    public static <T extends Collector> T safeRegister(CollectorRegistry registry,
T collector) {
+        try {
+            registry.register(collector);
+            return collector;
+        } catch (IllegalArgumentException e) {
+            // Collector is already registered. Return the existing instance
+            try {
+                Map<String, Collector> collectorsMap = (Map<String, Collector>)
collectorsMapField.get(registry);
+                List<String> collectorNames = (List<String>) collectorsNamesMethod.invoke(registry,
collector);
+                return (T) collectorsMap.get(collectorNames.get(0));
+
+            } catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException
e1) {
+                throw new RuntimeException(e1);
+            }
+        }
+    }
+}

-- 
To stop receiving notification emails like this one, please contact
['"commits@bookkeeper.apache.org" <commits@bookkeeper.apache.org>'].

Mime
View raw message