From 2b11b167c8d7633acaacc0c6aca6b829e8f4df6d Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Wed, 17 Oct 2018 14:15:48 -0700 Subject: Plugs-in the LongGauge into the registry (#1498) * plug-in longGauge into MetricRegistry * Minor fix * Add TODO and Fix build * Fix review comments --- .../implcore/metrics/MetricRegistryImpl.java | 91 +++---- .../implcore/metrics/MetricRegistryImplTest.java | 286 +++------------------ 2 files changed, 76 insertions(+), 301 deletions(-) (limited to 'impl_core/src') 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 3a30fd38..8f8dab2b 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 @@ -19,12 +19,9 @@ package io.opencensus.implcore.metrics; import static com.google.common.base.Preconditions.checkNotNull; import io.opencensus.common.Clock; -import io.opencensus.common.ToDoubleFunction; -import io.opencensus.common.ToLongFunction; -import io.opencensus.implcore.metrics.Gauge.DoubleGauge; -import io.opencensus.implcore.metrics.Gauge.LongGauge; +import io.opencensus.implcore.internal.Utils; import io.opencensus.metrics.LabelKey; -import io.opencensus.metrics.LabelValue; +import io.opencensus.metrics.LongGauge; import io.opencensus.metrics.MetricRegistry; import io.opencensus.metrics.export.Metric; import io.opencensus.metrics.export.MetricProducer; @@ -32,8 +29,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.Set; +import java.util.List; +import java.util.Map; /** Implementation of {@link MetricRegistry}. */ public final class MetricRegistryImpl extends MetricRegistry { @@ -46,56 +43,38 @@ public final class MetricRegistryImpl extends MetricRegistry { } @Override - public void addLongGauge( - String name, - String description, - String unit, - LinkedHashMap labels, - T obj, - ToLongFunction function) { - checkNotNull(labels, "labels"); - registeredMeters.registerMeter( - new LongGauge( + public LongGauge addLongGauge( + String name, String description, String unit, List labelKeys) { + Utils.checkListElementNotNull( + checkNotNull(labelKeys, "labelKeys"), "labelKey element should not be null."); + LongGaugeImpl longGaugeMetric = + new LongGaugeImpl( checkNotNull(name, "name"), checkNotNull(description, "description"), checkNotNull(unit, "unit"), - Collections.unmodifiableList(new ArrayList(labels.keySet())), - Collections.unmodifiableList(new ArrayList(labels.values())), - obj, - checkNotNull(function, "function"))); - } - - @Override - public void addDoubleGauge( - String name, - String description, - String unit, - LinkedHashMap labels, - T obj, - ToDoubleFunction function) { - checkNotNull(labels, "labels"); - registeredMeters.registerMeter( - new DoubleGauge( - checkNotNull(name, "name"), - checkNotNull(description, "description"), - checkNotNull(unit, "unit"), - Collections.unmodifiableList(new ArrayList(labels.keySet())), - Collections.unmodifiableList(new ArrayList(labels.values())), - obj, - checkNotNull(function, "function"))); + Collections.unmodifiableList(new ArrayList(labelKeys))); + registeredMeters.registerMeter(name, longGaugeMetric); + return longGaugeMetric; } private static final class RegisteredMeters { - private volatile Set registeredGauges = Collections.emptySet(); + private volatile Map registeredMeters = Collections.emptyMap(); - private Set getRegisteredMeters() { - return registeredGauges; + private Map getRegisteredMeters() { + return registeredMeters; } - private synchronized void registerMeter(Gauge gauge) { - Set newGaguesList = new LinkedHashSet(registeredGauges); - newGaguesList.add(gauge); - registeredGauges = Collections.unmodifiableSet(newGaguesList); + private synchronized void 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."); + } + + Map registeredMetersCopy = new LinkedHashMap(registeredMeters); + registeredMetersCopy.put(meterName, meter); + registeredMeters = Collections.unmodifiableMap(registeredMetersCopy); } } @@ -110,14 +89,18 @@ public final class MetricRegistryImpl extends MetricRegistry { @Override public Collection getMetrics() { - // Get a snapshot of the current registered gauges. - Set gaguges = registeredMeters.getRegisteredMeters(); - if (gaguges.isEmpty()) { + // Get a snapshot of the current registered meters. + Map meters = registeredMeters.getRegisteredMeters(); + if (meters.isEmpty()) { return Collections.emptyList(); } - ArrayList metrics = new ArrayList(); - for (Gauge gauge : gaguges) { - metrics.add(gauge.getMetric(clock)); + + List metrics = new ArrayList(meters.size()); + for (Map.Entry entry : meters.entrySet()) { + Metric metric = entry.getValue().getMetric(clock); + if (metric != null) { + metrics.add(metric); + } } return metrics; } 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 5210b266..c7cd99be 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 @@ -19,10 +19,9 @@ package io.opencensus.implcore.metrics; import static com.google.common.truth.Truth.assertThat; import io.opencensus.common.Timestamp; -import io.opencensus.common.ToDoubleFunction; -import io.opencensus.common.ToLongFunction; import io.opencensus.metrics.LabelKey; import io.opencensus.metrics.LabelValue; +import io.opencensus.metrics.LongGauge; import io.opencensus.metrics.export.Metric; import io.opencensus.metrics.export.MetricDescriptor; import io.opencensus.metrics.export.MetricDescriptor.Type; @@ -30,9 +29,9 @@ import io.opencensus.metrics.export.Point; import io.opencensus.metrics.export.TimeSeries; import io.opencensus.metrics.export.Value; import io.opencensus.testing.common.TestClock; +import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashMap; -import org.junit.Before; +import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -42,289 +41,82 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link MetricRegistryImpl}. */ @RunWith(JUnit4.class) public class MetricRegistryImplTest { + @Rule public ExpectedException thrown = ExpectedException.none(); + private static final String NAME = "name"; private static final String DESCRIPTION = "description"; private static final String UNIT = "1"; - private static final LabelKey LABEL_KEY = LabelKey.create("key", "key description"); - private static final LabelValue LABEL_VALUES = LabelValue.create("value"); - private static final Timestamp TEST_TIME = Timestamp.create(1234, 123); - - @Rule public ExpectedException thrown = ExpectedException.none(); + private static final List LABEL_KEY = + Collections.singletonList(LabelKey.create("key", "key description")); + private static final List LABEL_VALUES = + Collections.singletonList(LabelValue.create("value")); + private static final Timestamp TEST_TIME = Timestamp.create(1234, 123); private final TestClock testClock = TestClock.create(TEST_TIME); private final MetricRegistryImpl metricRegistry = new MetricRegistryImpl(testClock); - private final LinkedHashMap labels = - new LinkedHashMap(); - - @Before - public void setUp() { - labels.put(LABEL_KEY, LABEL_VALUES); - } - @Test - public void addDoubleGauge_NullName() { - thrown.expect(NullPointerException.class); - metricRegistry.addDoubleGauge( - null, - DESCRIPTION, - UNIT, - labels, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - } - - @Test - public void addDoubleGauge_NullDescription() { - thrown.expect(NullPointerException.class); - metricRegistry.addDoubleGauge( - NAME, - null, - UNIT, - labels, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - } - - @Test - public void addDoubleGauge_NullUnit() { - thrown.expect(NullPointerException.class); - metricRegistry.addDoubleGauge( - NAME, - DESCRIPTION, - null, - labels, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - } - - @Test - public void addDoubleGauge_NullLabels() { - thrown.expect(NullPointerException.class); - metricRegistry.addDoubleGauge( - NAME, - DESCRIPTION, - UNIT, - null, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - } - - @Test - public void addDoubleGauge_NullFunction() { - thrown.expect(NullPointerException.class); - metricRegistry.addDoubleGauge(NAME, DESCRIPTION, UNIT, labels, null, null); - } - - @Test - public void addDoubleGauge_GetMetrics() { - metricRegistry.addDoubleGauge( - NAME, - DESCRIPTION, - UNIT, - labels, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - assertThat(metricRegistry.getMetricProducer().getMetrics()) - .containsExactly( - Metric.create( - MetricDescriptor.create( - NAME, - DESCRIPTION, - UNIT, - Type.GAUGE_DOUBLE, - Collections.unmodifiableList(Collections.singletonList(LABEL_KEY))), - Collections.singletonList( - TimeSeries.createWithOnePoint( - Collections.unmodifiableList(Collections.singletonList(LABEL_VALUES)), - Point.create(Value.doubleValue(5.0), TEST_TIME), - null)))); - } + private static final MetricDescriptor METRIC_DESCRIPTOR = + MetricDescriptor.create(NAME, DESCRIPTION, UNIT, Type.GAUGE_INT64, LABEL_KEY); @Test public void addLongGauge_NullName() { thrown.expect(NullPointerException.class); - metricRegistry.addLongGauge( - null, - DESCRIPTION, - UNIT, - labels, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 7; - } - }); + thrown.expectMessage("name"); + metricRegistry.addLongGauge(null, DESCRIPTION, UNIT, LABEL_KEY); } @Test public void addLongGauge_NullDescription() { thrown.expect(NullPointerException.class); - metricRegistry.addLongGauge( - NAME, - null, - UNIT, - labels, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 5; - } - }); + thrown.expectMessage("description"); + metricRegistry.addLongGauge(NAME, null, UNIT, LABEL_KEY); } @Test public void addLongGauge_NullUnit() { thrown.expect(NullPointerException.class); - metricRegistry.addLongGauge( - NAME, - DESCRIPTION, - null, - labels, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 5; - } - }); + thrown.expectMessage("unit"); + metricRegistry.addLongGauge(NAME, DESCRIPTION, null, LABEL_KEY); } @Test public void addLongGauge_NullLabels() { thrown.expect(NullPointerException.class); - metricRegistry.addLongGauge( - NAME, - DESCRIPTION, - UNIT, - null, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 5; - } - }); + thrown.expectMessage("labelKeys"); + metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, null); } @Test - public void addLongGauge_NullFunction() { + public void addLongGauge_WithNullElement() { + List labelKeys = Collections.singletonList(null); thrown.expect(NullPointerException.class); - metricRegistry.addLongGauge("name", DESCRIPTION, UNIT, labels, null, null); + thrown.expectMessage("labelKey element should not be null."); + metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, labelKeys); } @Test public void addLongGauge_GetMetrics() { - metricRegistry.addLongGauge( - NAME, - DESCRIPTION, - UNIT, - labels, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 7; - } - }); - assertThat(metricRegistry.getMetricProducer().getMetrics()) - .containsExactly( - Metric.create( - MetricDescriptor.create( - NAME, - DESCRIPTION, - UNIT, - Type.GAUGE_INT64, - Collections.unmodifiableList(Collections.singletonList(LABEL_KEY))), - Collections.singletonList( - TimeSeries.createWithOnePoint( - Collections.unmodifiableList(Collections.singletonList(LABEL_VALUES)), - Point.create(Value.longValue(7), TEST_TIME), - null)))); - } + LongGauge longGauge = metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, LABEL_KEY); + longGauge.getOrCreateTimeSeries(LABEL_VALUES); - @Test - public void multipleMetrics_GetMetrics() { - metricRegistry.addLongGauge( - NAME, - DESCRIPTION, - UNIT, - labels, - null, - new ToLongFunction() { - @Override - public long applyAsLong(Object value) { - return 7; - } - }); - metricRegistry.addDoubleGauge( - NAME, - DESCRIPTION, - UNIT, - labels, - null, - new ToDoubleFunction() { - @Override - public double applyAsDouble(Object value) { - return 5.0; - } - }); - assertThat(metricRegistry.getMetricProducer().getMetrics()) + Collection metricCollections = metricRegistry.getMetricProducer().getMetrics(); + assertThat(metricCollections.size()).isEqualTo(1); + assertThat(metricCollections) .containsExactly( - Metric.create( - MetricDescriptor.create( - NAME, - DESCRIPTION, - UNIT, - Type.GAUGE_INT64, - Collections.unmodifiableList(Collections.singletonList(LABEL_KEY))), - Collections.singletonList( - TimeSeries.createWithOnePoint( - Collections.unmodifiableList(Collections.singletonList(LABEL_VALUES)), - Point.create(Value.longValue(7), TEST_TIME), - null))), - Metric.create( - MetricDescriptor.create( - NAME, - DESCRIPTION, - UNIT, - Type.GAUGE_DOUBLE, - Collections.unmodifiableList(Collections.singletonList(LABEL_KEY))), - Collections.singletonList( - TimeSeries.createWithOnePoint( - Collections.unmodifiableList(Collections.singletonList(LABEL_VALUES)), - Point.create(Value.doubleValue(5.0), TEST_TIME), - null)))); + Metric.createWithOneTimeSeries( + METRIC_DESCRIPTOR, + TimeSeries.createWithOnePoint( + LABEL_VALUES, Point.create(Value.longValue(0), TEST_TIME), null))); } @Test public void empty_GetMetrics() { assertThat(metricRegistry.getMetricProducer().getMetrics()).isEmpty(); } + + @Test + public void checkInstanceOf() { + LongGauge longGauge = metricRegistry.addLongGauge(NAME, DESCRIPTION, UNIT, LABEL_KEY); + assertThat(longGauge).isInstanceOf(LongGaugeImpl.class); + } } -- cgit v1.2.3