Factor out labelValue length check to abstract base class and small name fix

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=131902964
This commit is contained in:
shikhman 2016-08-31 19:47:13 -07:00 committed by Ben McIlwain
parent a0f1a8b0bc
commit c11ac3129f
7 changed files with 80 additions and 42 deletions

View file

@ -14,6 +14,7 @@
package google.registry.monitoring.metrics; package google.registry.monitoring.metrics;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import google.registry.monitoring.metrics.MetricSchema.Kind; import google.registry.monitoring.metrics.MetricSchema.Kind;

View file

@ -50,10 +50,6 @@ public final class Counter extends AbstractMetric<Long>
private static final float HASHMAP_LOAD_FACTOR = 0.75f; private static final float HASHMAP_LOAD_FACTOR = 0.75f;
private static final int HASHMAP_CONCURRENCY_LEVEL = 16; 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. * 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<Long>
} }
@VisibleForTesting @VisibleForTesting
void incrementBy(long offset, Instant startTime, ImmutableList<String> labelValues) { void incrementBy(long offset, Instant startTimestamp, ImmutableList<String> labelValues) {
Lock lock = valueLocks.get(labelValues); Lock lock = valueLocks.get(labelValues);
lock.lock(); lock.lock();
try { try {
values.addAndGet(labelValues, offset); values.addAndGet(labelValues, offset);
valueStartTimestamps.putIfAbsent(labelValues, startTime); valueStartTimestamps.putIfAbsent(labelValues, startTimestamp);
} finally { } finally {
lock.unlock(); lock.unlock();
} }
@ -103,7 +99,7 @@ public final class Counter extends AbstractMetric<Long>
@Override @Override
public final void incrementBy(long offset, String... labelValues) { 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"); checkArgument(offset >= 0, "The offset provided must be non-negative");
incrementBy(offset, Instant.now(), ImmutableList.copyOf(labelValues)); incrementBy(offset, Instant.now(), ImmutableList.copyOf(labelValues));
@ -111,7 +107,7 @@ public final class Counter extends AbstractMetric<Long>
@Override @Override
public final void increment(String... labelValues) { 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)); incrementBy(1L, Instant.now(), ImmutableList.copyOf(labelValues));
} }
@ -152,13 +148,13 @@ public final class Counter extends AbstractMetric<Long>
} }
@VisibleForTesting @VisibleForTesting
final void set(Long value, Instant startTime, ImmutableList<String> labelValues) { final void set(Long value, Instant startTimestamp, ImmutableList<String> labelValues) {
Lock lock = valueLocks.get(labelValues); Lock lock = valueLocks.get(labelValues);
lock.lock(); lock.lock();
try { try {
this.values.put(labelValues, value); this.values.put(labelValues, value);
valueStartTimestamps.putIfAbsent(labelValues, startTime); valueStartTimestamps.putIfAbsent(labelValues, startTimestamp);
} finally { } finally {
lock.unlock(); lock.unlock();
} }
@ -166,13 +162,13 @@ public final class Counter extends AbstractMetric<Long>
@Override @Override
public final void set(Long value, String... labelValues) { 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)); set(value, Instant.now(), ImmutableList.copyOf(labelValues));
} }
@VisibleForTesting @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 // 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. // after this call, without the possibility of interleaving with another reset() call.
Set<ImmutableList<String>> keys = values.asMap().keySet(); Set<ImmutableList<String>> keys = values.asMap().keySet();
@ -182,7 +178,7 @@ public final class Counter extends AbstractMetric<Long>
for (ImmutableList<String> labelValues : keys) { for (ImmutableList<String> labelValues : keys) {
this.values.put(labelValues, 0); this.values.put(labelValues, 0);
this.valueStartTimestamps.put(labelValues, startTime); this.valueStartTimestamps.put(labelValues, startTimestamp);
} }
for (int i = 0; i < valueLocks.size(); i++) { for (int i = 0; i < valueLocks.size(); i++) {
@ -196,13 +192,13 @@ public final class Counter extends AbstractMetric<Long>
} }
@VisibleForTesting @VisibleForTesting
final void reset(Instant startTime, ImmutableList<String> labelValues) { final void reset(Instant startTimestamp, ImmutableList<String> labelValues) {
Lock lock = valueLocks.get(labelValues); Lock lock = valueLocks.get(labelValues);
lock.lock(); lock.lock();
try { try {
this.values.put(labelValues, 0); this.values.put(labelValues, 0);
this.valueStartTimestamps.put(labelValues, startTime); this.valueStartTimestamps.put(labelValues, startTimestamp);
} finally { } finally {
lock.unlock(); lock.unlock();
} }
@ -210,7 +206,7 @@ public final class Counter extends AbstractMetric<Long>
@Override @Override
public final void reset(String... labelValues) { 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)); reset(Instant.now(), ImmutableList.copyOf(labelValues));
} }

