Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MetricRecorder implementation #11128

Merged
merged 7 commits into from Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -23,7 +23,7 @@
*/
@Internal
public final class DoubleCounterMetricInstrument extends PartialMetricInstrument {
DoubleCounterMetricInstrument(long index, String name, String description, String unit,
public DoubleCounterMetricInstrument(int index, String name, String description, String unit,
DNVindhya marked this conversation as resolved.
Show resolved Hide resolved
List<String> requiredLabelKeys, List<String> optionalLabelKeys, boolean enableByDefault) {
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault);
}
Expand Down
Expand Up @@ -25,7 +25,7 @@
public final class DoubleHistogramMetricInstrument extends PartialMetricInstrument {
private final List<Double> bucketBoundaries;

DoubleHistogramMetricInstrument(long index, String name, String description, String unit,
public DoubleHistogramMetricInstrument(int index, String name, String description, String unit,
List<Double> bucketBoundaries, List<String> requiredLabelKeys, List<String> optionalLabelKeys,
boolean enableByDefault) {
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault);
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/LongCounterMetricInstrument.java
Expand Up @@ -23,7 +23,7 @@
*/
@Internal
public final class LongCounterMetricInstrument extends PartialMetricInstrument {
LongCounterMetricInstrument(long index, String name, String description, String unit,
public LongCounterMetricInstrument(int index, String name, String description, String unit,
List<String> requiredLabelKeys, List<String> optionalLabelKeys, boolean enableByDefault) {
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault);
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/LongGaugeMetricInstrument.java
Expand Up @@ -23,7 +23,7 @@
*/
@Internal
public final class LongGaugeMetricInstrument extends PartialMetricInstrument {
LongGaugeMetricInstrument(long index, String name, String description, String unit,
public LongGaugeMetricInstrument(int index, String name, String description, String unit,
List<String> requiredLabelKeys, List<String> optionalLabelKeys, boolean enableByDefault) {
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault);
}
Expand Down
Expand Up @@ -25,7 +25,7 @@
public final class LongHistogramMetricInstrument extends PartialMetricInstrument {
private final List<Long> bucketBoundaries;

LongHistogramMetricInstrument(long index, String name, String description, String unit,
public LongHistogramMetricInstrument(int index, String name, String description, String unit,
List<Long> bucketBoundaries, List<String> requiredLabelKeys, List<String> optionalLabelKeys,
boolean enableByDefault) {
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault);
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/MetricInstrument.java
Expand Up @@ -28,7 +28,7 @@ public interface MetricInstrument {
*
* @return the index of the metric instrument.
*/
public long getIndex();
public int getIndex();

/**
* Returns the name of the metric.
Expand Down
200 changes: 139 additions & 61 deletions api/src/main/java/io/grpc/MetricInstrumentRegistry.java
Expand Up @@ -16,24 +16,36 @@

package io.grpc;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
import javax.annotation.concurrent.GuardedBy;

/**
* A registry for globally registered metric instruments.
*/
@Internal
public final class MetricInstrumentRegistry {
public static final int INITIAL_INSTRUMENT_CAPACITY = 5;
DNVindhya marked this conversation as resolved.
Show resolved Hide resolved
private static MetricInstrumentRegistry instance;
private final List<MetricInstrument> metricInstruments;
private final Object lock = new Object();
private final Set<String> registeredMetricNames;
private volatile MetricInstrument[] metricInstruments;
private volatile int instrumentListCapacity = INITIAL_INSTRUMENT_CAPACITY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use metricInstruments.length instead

@GuardedBy("lock")
private int nextAvailableMetricIndex;

private MetricInstrumentRegistry() {
this.metricInstruments = new CopyOnWriteArrayList<>();
this.registeredMetricNames = new CopyOnWriteArraySet<>();
@VisibleForTesting
MetricInstrumentRegistry() {
this.metricInstruments = new MetricInstrument[INITIAL_INSTRUMENT_CAPACITY];
this.registeredMetricNames = new HashSet<>();
}

/**
Expand All @@ -50,7 +62,10 @@ public static synchronized MetricInstrumentRegistry getDefaultRegistry() {
* Returns a list of registered metric instruments.
*/
public List<MetricInstrument> getMetricInstruments() {
return Collections.unmodifiableList(metricInstruments);
synchronized (lock) {
return Collections.unmodifiableList(
Arrays.asList(Arrays.copyOfRange(metricInstruments, 0, nextAvailableMetricIndex)));
}
}

/**
Expand All @@ -65,20 +80,30 @@ public List<MetricInstrument> getMetricInstruments() {
* @return the newly created DoubleCounterMetricInstrument
* @throws IllegalStateException if a metric with the same name already exists
*/
// TODO(dnvindhya): Evaluate locks over synchronized methods and update if needed
public synchronized DoubleCounterMetricInstrument registerDoubleCounter(String name,
public DoubleCounterMetricInstrument registerDoubleCounter(String name,
String description, String unit, List<String> requiredLabelKeys,
List<String> optionalLabelKeys, boolean enableByDefault) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name");
checkNotNull(description, "description");
checkNotNull(unit, "unit");
checkNotNull(requiredLabelKeys, "requiredLabelKeys");
checkNotNull(optionalLabelKeys, "optionalLabelKeys");
synchronized (lock) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
}
int index = nextAvailableMetricIndex;
if (index + 1 == instrumentListCapacity) {
resizeMetricInstruments();
}
DoubleCounterMetricInstrument instrument = new DoubleCounterMetricInstrument(
index, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments[index] = instrument;
registeredMetricNames.add(name);
nextAvailableMetricIndex += 1;
return instrument;
}
long instrumentIndex = metricInstruments.size();
DoubleCounterMetricInstrument instrument = new DoubleCounterMetricInstrument(
instrumentIndex, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments.add(instrument);
registeredMetricNames.add(name);
return instrument;
}

/**
Expand All @@ -93,20 +118,30 @@ public synchronized DoubleCounterMetricInstrument registerDoubleCounter(String n
* @return the newly created LongCounterMetricInstrument
* @throws IllegalStateException if a metric with the same name already exists
*/
public synchronized LongCounterMetricInstrument registerLongCounter(String name,
public LongCounterMetricInstrument registerLongCounter(String name,
String description, String unit, List<String> requiredLabelKeys,
List<String> optionalLabelKeys, boolean enableByDefault) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name");
checkNotNull(description, "description");
checkNotNull(unit, "unit");
checkNotNull(requiredLabelKeys, "requiredLabelKeys");
checkNotNull(optionalLabelKeys, "optionalLabelKeys");
synchronized (lock) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
}
int index = nextAvailableMetricIndex;
if (index + 1 == instrumentListCapacity) {
resizeMetricInstruments();
}
LongCounterMetricInstrument instrument = new LongCounterMetricInstrument(
index, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments[index] = instrument;
registeredMetricNames.add(name);
nextAvailableMetricIndex += 1;
return instrument;
}
// Acquire lock?
long instrumentIndex = metricInstruments.size();
LongCounterMetricInstrument instrument = new LongCounterMetricInstrument(
instrumentIndex, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments.add(instrument);
registeredMetricNames.add(name);
return instrument;
}

/**
Expand All @@ -122,20 +157,32 @@ public synchronized LongCounterMetricInstrument registerLongCounter(String name,
* @return the newly created DoubleHistogramMetricInstrument
* @throws IllegalStateException if a metric with the same name already exists
*/
public synchronized DoubleHistogramMetricInstrument registerDoubleHistogram(String name,
public DoubleHistogramMetricInstrument registerDoubleHistogram(String name,
String description, String unit, List<Double> bucketBoundaries,
List<String> requiredLabelKeys, List<String> optionalLabelKeys, boolean enableByDefault) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name");
checkNotNull(description, "description");
checkNotNull(unit, "unit");
checkNotNull(bucketBoundaries, "bucketBoundaries");
checkNotNull(requiredLabelKeys, "requiredLabelKeys");
checkNotNull(optionalLabelKeys, "optionalLabelKeys");
synchronized (lock) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
}
int index = nextAvailableMetricIndex;
if (index + 1 == instrumentListCapacity) {
resizeMetricInstruments();
}
DoubleHistogramMetricInstrument instrument = new DoubleHistogramMetricInstrument(
index, name, description, unit, bucketBoundaries, requiredLabelKeys,
optionalLabelKeys,
enableByDefault);
metricInstruments[index] = instrument;
registeredMetricNames.add(name);
nextAvailableMetricIndex += 1;
return instrument;
}
long indexToInsertInstrument = metricInstruments.size();
DoubleHistogramMetricInstrument instrument = new DoubleHistogramMetricInstrument(
indexToInsertInstrument, name, description, unit, bucketBoundaries, requiredLabelKeys,
optionalLabelKeys,
enableByDefault);
metricInstruments.add(instrument);
registeredMetricNames.add(name);
return instrument;
}

/**
Expand All @@ -151,20 +198,32 @@ public synchronized DoubleHistogramMetricInstrument registerDoubleHistogram(Stri
* @return the newly created LongHistogramMetricInstrument
* @throws IllegalStateException if a metric with the same name already exists
*/
public synchronized LongHistogramMetricInstrument registerLongHistogram(String name,
public LongHistogramMetricInstrument registerLongHistogram(String name,
String description, String unit, List<Long> bucketBoundaries, List<String> requiredLabelKeys,
List<String> optionalLabelKeys, boolean enableByDefault) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name");
checkNotNull(description, "description");
checkNotNull(unit, "unit");
checkNotNull(bucketBoundaries, "bucketBoundaries");
checkNotNull(requiredLabelKeys, "requiredLabelKeys");
checkNotNull(optionalLabelKeys, "optionalLabelKeys");
synchronized (lock) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
}
int index = nextAvailableMetricIndex;
if (index + 1 == instrumentListCapacity) {
resizeMetricInstruments();
}
LongHistogramMetricInstrument instrument = new LongHistogramMetricInstrument(
index, name, description, unit, bucketBoundaries, requiredLabelKeys,
optionalLabelKeys,
enableByDefault);
metricInstruments[index] = instrument;
registeredMetricNames.add(name);
nextAvailableMetricIndex += 1;
return instrument;
}
long indexToInsertInstrument = metricInstruments.size();
LongHistogramMetricInstrument instrument = new LongHistogramMetricInstrument(
indexToInsertInstrument, name, description, unit, bucketBoundaries, requiredLabelKeys,
optionalLabelKeys,
enableByDefault);
metricInstruments.add(instrument);
registeredMetricNames.add(name);
return instrument;
}


Expand All @@ -180,18 +239,37 @@ public synchronized LongHistogramMetricInstrument registerLongHistogram(String n
* @return the newly created LongGaugeMetricInstrument
* @throws IllegalStateException if a metric with the same name already exists
*/
public synchronized LongGaugeMetricInstrument registerLongGauge(String name, String description,
public LongGaugeMetricInstrument registerLongGauge(String name, String description,
ejona86 marked this conversation as resolved.
Show resolved Hide resolved
String unit, List<String> requiredLabelKeys, List<String> optionalLabelKeys, boolean
enableByDefault) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name");
checkNotNull(description, "description");
checkNotNull(unit, "unit");
checkNotNull(requiredLabelKeys, "requiredLabelKeys");
checkNotNull(optionalLabelKeys, "optionalLabelKeys");
synchronized (lock) {
if (registeredMetricNames.contains(name)) {
throw new IllegalStateException("Metric with name " + name + " already exists");
}
int index = nextAvailableMetricIndex;
if (index + 1 == instrumentListCapacity) {
resizeMetricInstruments();
}
LongGaugeMetricInstrument instrument = new LongGaugeMetricInstrument(
index, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments[index] = instrument;
registeredMetricNames.add(name);
nextAvailableMetricIndex += 1;
return instrument;
}
long indexToInsertInstrument = metricInstruments.size();
LongGaugeMetricInstrument instrument = new LongGaugeMetricInstrument(
indexToInsertInstrument, name, description, unit, requiredLabelKeys, optionalLabelKeys,
enableByDefault);
metricInstruments.add(instrument);
registeredMetricNames.add(name);
return instrument;
}

private synchronized void resizeMetricInstruments() {
// Increase the capacity of the metricInstruments array by INITIAL_INSTRUMENT_CAPACITY
instrumentListCapacity += INITIAL_INSTRUMENT_CAPACITY;
MetricInstrument[] resizedMetricInstruments = Arrays.copyOf(metricInstruments,
instrumentListCapacity);
metricInstruments = resizedMetricInstruments;
}
}
21 changes: 8 additions & 13 deletions api/src/main/java/io/grpc/MetricRecorder.java
Expand Up @@ -16,11 +16,6 @@

package io.grpc;

import io.grpc.DoubleCounterMetricInstrument;
import io.grpc.DoubleHistogramMetricInstrument;
import io.grpc.Internal;
import io.grpc.LongCounterMetricInstrument;
import io.grpc.LongHistogramMetricInstrument;
import java.util.List;

/**
Expand All @@ -30,25 +25,25 @@
@Internal
public interface MetricRecorder {
/**
* Records a value for a double-precision counter metric instrument.
* Adds a value for a double-precision counter metric instrument.
*
* @param metricInstrument The counter metric instrument to record the value against.
* @param value The value to record.
* @param metricInstrument The counter metric instrument to add the value against.
* @param value The value to add.
* @param requiredLabelValues A list of required label values for the metric.
* @param optionalLabelValues A list of additional, optional label values for the metric.
*/
default void recordDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value,
default void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@temawi updated counter APIs. Can you PTAL?

List<String> requiredLabelValues, List<String> optionalLabelValues) {}

/**
* Records a value for a long valued counter metric instrument.
* Adds a value for a long valued counter metric instrument.
*
* @param metricInstrument The counter metric instrument to record the value against.
* @param value The value to record.
* @param metricInstrument The counter metric instrument to add the value against.
* @param value The value to add.
* @param requiredLabelValues A list of required label values for the metric.
* @param optionalLabelValues A list of additional, optional label values for the metric.
*/
default void recordLongCounter(LongCounterMetricInstrument metricInstrument, long value,
default void addLongCounter(LongCounterMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {}

/**
Expand Down
2 changes: 2 additions & 0 deletions api/src/main/java/io/grpc/MetricSink.java
Expand Up @@ -98,4 +98,6 @@ default void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrum
default void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value,
List<String> requiredLabelValues, List<String> optionalLabelValues) {
}

default void updateMeasures(List<MetricInstrument> instruments) {}
}