core: Add label values size validation in MetricRecorder (#11306)

Enhance MetricRecorder: Validate label values count against registered label keys count for default record APIs
This commit is contained in:
Vindhya Ningegowda 2024-06-21 17:06:55 -07:00 committed by GitHub
parent 1eec1459c2
commit 4849e0a191
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 63 additions and 46 deletions

View File

@ -16,6 +16,8 @@
package io.grpc; package io.grpc;
import static com.google.common.base.Preconditions.checkArgument;
import java.util.List; import java.util.List;
/** /**
@ -33,7 +35,16 @@ public interface MetricRecorder {
* @param optionalLabelValues A list of additional, optional label values for the metric. * @param optionalLabelValues A list of additional, optional label values for the metric.
*/ */
default void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value, default void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {} List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
}
/** /**
* Adds a value for a long valued counter metric instrument. * Adds a value for a long valued counter metric instrument.
@ -44,7 +55,16 @@ public interface MetricRecorder {
* @param optionalLabelValues A list of additional, optional label values for the metric. * @param optionalLabelValues A list of additional, optional label values for the metric.
*/ */
default void addLongCounter(LongCounterMetricInstrument metricInstrument, long value, default void addLongCounter(LongCounterMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {} List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
}
/** /**
* Records a value for a double-precision histogram metric instrument. * Records a value for a double-precision histogram metric instrument.
@ -55,7 +75,16 @@ public interface MetricRecorder {
* @param optionalLabelValues A list of additional, optional label values for the metric. * @param optionalLabelValues A list of additional, optional label values for the metric.
*/ */
default void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrument, double value, default void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrument, double value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {} List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
}
/** /**
* Records a value for a long valued histogram metric instrument. * Records a value for a long valued histogram metric instrument.
@ -66,7 +95,16 @@ public interface MetricRecorder {
* @param optionalLabelValues A list of additional, optional label values for the metric. * @param optionalLabelValues A list of additional, optional label values for the metric.
*/ */
default void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value, default void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {} List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
}
/** /**
* Registers a callback to produce metric values for only the listed instruments. The returned * Registers a callback to produce metric values for only the listed instruments. The returned
@ -95,8 +133,17 @@ public interface MetricRecorder {
* @param requiredLabelValues A list of required label values for the metric. * @param requiredLabelValues A list of required label values for the metric.
* @param optionalLabelValues A list of additional, optional label values for the metric. * @param optionalLabelValues A list of additional, optional label values for the metric.
*/ */
void recordLongGauge(LongGaugeMetricInstrument metricInstrument, long value, default void recordLongGauge(LongGaugeMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues); List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
}
} }
/** A handle to a registration, that allows unregistration. */ /** A handle to a registration, that allows unregistration. */

View File

@ -63,14 +63,8 @@ final class MetricRecorderImpl implements MetricRecorder {
@Override @Override
public void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value, public void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value,
List<String> requiredLabelValues, List<String> optionalLabelValues) { List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null MetricRecorder.super.addDoubleCounter(metricInstrument, value, requiredLabelValues,
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(), optionalLabelValues);
"Incorrect number of required labels provided. Expected: "
+ metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: "
+ metricInstrument.getOptionalLabelKeys().size());
for (MetricSink sink : metricSinks) { for (MetricSink sink : metricSinks) {
// TODO(dnvindhya): Move updating measures logic from sink to here // TODO(dnvindhya): Move updating measures logic from sink to here
int measuresSize = sink.getMeasuresSize(); int measuresSize = sink.getMeasuresSize();
@ -95,14 +89,8 @@ final class MetricRecorderImpl implements MetricRecorder {
@Override @Override
public void addLongCounter(LongCounterMetricInstrument metricInstrument, long value, public void addLongCounter(LongCounterMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) { List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null MetricRecorder.super.addLongCounter(metricInstrument, value, requiredLabelValues,
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(), optionalLabelValues);
"Incorrect number of required labels provided. Expected: "
+ metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: "
+ metricInstrument.getOptionalLabelKeys().size());
for (MetricSink sink : metricSinks) { for (MetricSink sink : metricSinks) {
int measuresSize = sink.getMeasuresSize(); int measuresSize = sink.getMeasuresSize();
if (measuresSize <= metricInstrument.getIndex()) { if (measuresSize <= metricInstrument.getIndex()) {
@ -126,14 +114,8 @@ final class MetricRecorderImpl implements MetricRecorder {
@Override @Override
public void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrument, double value, public void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrument, double value,
List<String> requiredLabelValues, List<String> optionalLabelValues) { List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null MetricRecorder.super.recordDoubleHistogram(metricInstrument, value, requiredLabelValues,
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(), optionalLabelValues);
"Incorrect number of required labels provided. Expected: "
+ metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: "
+ metricInstrument.getOptionalLabelKeys().size());
for (MetricSink sink : metricSinks) { for (MetricSink sink : metricSinks) {
int measuresSize = sink.getMeasuresSize(); int measuresSize = sink.getMeasuresSize();
if (measuresSize <= metricInstrument.getIndex()) { if (measuresSize <= metricInstrument.getIndex()) {
@ -157,14 +139,8 @@ final class MetricRecorderImpl implements MetricRecorder {
@Override @Override
public void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value, public void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) { List<String> requiredLabelValues, List<String> optionalLabelValues) {
checkArgument(requiredLabelValues != null MetricRecorder.super.recordLongHistogram(metricInstrument, value, requiredLabelValues,
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(), optionalLabelValues);
"Incorrect number of required labels provided. Expected: "
+ metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: "
+ metricInstrument.getOptionalLabelKeys().size());
for (MetricSink sink : metricSinks) { for (MetricSink sink : metricSinks) {
int measuresSize = sink.getMeasuresSize(); int measuresSize = sink.getMeasuresSize();
if (measuresSize <= metricInstrument.getIndex()) { if (measuresSize <= metricInstrument.getIndex()) {
@ -220,16 +196,10 @@ final class MetricRecorderImpl implements MetricRecorder {
@Override @Override
public void recordLongGauge(LongGaugeMetricInstrument metricInstrument, long value, public void recordLongGauge(LongGaugeMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) { List<String> requiredLabelValues, List<String> optionalLabelValues) {
BatchRecorder.super.recordLongGauge(metricInstrument, value, requiredLabelValues,
optionalLabelValues);
checkArgument(allowedInstruments.get(metricInstrument.getIndex()), checkArgument(allowedInstruments.get(metricInstrument.getIndex()),
"Instrument was not listed when registering callback: %s", metricInstrument); "Instrument was not listed when registering callback: %s", metricInstrument);
checkArgument(requiredLabelValues != null
&& requiredLabelValues.size() == metricInstrument.getRequiredLabelKeys().size(),
"Incorrect number of required labels provided. Expected: %s",
metricInstrument.getRequiredLabelKeys().size());
checkArgument(optionalLabelValues != null
&& optionalLabelValues.size() == metricInstrument.getOptionalLabelKeys().size(),
"Incorrect number of optional labels provided. Expected: %s",
metricInstrument.getOptionalLabelKeys().size());
// Registering the callback checked that the instruments were be present in sink. // Registering the callback checked that the instruments were be present in sink.
sink.recordLongGauge(metricInstrument, value, requiredLabelValues, optionalLabelValues); sink.recordLongGauge(metricInstrument, value, requiredLabelValues, optionalLabelValues);
} }