View file

@ -14,8 +14,6 @@
package google.registry.monitoring.metrics; package google.registry.monitoring.metrics;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue; import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import org.joda.time.Instant; import org.joda.time.Instant;
@ -28,12 +26,6 @@ import org.joda.time.Interval;
@AutoValue @AutoValue
public abstract class MetricPoint<V> { public abstract class MetricPoint<V> {
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}. * Returns a new {@link MetricPoint} representing a value at a specific {@link Instant}.
* *
@ -42,8 +34,8 @@ public abstract class MetricPoint<V> {
*/ */
static <V> MetricPoint<V> create( static <V> MetricPoint<V> create(
Metric<V> metric, ImmutableList<String> labelValues, Instant timestamp, V value) { Metric<V> metric, ImmutableList<String> labelValues, Instant timestamp, V value) {
checkArgument( MetricsUtils.checkLabelValuesLength(metric, labelValues);
labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_COUNT_ERROR);
return new AutoValue_MetricPoint<>( return new AutoValue_MetricPoint<>(
metric, labelValues, new Interval(timestamp, timestamp), value); metric, labelValues, new Interval(timestamp, timestamp), value);
} }
@ -61,8 +53,8 @@ public abstract class MetricPoint<V> {
Instant startTime, Instant startTime,
Instant endTime, Instant endTime,
V value) { V value) {
checkArgument( MetricsUtils.checkLabelValuesLength(metric, labelValues);
labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_COUNT_ERROR);
return new AutoValue_MetricPoint<>( return new AutoValue_MetricPoint<>(
metric, labelValues, new Interval(startTime, endTime), value); metric, labelValues, new Interval(startTime, endTime), value);
} }

View file

@ -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<String> labelValues) {
checkArgument(labelValues.size() == metric.getMetricSchema().labels().size(), LABEL_SIZE_ERROR);
}
}

View file

@ -14,7 +14,6 @@
package google.registry.monitoring.metrics; package google.registry.monitoring.metrics;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
@ -41,10 +40,6 @@ import org.joda.time.Instant;
@ThreadSafe @ThreadSafe
public class StoredMetric<V> extends AbstractMetric<V> implements SettableMetric<V> { public class StoredMetric<V> extends AbstractMetric<V> implements SettableMetric<V> {
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<ImmutableList<String>, V> values = new ConcurrentHashMap<>(); private final ConcurrentHashMap<ImmutableList<String>, V> values = new ConcurrentHashMap<>();
StoredMetric( StoredMetric(
@ -63,7 +58,7 @@ public class StoredMetric<V> extends AbstractMetric<V> implements SettableMetric
@Override @Override
public final void set(V value, String... labelValues) { 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)); set(value, ImmutableList.copyOf(labelValues));
} }

View file

@ -20,14 +20,18 @@ import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import org.joda.time.Instant; import org.joda.time.Instant;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.runners.MockitoJUnitRunner; import org.junit.runners.JUnit4;
/** Unit tests for {@link Counter}. */ /** Unit tests for {@link Counter}. */
@RunWith(MockitoJUnitRunner.class) @RunWith(JUnit4.class)
public class CounterTest { public class CounterTest {
@Rule public final ExpectedException thrown = ExpectedException.none();
@Test @Test
public void testGetCardinality_reflectsCurrentCardinality() { public void testGetCardinality_reflectsCurrentCardinality() {
Counter counter = Counter counter =
@ -56,11 +60,11 @@ public class CounterTest {
ImmutableSet.of( ImmutableSet.of(
LabelDescriptor.create("label1", "bar"), LabelDescriptor.create("label2", "bar"))); LabelDescriptor.create("label1", "bar"), LabelDescriptor.create("label2", "bar")));
try { thrown.expect(IllegalArgumentException.class);
counter.increment("blah"); thrown.expectMessage(
fail("expected IllegalArgumentException"); "The count of labelValues must be equal to the underlying Metric's count of labels.");
} catch (IllegalArgumentException expected) {
} counter.increment("blah");
} }
@Test @Test

View file

@ -69,7 +69,9 @@ public class StoredMetricTest {
Boolean.class); Boolean.class);
thrown.expect(IllegalArgumentException.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"); dimensionalMetric.set(true, "foo");
} }