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 index 441afa1518..3d0966f325 100644 --- 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., 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 index 58b772703e..3c63a60ceb 100644 --- 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., 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 index 28134fb78d..08e627ad69 100644 --- 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., 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 index fa4562e96a..8a27c59ffa 100644 --- 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., 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 index 3317844289..5f4f7c6464 100644 --- 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., PointImpl>emptyMap(); } + @Override + public MetricDescriptor getMetricDescriptor() { + return metricDescriptor; + } + private synchronized DoublePoint registerTimeSeries(List 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 index fc60abda49..87f17ab296 100644 --- 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., PointImpl>emptyMap(); } + @Override + public MetricDescriptor getMetricDescriptor() { + return metricDescriptor; + } + private synchronized DoublePoint registerTimeSeries(List 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 index 503d037113..5b7ea4a01f 100644 --- 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., PointImpl>emptyMap(); } + @Override + public MetricDescriptor getMetricDescriptor() { + return metricDescriptor; + } + private synchronized LongPoint registerTimeSeries(List 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 index c6045d79a7..7d64953425 100644 --- 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., PointImpl>emptyMap(); } + @Override + public MetricDescriptor getMetricDescriptor() { + return metricDescriptor; + } + private synchronized LongPoint registerTimeSeries(List 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 index f5a8dc8f61..8a256a3987 100644 --- 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 index 5e193a2189..862cfe9cc8 100644 --- 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 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 registeredMetersCopy = new LinkedHashMap(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 index 005194a783..ff9c5c806c 100644 --- 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 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); + } }