feat: Allow users to register the same Meter multiple times without exception (#2017)

mayurkale22 · web-flow · commit d2367b039ed3 · 2020-03-10T16:23:41.000-07:00
* feat: Allow register same metric multiple times

* add more test

* remove redundant test
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedDoubleCumulativeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedDoubleCumulativeImpl.java
@@ -127,6 +127,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointWithFunction<?>>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   @javax.annotation.Nullable
   @Override
   public Metric getMetric(Clock clock) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedDoubleGaugeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedDoubleGaugeImpl.java
@@ -121,6 +121,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointWithFunction<?>>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   @javax.annotation.Nullable
   @Override
   public Metric getMetric(Clock clock) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedLongCumulativeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedLongCumulativeImpl.java
@@ -127,6 +127,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointWithFunction<?>>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   @javax.annotation.Nullable
   @Override
   public Metric getMetric(Clock clock) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedLongGaugeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DerivedLongGaugeImpl.java
@@ -121,6 +121,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointWithFunction<?>>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   @javax.annotation.Nullable
   @Override
   public Metric getMetric(Clock clock) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DoubleCumulativeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DoubleCumulativeImpl.java
@@ -125,6 +125,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointImpl>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   private synchronized DoublePoint registerTimeSeries(List<LabelValue> labelValues) {
     PointImpl existingPoint = registeredPoints.get(labelValues);
     if (existingPoint != null) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/DoubleGaugeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/DoubleGaugeImpl.java
@@ -121,6 +121,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointImpl>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   private synchronized DoublePoint registerTimeSeries(List<LabelValue> labelValues) {
     PointImpl existingPoint = registeredPoints.get(labelValues);
     if (existingPoint != null) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/LongCumulativeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/LongCumulativeImpl.java
@@ -125,6 +125,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointImpl>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   private synchronized LongPoint registerTimeSeries(List<LabelValue> labelValues) {
     PointImpl existingPoint = registeredPoints.get(labelValues);
     if (existingPoint != null) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/LongGaugeImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/LongGaugeImpl.java
@@ -121,6 +121,11 @@ public synchronized void clear() {
     registeredPoints = Collections.<List<LabelValue>, PointImpl>emptyMap();
   }
 
+  @Override
+  public MetricDescriptor getMetricDescriptor() {
+    return metricDescriptor;
+  }
+
   private synchronized LongPoint registerTimeSeries(List<LabelValue> labelValues) {
     PointImpl existingPoint = registeredPoints.get(labelValues);
     if (existingPoint != null) {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/Meter.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/Meter.java
@@ -18,6 +18,7 @@
 
 import io.opencensus.common.Clock;
 import io.opencensus.metrics.export.Metric;
+import io.opencensus.metrics.export.MetricDescriptor;
 import javax.annotation.Nullable;
 
 interface Meter {
@@ -31,4 +32,11 @@ interface Meter {
    */
   @Nullable
   Metric getMetric(Clock clock);
+
+  /**
+   * Provides a {@link io.opencensus.metrics.export.MetricDescriptor}.
+   *
+   * @return a {@code MetricDescriptor}.
+   */
+  MetricDescriptor getMetricDescriptor();
 }
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/MetricRegistryImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/MetricRegistryImpl.java
@@ -59,8 +59,7 @@ public LongGauge addLongGauge(String name, MetricOptions options) {
             options.getUnit(),
             options.getLabelKeys(),
             options.getConstantLabels());
-    registeredMeters.registerMeter(name, longGaugeMetric);
-    return longGaugeMetric;
+    return (LongGauge) registeredMeters.registerMeter(name, longGaugeMetric);
   }
 
   @Override
@@ -72,8 +71,7 @@ public DoubleGauge addDoubleGauge(String name, MetricOptions options) {
             options.getUnit(),
             options.getLabelKeys(),
             options.getConstantLabels());
-    registeredMeters.registerMeter(name, doubleGaugeMetric);
-    return doubleGaugeMetric;
+    return (DoubleGauge) registeredMeters.registerMeter(name, doubleGaugeMetric);
   }
 
   @Override
@@ -85,8 +83,7 @@ public DerivedLongGauge addDerivedLongGauge(String name, MetricOptions options)
             options.getUnit(),
             options.getLabelKeys(),
             options.getConstantLabels());
-    registeredMeters.registerMeter(name, derivedLongGauge);
-    return derivedLongGauge;
+    return (DerivedLongGauge) registeredMeters.registerMeter(name, derivedLongGauge);
   }
 
   @Override
@@ -98,8 +95,7 @@ public DerivedDoubleGauge addDerivedDoubleGauge(String name, MetricOptions optio
             options.getUnit(),
             options.getLabelKeys(),
             options.getConstantLabels());
-    registeredMeters.registerMeter(name, derivedDoubleGauge);
-    return derivedDoubleGauge;
+    return (DerivedDoubleGauge) registeredMeters.registerMeter(name, derivedDoubleGauge);
   }
 
   @Override
@@ -112,8 +108,7 @@ public LongCumulative addLongCumulative(String name, MetricOptions options) {
             options.getLabelKeys(),
             options.getConstantLabels(),
             clock.now());
-    registeredMeters.registerMeter(name, longCumulativeMetric);
-    return longCumulativeMetric;
+    return (LongCumulative) registeredMeters.registerMeter(name, longCumulativeMetric);
   }
 
   @Override
@@ -126,8 +121,7 @@ public DoubleCumulative addDoubleCumulative(String name, MetricOptions options)
             options.getLabelKeys(),
             options.getConstantLabels(),
             clock.now());
