diff --git a/java/google/registry/monitoring/metrics/AbstractMetric.java b/java/google/registry/monitoring/metrics/AbstractMetric.java index 58b33d7dd..bb508f2fd 100644 --- a/java/google/registry/monitoring/metrics/AbstractMetric.java +++ b/java/google/registry/monitoring/metrics/AbstractMetric.java @@ -14,6 +14,7 @@ package google.registry.monitoring.metrics; + import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import google.registry.monitoring.metrics.MetricSchema.Kind; diff --git a/java/google/registry/monitoring/metrics/Counter.java b/java/google/registry/monitoring/metrics/Counter.java index 45b6f50bb..26ca9a578 100644 --- a/java/google/registry/monitoring/metrics/Counter.java +++ b/java/google/registry/monitoring/metrics/Counter.java @@ -50,10 +50,6 @@ public final class Counter extends AbstractMetric private static final float HASHMAP_LOAD_FACTOR = 0.75f; private static final int HASHMAP_CONCURRENCY_LEVEL = 16; - private static final String LABEL_COUNT_ERROR = - "The count of labelValues must be equal to the underlying " - + "MetricDescriptor's count of labels."; - /** * A map of the {@link Counter} values, with a list of label values as the keys. * @@ -89,13 +85,13 @@ public final class Counter extends AbstractMetric } @VisibleForTesting - void incrementBy(long offset, Instant startTime, ImmutableList labelValues) { + void incrementBy(long offset, Instant startTimestamp, ImmutableList labelValues) { Lock lock = valueLocks.get(labelValues); lock.lock(); try { values.addAndGet(labelValues, offset); - valueStartTimestamps.putIfAbsent(labelValues, startTime); + valueStartTimestamps.putIfAbsent(labelValues, startTimestamp); } finally { lock.unlock(); } @@ -103,7 +99,7 @@ public final class Counter extends AbstractMetric @Override public final void incrementBy(long offset, String... labelValues) { - checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(this, labelValues); checkArgument(offset >= 0, "The offset provided must be non-negative"); incrementBy(offset, Instant.now(), ImmutableList.copyOf(labelValues)); @@ -111,7 +107,7 @@ public final class Counter extends AbstractMetric @Override public final void increment(String... labelValues) { - checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(this, labelValues); incrementBy(1L, Instant.now(), ImmutableList.copyOf(labelValues)); } @@ -152,13 +148,13 @@ public final class Counter extends AbstractMetric } @VisibleForTesting - final void set(Long value, Instant startTime, ImmutableList labelValues) { + final void set(Long value, Instant startTimestamp, ImmutableList labelValues) { Lock lock = valueLocks.get(labelValues); lock.lock(); try { this.values.put(labelValues, value); - valueStartTimestamps.putIfAbsent(labelValues, startTime); + valueStartTimestamps.putIfAbsent(labelValues, startTimestamp); } finally { lock.unlock(); } @@ -166,13 +162,13 @@ public final class Counter extends AbstractMetric @Override public final void set(Long value, String... labelValues) { - checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(this, labelValues); set(value, Instant.now(), ImmutableList.copyOf(labelValues)); } @VisibleForTesting - final void reset(Instant startTime) { + final void reset(Instant startTimestamp) { // Lock the entire set of values so that all existing values will have a consistent timestamp // after this call, without the possibility of interleaving with another reset() call. Set> keys = values.asMap().keySet(); @@ -182,7 +178,7 @@ public final class Counter extends AbstractMetric for (ImmutableList labelValues : keys) { this.values.put(labelValues, 0); - this.valueStartTimestamps.put(labelValues, startTime); + this.valueStartTimestamps.put(labelValues, startTimestamp); } for (int i = 0; i < valueLocks.size(); i++) { @@ -196,13 +192,13 @@ public final class Counter extends AbstractMetric } @VisibleForTesting - final void reset(Instant startTime, ImmutableList labelValues) { + final void reset(Instant startTimestamp, ImmutableList labelValues) { Lock lock = valueLocks.get(labelValues); lock.lock(); try { this.values.put(labelValues, 0); - this.valueStartTimestamps.put(labelValues, startTime); + this.valueStartTimestamps.put(labelValues, startTimestamp); } finally { lock.unlock(); } @@ -210,7 +206,7 @@ public final class Counter extends AbstractMetric @Override public final void reset(String... labelValues) { - checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(this, labelValues); reset(Instant.now(), ImmutableList.copyOf(labelValues)); } diff --git a/java/google/registry/monitoring/metrics/MetricPoint.java b/java/google/registry/monitoring/metrics/MetricPoint.java index 1a3c3ce53..2f427cd59 100644 --- a/java/google/registry/monitoring/metrics/MetricPoint.java +++ b/java/google/registry/monitoring/metrics/MetricPoint.java @@ -14,8 +14,6 @@ package google.registry.monitoring.metrics; -import static com.google.common.base.Preconditions.checkArgument; - import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import org.joda.time.Instant; @@ -28,12 +26,6 @@ import org.joda.time.Interval; @AutoValue public abstract class MetricPoint { - private static final String LABEL_COUNT_ERROR = - "The count of labelsValues must be equal to the underlying " - + "MetricDescriptor's count of labels."; - - MetricPoint() {} - /** * Returns a new {@link MetricPoint} representing a value at a specific {@link Instant}. * @@ -42,8 +34,8 @@ public abstract class MetricPoint { */ static MetricPoint create( Metric metric, ImmutableList labelValues, Instant timestamp, V value) { - checkArgument( - labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(metric, labelValues); + return new AutoValue_MetricPoint<>( metric, labelValues, new Interval(timestamp, timestamp), value); } @@ -61,8 +53,8 @@ public abstract class MetricPoint { Instant startTime, Instant endTime, V value) { - checkArgument( - labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(metric, labelValues); + return new AutoValue_MetricPoint<>( metric, labelValues, new Interval(startTime, endTime), value); } diff --git a/java/google/registry/monitoring/metrics/MetricsUtils.java b/java/google/registry/monitoring/metrics/MetricsUtils.java new file mode 100644 index 000000000..7e619e688 --- /dev/null +++ b/java/google/registry/monitoring/metrics/MetricsUtils.java @@ -0,0 +1,48 @@ +// Copyright 2016 The Domain Registry Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.monitoring.metrics; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.collect.ImmutableList; + +/** Static helper methods for the Metrics library. */ +final class MetricsUtils { + + private static final String LABEL_SIZE_ERROR = + "The count of labelValues must be equal to the underlying Metric's count of labels."; + + private MetricsUtils() {} + + /** + * Check that the given {@code labelValues} match in count with the count of {@link + * LabelDescriptor} instances on the given {@code metric} + * + * @throws IllegalArgumentException if there is a count mismatch. + */ + static void checkLabelValuesLength(Metric metric, String[] labelValues) { + checkArgument(labelValues.length == metric.getMetricSchema().labels().size(), LABEL_SIZE_ERROR); + } + + /** + * Check that the given {@code labelValues} match in count with the count of {@link + * LabelDescriptor} instances on the given {@code metric} + * + * @throws IllegalArgumentException if there is a count mismatch. + */ + static void checkLabelValuesLength(Metric metric, ImmutableList labelValues) { + checkArgument(labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_SIZE_ERROR); + } +} diff --git a/java/google/registry/monitoring/metrics/StoredMetric.java b/java/google/registry/monitoring/metrics/StoredMetric.java index 0c21a9c29..573e9d376 100644 --- a/java/google/registry/monitoring/metrics/StoredMetric.java +++ b/java/google/registry/monitoring/metrics/StoredMetric.java @@ -14,7 +14,6 @@ package google.registry.monitoring.metrics; -import static com.google.common.base.Preconditions.checkArgument; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -41,10 +40,6 @@ import org.joda.time.Instant; @ThreadSafe public class StoredMetric extends AbstractMetric implements SettableMetric { - private static final String LABEL_COUNT_ERROR = - "The count of labelValues must be equal to the underlying " - + "MetricDescriptor's count of labels."; - private final ConcurrentHashMap, V> values = new ConcurrentHashMap<>(); StoredMetric( @@ -63,7 +58,7 @@ public class StoredMetric extends AbstractMetric implements SettableMetric @Override public final void set(V value, String... labelValues) { - checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR); + MetricsUtils.checkLabelValuesLength(this, labelValues); set(value, ImmutableList.copyOf(labelValues)); } diff --git a/javatests/google/registry/monitoring/metrics/CounterTest.java b/javatests/google/registry/monitoring/metrics/CounterTest.java index 9ca2f6de4..a57019413 100644 --- a/javatests/google/registry/monitoring/metrics/CounterTest.java +++ b/javatests/google/registry/monitoring/metrics/CounterTest.java @@ -20,14 +20,18 @@ import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import org.joda.time.Instant; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; +import org.junit.runners.JUnit4; /** Unit tests for {@link Counter}. */ -@RunWith(MockitoJUnitRunner.class) +@RunWith(JUnit4.class) public class CounterTest { + @Rule public final ExpectedException thrown = ExpectedException.none(); + @Test public void testGetCardinality_reflectsCurrentCardinality() { Counter counter = @@ -56,11 +60,11 @@ public class CounterTest { ImmutableSet.of( LabelDescriptor.create("label1", "bar"), LabelDescriptor.create("label2", "bar"))); - try { - counter.increment("blah"); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage( + "The count of labelValues must be equal to the underlying Metric's count of labels."); + + counter.increment("blah"); } @Test diff --git a/javatests/google/registry/monitoring/metrics/StoredMetricTest.java b/javatests/google/registry/monitoring/metrics/StoredMetricTest.java index a4ad1c1f2..39733a60f 100644 --- a/javatests/google/registry/monitoring/metrics/StoredMetricTest.java +++ b/javatests/google/registry/monitoring/metrics/StoredMetricTest.java @@ -69,7 +69,9 @@ public class StoredMetricTest { Boolean.class); thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("The count of labelValues must be equal to"); + thrown.expectMessage( + "The count of labelValues must be equal to the underlying Metric's count of labels."); + dimensionalMetric.set(true, "foo"); }