Make JvmMetrics.register idempotent with the default registry (#987)

* Make JvmMetrics.register idempotent with the default registry Signed-off-by: Mickael Maison <[email protected]> * Make JvmMetrics.register() methods idempotent Signed-off-by: Mickael Maison <[email protected]> --------- Signed-off-by: Mickael Maison <[email protected]>
prometheus · Sep 30, 2024 · 14c0908 · 14c0908
1 parent 4ce0d15
commit 14c0908
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 21 deletions.
diff --git a/...strumentation-jvm/src/main/java/io/prometheus/metrics/instrumentation/jvm/JvmMetrics.java b/...strumentation-jvm/src/main/java/io/prometheus/metrics/instrumentation/jvm/JvmMetrics.java
@@ -3,7 +3,8 @@
 import io.prometheus.metrics.config.PrometheusProperties;
 import io.prometheus.metrics.model.registry.PrometheusRegistry;
 
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Registers all JVM metrics. Example usage:
@@ -13,15 +14,14 @@
  */
 public class JvmMetrics {
 
-    private static AtomicBoolean registeredWithTheDefaultRegistry = new AtomicBoolean(false);
-
+    private static final Set<PrometheusRegistry> REGISTERED = ConcurrentHashMap.newKeySet();
     public static Builder builder() {
         return new Builder(PrometheusProperties.get());
     }
 
     // Note: Currently there is no configuration for JVM metrics, so it doesn't matter whether you pass a config or not.
     // However, we will add config options in the future, like whether you want to use Prometheus naming conventions
-    //'or OpenTelemetry semantic conventions for JVM metrics.
+    // or OpenTelemetry semantic conventions for JVM metrics.
     public static Builder builder(PrometheusProperties config) {
         return new Builder(config);
     }
@@ -37,32 +37,32 @@ private Builder(PrometheusProperties config) {
         /**
          * Register all JVM metrics with the default registry.
          * <p>
-         * It's safe to call this multiple times:
-         * Only the first call will register the metrics, all subsequent calls will be ignored.
+         * It's safe to call this multiple times, only the first call will register the metrics, all subsequent calls
+         * will be ignored.
          */
         public void register() {
-            if (!registeredWithTheDefaultRegistry.getAndSet(true)) {
-                register(PrometheusRegistry.defaultRegistry);
-            }
+            register(PrometheusRegistry.defaultRegistry);
         }
 
         /**
          * Register all JVM metrics with the {@code registry}.
          * <p>
-         * You must make sure to call this only once per {@code registry}, otherwise it will
-         * throw an Exception because you are trying to register duplicate metrics.
+         * It's safe to call this multiple times, only the first call will register the metrics, all subsequent calls
+         * will be ignored.
          */
         public void register(PrometheusRegistry registry) {
-            JvmThreadsMetrics.builder(config).register(registry);
-            JvmBufferPoolMetrics.builder(config).register(registry);
-            JvmClassLoadingMetrics.builder(config).register(registry);
-            JvmCompilationMetrics.builder(config).register(registry);
-            JvmGarbageCollectorMetrics.builder(config).register(registry);
-            JvmMemoryPoolAllocationMetrics.builder(config).register(registry);
-            JvmMemoryMetrics.builder(config).register(registry);
-            JvmNativeMemoryMetrics.builder(config).register(registry);
-            JvmRuntimeInfoMetric.builder(config).register(registry);
-            ProcessMetrics.builder(config).register(registry);
+            if (REGISTERED.add(registry)) {
+                JvmThreadsMetrics.builder(config).register(registry);
+                JvmBufferPoolMetrics.builder(config).register(registry);
+                JvmClassLoadingMetrics.builder(config).register(registry);
+                JvmCompilationMetrics.builder(config).register(registry);
+                JvmGarbageCollectorMetrics.builder(config).register(registry);
+                JvmMemoryPoolAllocationMetrics.builder(config).register(registry);
+                JvmMemoryMetrics.builder(config).register(registry);
+                JvmNativeMemoryMetrics.builder(config).register(registry);
+                JvmRuntimeInfoMetric.builder(config).register(registry);
+                ProcessMetrics.builder(config).register(registry);
+            }
         }
     }
 }
diff --git a/...mentation-jvm/src/test/java/io/prometheus/metrics/instrumentation/jvm/JvmMetricsTest.java b/...mentation-jvm/src/test/java/io/prometheus/metrics/instrumentation/jvm/JvmMetricsTest.java
@@ -0,0 +1,19 @@
+package io.prometheus.metrics.instrumentation.jvm;
+
+import io.prometheus.metrics.model.registry.PrometheusRegistry;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class JvmMetricsTest {
+
+    @Test
+    public void testRegisterIdempotent() {
+        PrometheusRegistry registry = new PrometheusRegistry();
+        assertEquals(0, registry.scrape().size());
+        JvmMetrics.builder().register(registry);
+        assertTrue(registry.scrape().size() > 0);
+        JvmMetrics.builder().register(registry);
+    }
+}