Create a unique key for cached metric maps

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
hyperledger · Nov 22, 2024 · f6c79cc · f6c79cc
1 parent 5a577d2
commit f6c79cc
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 35 deletions.
diff --git a/...eus/PrometheusSuppliedValueCollector.java → ...ractPrometheusSuppliedValueCollector.java b/...eus/PrometheusSuppliedValueCollector.java → ...ractPrometheusSuppliedValueCollector.java
@@ -28,15 +28,15 @@
 import io.prometheus.metrics.model.registry.PrometheusRegistry;
 import io.prometheus.metrics.model.snapshots.DataPointSnapshot;
 
-abstract class PrometheusSuppliedValueCollector extends CategorizedPrometheusCollector
+abstract class AbstractPrometheusSuppliedValueCollector extends CategorizedPrometheusCollector
     implements LabelledSuppliedMetric {
   /** The collector */
   protected final Collector collector;
 
   /** Map label values with the collector callback data */
   protected final Map<List<String>, CallbackData> labelledCallbackData = new ConcurrentHashMap<>();
 
-  protected PrometheusSuppliedValueCollector(
+  protected AbstractPrometheusSuppliedValueCollector(
       final MetricCategory category,
       final String name,
       final String help,

diff --git a/...s/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java b/...s/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java
@@ -32,7 +32,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.DoubleSupplier;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
@@ -65,9 +64,9 @@ public class PrometheusMetricsSystem implements ObservableMetricsSystem {
   private final Map<MetricCategory, Collection<PrometheusCollector>> collectors =
       new ConcurrentHashMap<>();
   private final PrometheusRegistry registry = PrometheusRegistry.defaultRegistry;
-  private final Map<String, LabelledMetric<org.hyperledger.besu.plugin.services.metrics.Counter>>
-      cachedCounters = new ConcurrentHashMap<>();
-  private final Map<String, LabelledMetric<OperationTimer>> cachedTimers =
+  private final Map<CachedMetricKey, LabelledMetric<Counter>> cachedCounters =
+      new ConcurrentHashMap<>();
+  private final Map<CachedMetricKey, LabelledMetric<OperationTimer>> cachedTimers =
       new ConcurrentHashMap<>();
   private final PrometheusGuavaCache.Context guavaCacheCollectorContext =
       new PrometheusGuavaCache.Context();
@@ -117,15 +116,14 @@ public LabelledMetric<Counter> createLabelledCounter(
       final String help,
       final String... labelNames) {
     return cachedCounters.computeIfAbsent(
-        name,
-        (k) -> {
+        CachedMetricKey.of(category, name),
+        k -> {
           if (isCategoryEnabled(category)) {
             final var counter = new PrometheusCounter(category, name, help, labelNames);
             registerCollector(category, counter);
             return counter;
-          } else {
-            return NoOpMetricsSystem.getCounterLabelledMetric(labelNames.length);
           }
+          return NoOpMetricsSystem.getCounterLabelledMetric(labelNames.length);
         });
   }
 
@@ -136,16 +134,15 @@ public LabelledMetric<OperationTimer> createLabelledTimer(
       final String help,
       final String... labelNames) {
     return cachedTimers.computeIfAbsent(
-        name,
-        (k) -> {
+        CachedMetricKey.of(category, name),
+        k -> {
           if (timersEnabled && isCategoryEnabled(category)) {
             final var summary =
                 new PrometheusTimer(category, name, help, DEFAULT_SUMMARY_QUANTILES, labelNames);
             registerCollector(category, summary);
             return summary;
-          } else {
-            return NoOpMetricsSystem.getOperationTimerLabelledMetric(labelNames.length);
           }
+          return NoOpMetricsSystem.getOperationTimerLabelledMetric(labelNames.length);
         });
   }
 
@@ -156,32 +153,18 @@ public LabelledMetric<OperationTimer> createSimpleLabelledTimer(
       final String help,
       final String... labelNames) {
     return cachedTimers.computeIfAbsent(
-        name,
-        (k) -> {
+        CachedMetricKey.of(category, name),
+        k -> {
           if (timersEnabled && isCategoryEnabled(category)) {
             final var histogram =
                 new PrometheusSimpleTimer(category, name, help, new double[] {1D}, labelNames);
             registerCollector(category, histogram);
             return histogram;
-          } else {
-            return NoOpMetricsSystem.getOperationTimerLabelledMetric(labelNames.length);
           }
+          return NoOpMetricsSystem.getOperationTimerLabelledMetric(labelNames.length);
         });
   }
 
-  @Override
-  public void createGauge(
-      final MetricCategory category,
-      final String name,
-      final String help,
-      final DoubleSupplier valueSupplier) {
-    if (isCategoryEnabled(category)) {
-      final var gauge = new PrometheusSuppliedGauge(category, name, help);
-      gauge.labels(valueSupplier);
-      registerCollector(category, gauge);
-    }
-  }
-
   @Override
   public void trackExternalSummary(
       final MetricCategory category,
@@ -278,4 +261,10 @@ public void shutdown() {
     cachedTimers.clear();
     guavaCacheCollectorContext.clear();
   }
+
+  private record CachedMetricKey(MetricCategory category, String name) {
+    static CachedMetricKey of(final MetricCategory category, final String name) {
+      return new CachedMetricKey(category, name);
+    }
+  }
 }
diff --git a/...core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedCounter.java b/...core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedCounter.java
@@ -27,7 +27,7 @@
 import io.prometheus.metrics.model.snapshots.DataPointSnapshot;
 
 /** The Prometheus supplied counter. */
-public class PrometheusSuppliedCounter extends PrometheusSuppliedValueCollector {
+public class PrometheusSuppliedCounter extends AbstractPrometheusSuppliedValueCollector {
 
   /**
    * Instantiates a new labelled Prometheus supplied counter.

diff --git a/...s/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedGauge.java b/...s/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedGauge.java
@@ -29,7 +29,7 @@
 
 /** The Prometheus gauge. */
 @SuppressWarnings("removal")
-public class PrometheusSuppliedGauge extends PrometheusSuppliedValueCollector
+public class PrometheusSuppliedGauge extends AbstractPrometheusSuppliedValueCollector
     implements LabelledGauge {
 
   /**

diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle
@@ -71,7 +71,7 @@ Calculated : ${currentHash}
 tasks.register('checkAPIChanges', FileStateChecker) {
   description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
   files = sourceSets.main.allJava.files
-  knownHash = 'vI9RTAGD6sRzAaMp1BMJY7x4cB3wh7FA4rYvDGYRceg='
+  knownHash = '8X9cB9p6OUuOyCMC2ezwb6kJwWfz0/5vk/UXqSyad8o='
 }
 check.dependsOn('checkAPIChanges')
 

diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java
@@ -184,7 +184,13 @@ LabelledMetric<OperationTimer> createSimpleLabelledTimer(
    * @param help A human readable description of the metric.
    * @param valueSupplier A supplier for the double value to be presented.
    */
-  void createGauge(MetricCategory category, String name, String help, DoubleSupplier valueSupplier);
+  default void createGauge(
+      final MetricCategory category,
+      final String name,
+      final String help,
+      final DoubleSupplier valueSupplier) {
+    createLabelledSuppliedGauge(category, name, help).labels(valueSupplier);
+  }
 
   /**
    * Creates a gauge for displaying integer values.