prometheus · brian-brazil · Nov 25, 2016 · Nov 25, 2016
diff --git a/simpleclient/src/main/java/io/prometheus/client/Collector.java b/simpleclient/src/main/java/io/prometheus/client/Collector.java
@@ -129,6 +129,25 @@ public <T extends Collector> T register(CollectorRegistry registry) {
     return (T)this;
   }
 
+  public interface Describable {
+    /**
+     *  Provide a list of metric families this Collector is expected to return.
+     *
+     *  These should exclude the samples. This is used by the registry to
+     *  detect collisions and duplicate registrations.
+     *
+     *  Usually custom collectors do not have to implement Describable. If
+     *  Describable is not implemented and the CollectorRegistry was created
+     *  with auto desribe enabled (which is the case for the default registry)
+     *  then {@link collect} will be called at registration time instead of
+     *  describe. If this could cause problems, either implement a proper
+     *  describe, or if that's not practical have describe return an empty
+     *  list.
+     */
+    public List<MetricFamilySamples> describe();
+  }
+
+
   /* Various utility functions for implementing Collectors. */
 
   /**

diff --git a/simpleclient/src/main/java/io/prometheus/client/CollectorRegistry.java b/simpleclient/src/main/java/io/prometheus/client/CollectorRegistry.java
@@ -1,12 +1,16 @@
 package io.prometheus.client;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
+import java.util.List;
 
 /**
  * A registry of Collectors.
@@ -21,31 +25,98 @@ public class CollectorRegistry {
   /**
    * The default registry.
    */
-  public static final CollectorRegistry defaultRegistry = new CollectorRegistry();
+  public static final CollectorRegistry defaultRegistry = new CollectorRegistry(true);
 
-  private final Set<Collector> collectors = 
-      Collections.newSetFromMap(new ConcurrentHashMap<Collector, Boolean>());
+
+  private final Map<Collector, List<String>> collectorsToNames = new HashMap<Collector, List<String>>();
+  private final Map<String, Collector> namesToCollectors = new HashMap<String, Collector>();
+
+  private final boolean autoDescribe;
+
+  public CollectorRegistry(){
+    this(false);
+  }
+
+  public CollectorRegistry(boolean autoDescribe) {
+    this.autoDescribe = autoDescribe;
+  }
 
   /**
    * Register a Collector.
    * <p>
    * A collector can be registered to multiple CollectorRegistries.
    */
   public void register(Collector m) {
-    collectors.add(m);
+    List<String> names = collectorNames(m);
+    synchronized (collectorsToNames) {
+      for (String name : names) {
+        if(namesToCollectors.containsKey(name)) {
+          throw new IllegalArgumentException("Collector already registered that provides name: " + name);
+        }
+      }
+      for (String name : names) {
+        namesToCollectors.put(name, m);
+      }
+      collectorsToNames.put(m, names);
+    }
   }
-  
+
   /**
    * Unregister a Collector.
    */
   public void unregister(Collector m) {
-    collectors.remove(m);
+    synchronized (collectorsToNames) {
+      for (String name : collectorsToNames.get(m)) {
+        namesToCollectors.remove(name);
+      }
+      collectorsToNames.remove(m);
+    }
   }
   /**
    * Unregister all Collectors.
    */
   public void clear() {
-    collectors.clear();
+    synchronized (collectorsToNames) {
+      collectorsToNames.clear();
+      namesToCollectors.clear();
+    }
+  }
+
+  /**
+   * A snapshot of the current collectors.
+   */
+  private Set<Collector> collectors() {
+    synchronized (collectorsToNames) {
+      return new HashSet(collectorsToNames.keySet());
+    }
+  }
+
+  private List<String> collectorNames(Collector m) {
+    List<Collector.MetricFamilySamples> mfs;
+    if (m instanceof Collector.Describable) {
+      mfs = ((Collector.Describable)m).describe();
+    } else if (autoDescribe) {
+      mfs = m.collect();
+    } else {
+      mfs = Collections.emptyList();
+    }
+
+    List<String> names = new ArrayList<String>();
+    for (Collector.MetricFamilySamples family : mfs) {
+      switch (family.type) {
+        case SUMMARY:
+          names.add(family.name + "_count");
+          names.add(family.name + "_sum");
+          names.add(family.name);
+        case HISTOGRAM:
+          names.add(family.name + "_count");
+          names.add(family.name + "_sum");
+          names.add(family.name + "_bucket");
+        default:
+          names.add(family.name);
+      }
+    }
+    return names;
   }
 
   /**
@@ -56,14 +127,14 @@ public Enumeration<Collector.MetricFamilySamples> metricFamilySamples() {
   }
   class MetricFamilySamplesEnumeration implements Enumeration<Collector.MetricFamilySamples> {
 
-    private final Iterator<Collector> collectorIter = collectors.iterator();
+    private final Iterator<Collector> collectorIter = collectors().iterator();
     private Iterator<Collector.MetricFamilySamples> metricFamilySamples;
     private Collector.MetricFamilySamples next;
 
     MetricFamilySamplesEnumeration() {
       findNextElement();
     }
-    
+
     private void findNextElement() {
       if (metricFamilySamples != null && metricFamilySamples.hasNext()) {
         next = metricFamilySamples.next();
@@ -87,7 +158,7 @@ public Collector.MetricFamilySamples nextElement() {
       findNextElement();
       return current;
     }
-    
+
     public boolean hasMoreElements() {
       return next != null;
     }

diff --git a/simpleclient/src/main/java/io/prometheus/client/Counter.java b/simpleclient/src/main/java/io/prometheus/client/Counter.java
@@ -64,7 +64,7 @@
  * These can be aggregated and processed together much more easily in the Promtheus 
  * server than individual metrics for each labelset.
  */