-    registeredMeters.registerMeter(name, longCumulativeMetric);
-    return longCumulativeMetric;
+    return (DoubleCumulative) registeredMeters.registerMeter(name, longCumulativeMetric);
   }
 
   @Override
@@ -140,8 +134,7 @@ public DerivedLongCumulative addDerivedLongCumulative(String name, MetricOptions
             options.getLabelKeys(),
             options.getConstantLabels(),
             clock.now());
-    registeredMeters.registerMeter(name, derivedLongCumulative);
-    return derivedLongCumulative;
+    return (DerivedLongCumulative) registeredMeters.registerMeter(name, derivedLongCumulative);
   }
 
   @Override
@@ -154,8 +147,7 @@ public DerivedDoubleCumulative addDerivedDoubleCumulative(String name, MetricOpt
             options.getLabelKeys(),
             options.getConstantLabels(),
             clock.now());
-    registeredMeters.registerMeter(name, derivedDoubleCumulative);
-    return derivedDoubleCumulative;
+    return (DerivedDoubleCumulative) registeredMeters.registerMeter(name, derivedDoubleCumulative);
   }
 
   private static final class RegisteredMeters {
@@ -165,17 +157,21 @@ private Map<String, Meter> getRegisteredMeters() {
       return registeredMeters;
     }
 
-    private synchronized void registerMeter(String meterName, Meter meter) {
+    private synchronized Meter registerMeter(String meterName, Meter meter) {
       Meter existingMeter = registeredMeters.get(meterName);
       if (existingMeter != null) {
-        // TODO(mayurkale): Allow users to register the same Meter multiple times without exception.
-        throw new IllegalArgumentException(
-            "A different metric with the same name already registered.");
+        if (!existingMeter.getMetricDescriptor().equals(meter.getMetricDescriptor())) {
+          throw new IllegalArgumentException(
+              "A different metric with the same name already registered.");
+        } else {
+          return existingMeter;
+        }
       }
 
       Map<String, Meter> registeredMetersCopy = new LinkedHashMap<String, Meter>(registeredMeters);
       registeredMetersCopy.put(meterName, meter);
       registeredMeters = Collections.unmodifiableMap(registeredMetersCopy);
+      return meter;
     }
   }
 
diff --git a/impl_core/src/test/java/io/opencensus/implcore/metrics/MetricRegistryImplTest.java b/impl_core/src/test/java/io/opencensus/implcore/metrics/MetricRegistryImplTest.java
@@ -244,10 +244,47 @@ public void getMetrics() {
   }
 
   @Test
-  public void registerDifferentMetricSameName() {
+  public void shouldReturnSameObjectOnMultipleRegisterCall() {
+    LongGauge longGauge = metricRegistry.addLongGauge(NAME, METRIC_OPTIONS);
+    LongPoint longPoint = longGauge.getOrCreateTimeSeries(LABEL_VALUES);
+    longPoint.set(200);
+
+    LongGauge longGauge1 = metricRegistry.addLongGauge(NAME, METRIC_OPTIONS);
+    LongPoint longPoint1 =
+        longGauge1.getOrCreateTimeSeries(Collections.singletonList(LABEL_VALUE_2));
+    longPoint1.set(300);
+
+    assertThat(longGauge).isEqualTo(longGauge1);
+    Collection<Metric> metricCollections = metricRegistry.getMetricProducer().getMetrics();
+    assertThat(metricCollections.size()).isEqualTo(1);
+    assertThat(metricCollections)
+        .containsExactly(
+            Metric.create(
+                LONG_METRIC_DESCRIPTOR,
+                Arrays.asList(
+                    TimeSeries.createWithOnePoint(
+                        Arrays.asList(LABEL_VALUE, LABEL_VALUE_2),
+                        Point.create(Value.longValue(200), TEST_TIME),
+                        null),
+                    TimeSeries.createWithOnePoint(
+                        Arrays.asList(LABEL_VALUE_2, LABEL_VALUE_2),
+                        Point.create(Value.longValue(300), TEST_TIME),
+                        null))));
+  }
+
+  @Test
+  public void shouldThrowWhenRegisterExistingMetricWithDiffType() {
     metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, LABEL_KEYS);
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("A different metric with the same name already registered.");
     metricRegistry.addDoubleGauge(NAME, DESCRIPTION, UNIT, LABEL_KEYS);
   }
+
+  @Test
+  public void shouldThrowWhenRegisterExistingMetricWithDiffDesc() {
+    metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, LABEL_KEYS);
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("A different metric with the same name already registered.");
+    metricRegistry.addLongGauge(NAME, "duplicate", UNIT, LABEL_KEYS);
+  }
 }
