Enforce monotonicity for IncrementableMetrics

This change enforces that IncrementableMetrics should only monotonically
increase in value, and adds a new increment() method to increment by one, which is slightly faster than incrementBy (due to a lack of non-negative checking) in the common case that the counter should only be incremented by one.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=130578421
This commit is contained in:
shikhman 2016-08-17 16:35:08 -07:00 committed by Ben McIlwain
parent 01e38790fd
commit f10a7d8fb0
4 changed files with 33 additions and 9 deletions

View file

@ -49,17 +49,25 @@ public final class Counter extends AbstractMetric<Long>
} }
@VisibleForTesting @VisibleForTesting
void incrementBy(Number offset, ImmutableList<String> labelValues) { void incrementBy(long offset, ImmutableList<String> labelValues) {
values.addAndGet(labelValues, offset.longValue()); values.addAndGet(labelValues, offset);
} }
@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); checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR);
checkArgument(offset >= 0, "The offset provided must be non-negative");
incrementBy(offset, ImmutableList.copyOf(labelValues)); incrementBy(offset, ImmutableList.copyOf(labelValues));
} }
@Override
public final void increment(String... labelValues) {
checkArgument(labelValues.length == this.getMetricSchema().labels().size(), LABEL_COUNT_ERROR);
incrementBy(1L, ImmutableList.copyOf(labelValues));
}
/** /**
* Returns a snapshot of the metric's values. The timestamp of each {@link MetricPoint} will be * Returns a snapshot of the metric's values. The timestamp of each {@link MetricPoint} will be
* the last modification time for that tuple of label values. * the last modification time for that tuple of label values.

View file

@ -17,19 +17,35 @@ package google.registry.monitoring.metrics;
/** /**
* A {@link Metric} which can be incremented. * A {@link Metric} which can be incremented.
* *
* <p>This is a view into a {@link Counter} to provide compile-time checking to disallow re-setting * <p>This is a view into a {@link Counter} to provide compile-time checking to disallow arbitrarily
* the metric, which is useful for metrics which should be monotonic. * setting the metric, which is useful for metrics which should be monotonically increasing.
*/ */
public interface IncrementableMetric extends Metric<Long> { public interface IncrementableMetric extends Metric<Long> {
/** /**
* Increments a metric for a given set of label values. * Increments a metric by 1 for the given set of label values.
* *
* <p>If the metric is undefined for given label values, it will first be set to zero. * Use this method rather than {@link IncrementableMetric#incrementBy(long, String...)} if the
* increment value is zero, as it will be slightly more performant.
*
* <p>If the metric is undefined for given label values, it will be incremented from zero.
* *
* <p>The metric's timestamp will be updated to the current time for the given label values. * <p>The metric's timestamp will be updated to the current time for the given label values.
* *
* <p>The count of {@code labelValues} must be equal to the underlying metric's count of labels. * <p>The count of {@code labelValues} must be equal to the underlying metric's count of labels.
*/ */
void increment(String... labelValues);
/**
* Increments a metric by the given non-negative offset for the given set of label values.
*
* <p>If the metric is undefined for given label values, it will be incremented from zero.
*
* <p>The metric's timestamp will be updated to the current time for the given label values.
*
* <p>The count of {@code labelValues} must be equal to the underlying metric's count of labels.
*
* @throws IllegalArgumentException if the offset is negative.
*/
void incrementBy(long offset, String... labelValues); void incrementBy(long offset, String... labelValues);
} }

View file

@ -104,7 +104,7 @@ public class MetricReporter extends AbstractScheduledService {
for (Metric<?> metric : metricRegistry.getRegisteredMetrics()) { for (Metric<?> metric : metricRegistry.getRegisteredMetrics()) {
points.addAll(metric.getTimestampedValues()); points.addAll(metric.getTimestampedValues());
logger.fine(String.format("Enqueued metric %s", metric)); logger.fine(String.format("Enqueued metric %s", metric));
MetricMetrics.pushedPoints.incrementBy(1, MetricMetrics.pushedPoints.increment(
metric.getMetricSchema().kind().name(), metric.getValueClass().toString()); metric.getMetricSchema().kind().name(), metric.getValueClass().toString());
} }
@ -112,7 +112,7 @@ public class MetricReporter extends AbstractScheduledService {
logger.severe("writeQueue full, dropped a reporting interval of points"); logger.severe("writeQueue full, dropped a reporting interval of points");
} }
MetricMetrics.pushIntervals.incrementBy(1); MetricMetrics.pushIntervals.increment();
} }
@Override @Override

View file

@ -195,7 +195,7 @@ public class StackdriverWriter implements MetricWriter {
monitoringClient.projects().timeSeries().create(projectResource, request).execute(); monitoringClient.projects().timeSeries().create(projectResource, request).execute();
for (TimeSeries timeSeries : timeSeriesList) { for (TimeSeries timeSeries : timeSeriesList) {
pushedPoints.incrementBy(1, timeSeries.getMetricKind(), timeSeries.getValueType()); pushedPoints.increment(timeSeries.getMetricKind(), timeSeries.getValueType());
} }
logger.info(String.format("Flushed %d metrics to Stackdriver", timeSeriesList.size())); logger.info(String.format("Flushed %d metrics to Stackdriver", timeSeriesList.size()));
} }