-public class Counter extends SimpleCollector<Counter.Child> {
+public class Counter extends SimpleCollector<Counter.Child> implements Collector.Describable {
 
   Counter(Builder b) {
     super(b);
@@ -148,4 +148,10 @@ public List<MetricFamilySamples> collect() {
     mfsList.add(mfs);
     return mfsList;
   }
+
+  public List<MetricFamilySamples> describe() {
+    List<MetricFamilySamples> mfsList = new ArrayList<MetricFamilySamples>();
+    mfsList.add(new CounterMetricFamily(fullname, help, labelNames));
+    return mfsList;
+  }
 }
diff --git a/simpleclient/src/main/java/io/prometheus/client/Gauge.java b/simpleclient/src/main/java/io/prometheus/client/Gauge.java
@@ -62,7 +62,7 @@
  * These can be aggregated and processed together much more easily in the Prometheus
  * server than individual metrics for each labelset.
  */
-public class Gauge extends SimpleCollector<Gauge.Child> {
+public class Gauge extends SimpleCollector<Gauge.Child> implements Collector.Describable {
 
   Gauge(Builder b) {
     super(b);
@@ -254,6 +254,12 @@ public List<MetricFamilySamples> collect() {
     return mfsList;
   }
 
+  public List<MetricFamilySamples> describe() {
+    List<MetricFamilySamples> mfsList = new ArrayList<MetricFamilySamples>();
+    mfsList.add(new GaugeMetricFamily(fullname, help, labelNames));
+    return mfsList;
+  }
+
   static class TimeProvider {
     long currentTimeMillis() {
       return System.currentTimeMillis();

diff --git a/simpleclient/src/main/java/io/prometheus/client/Histogram.java b/simpleclient/src/main/java/io/prometheus/client/Histogram.java
@@ -52,7 +52,7 @@
  * {@link Histogram.Builder#exponentialBuckets(double, double, int) exponentialBuckets}
  * offer easy ways to set common bucket patterns.
  */
-public class Histogram extends SimpleCollector<Histogram.Child> {
+public class Histogram extends SimpleCollector<Histogram.Child> implements Collector.Describable {
   private final double[] buckets;
 
   Histogram(Builder b) {
@@ -269,6 +269,12 @@ public List<MetricFamilySamples> collect() {
     return mfsList;
   }
 
+  public List<MetricFamilySamples> describe() {
+    List<MetricFamilySamples> mfsList = new ArrayList<MetricFamilySamples>();
+    mfsList.add(new MetricFamilySamples(fullname, Type.HISTOGRAM, help, new ArrayList<MetricFamilySamples.Sample>()));
+    return mfsList;
+  }
+
   double[] getBuckets() {
     return buckets;
   }

diff --git a/simpleclient/src/main/java/io/prometheus/client/Summary.java b/simpleclient/src/main/java/io/prometheus/client/Summary.java
@@ -70,7 +70,7 @@
  *
  * See https://prometheus.io/docs/practices/histograms/ for more info on quantiles.
  */
-public class Summary extends SimpleCollector<Summary.Child> {
+public class Summary extends SimpleCollector<Summary.Child> implements Counter.Describable {
 
   final List<Quantile> quantiles; // Can be empty, but can never be null.
   final long maxAgeSeconds;
@@ -284,6 +284,12 @@ public List<MetricFamilySamples> collect() {
     return mfsList;
   }
 
+  public List<MetricFamilySamples> describe() {
+    List<MetricFamilySamples> mfsList = new ArrayList<MetricFamilySamples>();
+    mfsList.add(new SummaryMetricFamily(fullname, help, labelNames));
+    return mfsList;
+  }
+
   static class TimeProvider {
     long nanoTime() {
       return System.nanoTime();

diff --git a/simpleclient/src/test/java/io/prometheus/client/CollectorRegistryTest.java b/simpleclient/src/test/java/io/prometheus/client/CollectorRegistryTest.java
@@ -81,5 +81,60 @@ public void testRegistryWithEmptyCollectorHasNoMoreElements() {
     registry.register(new EmptyCollector());
     assertFalse(registry.metricFamilySamples().hasMoreElements());
   }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testCounterAndGaugeWithSameNameThrows() {
+    Gauge.build().name("g").help("h").register(registry);
+    Counter.build().name("g").help("h").register(registry);
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testCounterAndSummaryWithSameNameThrows() {
+    Counter.build().name("s").help("h").register(registry);
+    Summary.build().name("s").help("h").register(registry);
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testCounterSumAndSummaryWithSameNameThrows() {
+    Counter.build().name("s_sum").help("h").register(registry);
+    Summary.build().name("s").help("h").register(registry);
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testHistogramAndSummaryWithSameNameThrows() {
+    Histogram.build().name("s").help("h").register(registry);
+    Summary.build().name("s").help("h").register(registry);
+  }
+
+  @Test
+  public void testCanUnAndReregister() {
+    Histogram h = Histogram.build().name("s").help("h").create();
+    registry.register(h);
+    registry.unregister(h);
+    registry.register(h);
+  }
+
+  class MyCollector extends Collector {
+    public List<MetricFamilySamples> collect() {
+      List<MetricFamilySamples> mfs = new ArrayList<MetricFamilySamples>();
+      mfs.add(new GaugeMetricFamily("g", "help", 42));
+      return mfs;
+    }
+  }
+
+  @Test
+  public void testAutoDescribeDisabledByDefault() {
+    CollectorRegistry r = new CollectorRegistry();
+    new MyCollector().register(r);
+    // This doesn't throw.
+    new MyCollector().register(r);
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testAutoDescribeThrowsOnReregisteringCustomCollector() {
+    CollectorRegistry r = new CollectorRegistry(true);
+    new MyCollector().register(r);
+    new MyCollector().register(r);
+  }
 
 }
diff --git a/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/DropwizardExports.java b/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/DropwizardExports.java
@@ -13,7 +13,7 @@
 /**
  * Collect Dropwizard metrics from a MetricRegistry.
  */
-public class DropwizardExports extends io.prometheus.client.Collector {
+public class DropwizardExports extends io.prometheus.client.Collector implements io.prometheus.client.Collector.Describable {
     private MetricRegistry registry;
     private static final Logger LOGGER = Logger.getLogger(DropwizardExports.class.getName());
 
@@ -146,4 +146,8 @@ public List<MetricFamilySamples> collect() {
         }
         return mfSamples;
     }
+
+    public List<MetricFamilySamples> describe() {
+      return new ArrayList<MetricFamilySamples>();
+    }
 }
diff --git a/...pring_boot/src/main/java/io/prometheus/client/spring/boot/SpringBootMetricsCollector.java b/...pring_boot/src/main/java/io/prometheus/client/spring/boot/SpringBootMetricsCollector.java
@@ -23,7 +23,7 @@
  * </code></pre>
  */
 @Component
-public class SpringBootMetricsCollector extends Collector {
+public class SpringBootMetricsCollector extends Collector implements Collector.Describable {
   private final Collection<PublicMetrics> publicMetrics;
 
   @Autowired
@@ -46,4 +46,8 @@ public List<MetricFamilySamples> collect() {
     }
     return samples;
   }
+
+  public List<MetricFamilySamples> describe() {
+    return new ArrayList<MetricFamilySamples>();
+  }
 }