opendev/gerrit
Code Issues Proposed changes

Convert class for metric field to AutoValue

Browse Source 
This reduces the number of static methods for creating fields and allows
us to extend the Field class in future.

Change-Id: Ieaaacddd03b1e04e4e91f000eff950ecb292ec25
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2019-06-26 11:12:14 +02:00
parent 9ded2760b0
commit 441ac9783d
22 changed files with 144 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
java/com/google/gerrit/httpd/RequestMetrics.java
View File

@@ -28,15 +28,18 @@ public class RequestMetrics {
@Inject
public RequestMetrics(MetricMaker metricMaker) {
Field<Integer> statusCodeField =
Field.ofInteger("status").description("HTTP status code").build();
errors =
metricMaker.newCounter(
"http/server/error_count",
new Description("Rate of REST API error responses").setRate().setUnit("errors"),
Field.ofInteger("status", "HTTP status code"));
statusCodeField);
successes =
metricMaker.newCounter(
"http/server/success_count",
new Description("Rate of REST API success responses").setRate().setUnit("successes"),
Field.ofInteger("status", "HTTP status code"));
statusCodeField);
}
}

13
java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
View File

@@ -41,19 +41,20 @@ public class RestApiMetrics {
@Inject
RestApiMetrics(MetricMaker metrics) {
Field<String> view = Field.ofString("view", "view implementation class");
Field<String> viewField =
Field.ofString("view").description("view implementation class").build();
count =
metrics.newCounter(
"http/server/rest_api/count",
new Description("REST API calls by view").setRate(),
view);
viewField);
errorCount =
metrics.newCounter(
"http/server/rest_api/error_count",
new Description("REST API errors by view").setRate(),
view,
Field.ofInteger("error_code", "HTTP status code"));
viewField,
Field.ofInteger("error_code").description("HTTP status code").build());
serverLatency =
metrics.newTimer(
@@ -61,7 +62,7 @@ public class RestApiMetrics {
new Description("REST API call latency by view")
.setCumulative()
.setUnit(Units.MILLISECONDS),
view);
viewField);
responseBytes =
metrics.newHistogram(
@@ -69,7 +70,7 @@ public class RestApiMetrics {
new Description("Size of response on network (may be gzip compressed)")
.setCumulative()
.setUnit(Units.BYTES),
view);
viewField);
}
String view(ViewData viewData) {

3
java/com/google/gerrit/index/query/QueryProcessor.java
View File

@@ -60,14 +60,13 @@ public abstract class QueryProcessor<T> {
final Timer1<String> executionTime;
Metrics(MetricMaker metricMaker) {
Field<String> index = Field.ofString("index", "index name");
executionTime =
metricMaker.newTimer(
"query/query_latency",
new Description("Successful query latency, accumulated over the life of the process")
.setCumulative()
.setUnit(Description.Units.MILLISECONDS),
index);
Field.ofString("index").description("index name").build());
}
}

2
java/com/google/gerrit/metrics/BUILD
View File

@@ -8,6 +8,8 @@ java_library(
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/server/logging",
"//lib:guava",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",

150
java/com/google/gerrit/metrics/Field.java
View File

@@ -16,6 +16,8 @@ package com.google.gerrit.metrics;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue;
import java.util.Optional;
import java.util.function.Function;
/**
@@ -23,26 +25,19 @@ import java.util.function.Function;
*
* @param <T> type of field
*/
public class Field<T> {
@AutoValue
public abstract class Field<T> {
/**
* Break down metrics by boolean true/false.
*
* @param name field name
* @return boolean field
* @return builder for the boolean field
*/
public static Field<Boolean> ofBoolean(String name) {
return ofBoolean(name, null);
}
/**
* Break down metrics by boolean true/false.
*
* @param name field name
* @param description field description
* @return boolean field
*/
public static Field<Boolean> ofBoolean(String name, String description) {
return new Field<>(name, Boolean.class, description);
public static Field.Builder<Boolean> ofBoolean(String name) {
return new AutoValue_Field.Builder<Boolean>()
.valueType(Boolean.class)
.formatter(Object::toString)
.name(name);
}
/**
@@ -50,50 +45,10 @@ public class Field<T> {
*
* @param enumType type of enum
* @param name field name
* @return enum field
* @return builder for the enum field
*/
public static <E extends Enum<E>> Field<E> ofEnum(Class<E> enumType, String name) {
return ofEnum(enumType, name, null);
}
/**
* Break down metrics by cases of an enum.
*
* @param enumType type of enum
* @param name field name
* @param description field description
* @return enum field
*/
public static <E extends Enum<E>> Field<E> ofEnum(
Class<E> enumType, String name, String description) {
return new Field<>(name, enumType, description);
}
/**
* Break down metrics by string.
*
* <p>Each unique string will allocate a new submetric. <b>Do not use user content as a field
* value</b> as field values are never reclaimed.
*
* @param name field name
* @return string field
*/
public static Field<String> ofString(String name) {
return ofString(name, null);
}
/**
* Break down metrics by string.
*
* <p>Each unique string will allocate a new submetric. <b>Do not use user content as a field
* value</b> as field values are never reclaimed.
*
* @param name field name
* @param description field description
* @return string field
*/
public static Field<String> ofString(String name, String description) {
return new Field<>(name, String.class, description);
public static <E extends Enum<E>> Field.Builder<E> ofEnum(Class<E> enumType, String name) {
return new AutoValue_Field.Builder<E>().valueType(enumType).formatter(Enum::name).name(name);
}
/**
@@ -103,66 +58,59 @@ public class Field<T> {
* value</b> as field values are never reclaimed.
*
* @param name field name
* @return integer field
* @return builder for the integer field
*/
public static Field<Integer> ofInteger(String name) {
return ofInteger(name, null);
public static Field.Builder<Integer> ofInteger(String name) {
return new AutoValue_Field.Builder<Integer>()
.valueType(Integer.class)
.formatter(Object::toString)
.name(name);
}
/**
* Break down metrics by integer.
* Break down metrics by string.
*
* <p>Each unique integer will allocate a new submetric. <b>Do not use user content as a field
* <p>Each unique string will allocate a new submetric. <b>Do not use user content as a field
* value</b> as field values are never reclaimed.
*
* @param name field name
* @param description field description
* @return integer field
* @return builder for the string field
*/
public static Field<Integer> ofInteger(String name, String description) {
return new Field<>(name, Integer.class, description);
}
private final String name;
private final Class<T> keyType;
private final Function<T, String> formatter;
private final String description;
private Field(String name, Class<T> keyType, String description) {
checkArgument(name.matches("^[a-z_]+$"), "name must match [a-z_]");
this.name = name;
this.keyType = keyType;
this.formatter = initFormatter(keyType);
this.description = description;
public static Field.Builder<String> ofString(String name) {
return new AutoValue_Field.Builder<String>()
.valueType(String.class)
.formatter(s -> s)
.name(name);
}
/** @return name of this field within the metric. */
public String getName() {
return name;
}
public abstract String name();
/** @return type of value used within the field. */
public Class<T> getType() {
return keyType;
}
public abstract Class<T> valueType();
/** @return description text for the field explaining its range of values. */
public String getDescription() {
return description;
}
public abstract Optional<String> description();
public Function<T, String> formatter() {
return formatter;
}
/** @return formatter to format field values. */
public abstract Function<T, String> formatter();
private static <T> Function<T, String> initFormatter(Class<T> keyType) {
if (keyType == String.class) {
return s -> (String) s;
} else if (keyType == Integer.class || keyType == Boolean.class) {
return Object::toString;
} else if (Enum.class.isAssignableFrom(keyType)) {
return in -> ((Enum<?>) in).name();
}
throw new IllegalStateException("unsupported type " + keyType.getName());
@AutoValue.Builder
public abstract static class Builder<T> {
abstract Builder<T> name(String name);
abstract Builder<T> valueType(Class<T> type);
abstract Builder<T> formatter(Function<T, String> formatter);
public abstract Builder<T> description(String description);
abstract Field<T> autoBuild();
public Field<T> build() {
Field<T> field = autoBuild();
checkArgument(field.name().matches("^[a-z_]+$"), "name must match [a-z_]");
return field;
}
}
}

6
java/com/google/gerrit/metrics/dropwizard/MetricJson.java
View File

@@ -189,10 +189,10 @@ class MetricJson {
String description;
FieldJson(Field<?> field) {
this.name = field.getName();
this.description = field.getDescription();
this.name = field.name();
this.description = field.description().orElse(null);
this.type =
Enum.class.isAssignableFrom(field.getType()) ? field.getType().getSimpleName() : null;
Enum.class.isAssignableFrom(field.valueType()) ? field.valueType().getSimpleName() : null;
}
}
}

7
java/com/google/gerrit/metrics/proc/ProcMetricModule.java
View File

@@ -148,12 +148,15 @@ public class ProcMetricModule extends MetricModule {
}
private void procJvmGc(MetricMaker metrics) {
Field<String> gcNameField =
Field.ofString("gc_name").description("The name of the garbage collector").build();
CallbackMetric1<String, Long> gcCount =
metrics.newCallbackMetric(
"proc/jvm/gc/count",
Long.class,
new Description("Number of GCs").setCumulative(),
Field.ofString("gc_name", "The name of the garbage collector"));
gcNameField);
CallbackMetric1<String, Long> gcTime =
metrics.newCallbackMetric(
@@ -162,7 +165,7 @@ public class ProcMetricModule extends MetricModule {
new Description("Approximate accumulated GC elapsed time")
.setCumulative()
.setUnit(Units.MILLISECONDS),
Field.ofString("gc_name", "The name of the garbage collector"));
gcNameField);
metrics.newTrigger(
gcCount,

2
java/com/google/gerrit/server/cache/CacheMetrics.java vendored
View File

@@ -33,7 +33,7 @@ import java.util.Set;
public class CacheMetrics {
@Inject
public CacheMetrics(MetricMaker metrics, DynamicMap<Cache<?, ?>> cacheMap) {
Field<String> F_NAME = Field.ofString("cache_name");
Field<String> F_NAME = Field.ofString("cache_name").build();
CallbackMetric1<String, Long> memEnt =
metrics.newCallbackMetric(

2
java/com/google/gerrit/server/change/ChangeFinder.java
View File

@@ -98,7 +98,7 @@ public class ChangeFinder {
new Description("Total number of API calls per identifier type.")
.setRate()
.setUnit("requests"),
Field.ofEnum(ChangeIdType.class, "change_id_type"));
Field.ofEnum(ChangeIdType.class, "change_id_type").build());
List<ChangeIdType> configuredChangeIdTypes =
ConfigUtil.getEnumList(config, "change", "api", "allowedIdentifier", ChangeIdType.ALL);
// Ensure that PROJECT_NUMERIC_ID can't be removed

2
java/com/google/gerrit/server/events/EventsMetrics.java
View File

@@ -31,7 +31,7 @@ public class EventsMetrics implements EventListener {
metricMaker.newCounter(
"events",
new Description("Triggered events").setRate().setUnit("triggered events"),
Field.ofString("type"));
Field.ofString("type").build());
}
@Override

2
java/com/google/gerrit/server/extensions/webui/UiActions.java
View File

@@ -71,7 +71,7 @@ public class UiActions {
new com.google.gerrit.metrics.Description("Latency for RestView#getDescription calls")
.setCumulative()
.setUnit(Units.MILLISECONDS),
Field.ofString("view"));
Field.ofString("view").build());
}
public <R extends RestResource> Iterable<UiAction.Description> from(

12
java/com/google/gerrit/server/git/UploadPackMetricsHook.java
View File

@@ -43,14 +43,14 @@ public class UploadPackMetricsHook implements PostUploadHook {
@Inject
UploadPackMetricsHook(MetricMaker metricMaker) {
Field<Operation> operation = Field.ofEnum(Operation.class, "operation");
Field<Operation> operationField = Field.ofEnum(Operation.class, "operation").build();
requestCount =
metricMaker.newCounter(
"git/upload-pack/request_count",
new Description("Total number of git-upload-pack requests")
.setRate()
.setUnit("requests"),
operation);
operationField);
counting =
metricMaker.newTimer(
@@ -58,7 +58,7 @@ public class UploadPackMetricsHook implements PostUploadHook {
new Description("Time spent in the 'Counting...' phase")
.setCumulative()
.setUnit(Units.MILLISECONDS),
operation);
operationField);
compressing =
metricMaker.newTimer(
@@ -66,7 +66,7 @@ public class UploadPackMetricsHook implements PostUploadHook {
new Description("Time spent in the 'Compressing...' phase")
.setCumulative()
.setUnit(Units.MILLISECONDS),
operation);
operationField);
writing =
metricMaker.newTimer(
@@ -74,7 +74,7 @@ public class UploadPackMetricsHook implements PostUploadHook {
new Description("Time spent transferring bytes to client")
.setCumulative()
.setUnit(Units.MILLISECONDS),
operation);
operationField);
packBytes =
metricMaker.newHistogram(
@@ -82,7 +82,7 @@ public class UploadPackMetricsHook implements PostUploadHook {
new Description("Distribution of sizes of packs sent to clients")
.setCumulative()
.setUnit(Units.BYTES),
operation);
operationField);
}
@Override

15
java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
View File

@@ -199,7 +199,14 @@ public class AsyncReceiveCommits implements PreReceiveHook {
metricMaker.newHistogram(
"receivecommits/changes_per_push",
new Description("number of changes uploaded in a single push.").setCumulative(),
Field.ofEnum(PushType.class, "type", "type of push (create/replace, autoclose)"));
Field.ofEnum(PushType.class, "type")
.description("type of push (create/replace, autoclose)")
.build());
Field<PushType> pushTypeField =
Field.ofEnum(PushType.class, "type")
.description("type of push (create/replace, autoclose, normal)")
.build();
latencyPerChange =
metricMaker.newTimer(
@@ -209,8 +216,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
+ "(Only includes pushes which contain changes.)")
.setUnit(Units.MILLISECONDS)
.setCumulative(),
Field.ofEnum(
PushType.class, "type", "type of push (create/replace, autoclose, normal)"));
pushTypeField);
latencyPerPush =
metricMaker.newTimer(
@@ -218,8 +224,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
new Description("processing delay for a processing single push")
.setUnit(Units.MILLISECONDS)
.setCumulative(),
Field.ofEnum(
PushType.class, "type", "type of push (create/replace, autoclose, normal)"));
pushTypeField);
timeouts =
metricMaker.newCounter(

10
java/com/google/gerrit/server/notedb/NoteDbMetrics.java
View File

@@ -45,7 +45,7 @@ class NoteDbMetrics {
@Inject
NoteDbMetrics(MetricMaker metrics) {
Field<NoteDbTable> view = Field.ofEnum(NoteDbTable.class, "table");
Field<NoteDbTable> tableField = Field.ofEnum(NoteDbTable.class, "table").build();
updateLatency =
metrics.newTimer(
@@ -53,7 +53,7 @@ class NoteDbMetrics {
new Description("NoteDb update latency by table")
.setCumulative()
.setUnit(Units.MILLISECONDS),
view);
tableField);
stageUpdateLatency =
metrics.newTimer(
@@ -61,7 +61,7 @@ class NoteDbMetrics {
new Description("Latency for staging updates to NoteDb by table")
.setCumulative()
.setUnit(Units.MICROSECONDS),
view);
tableField);
readLatency =
metrics.newTimer(
@@ -69,7 +69,7 @@ class NoteDbMetrics {
new Description("NoteDb read latency by table")
.setCumulative()
.setUnit(Units.MILLISECONDS),
view);
tableField);
parseLatency =
metrics.newTimer(
@@ -77,6 +77,6 @@ class NoteDbMetrics {
new Description("NoteDb parse latency by table")
.setCumulative()
.setUnit(Units.MICROSECONDS),
view);
tableField);
}
}

4
java/com/google/gerrit/server/notedb/Sequences.java
View File

@@ -95,8 +95,8 @@ public class Sequences {
new Description("Latency of requesting IDs from repo sequences")
.setCumulative()
.setUnit(Units.MILLISECONDS),
Field.ofEnum(SequenceType.class, "sequence"),
Field.ofBoolean("multiple"));
Field.ofEnum(SequenceType.class, "sequence").build(),
Field.ofBoolean("multiple").build());
}
public int nextAccountId() {

16
java/com/google/gerrit/server/plugincontext/PluginContext.java
View File

@@ -118,22 +118,26 @@ public class PluginContext<T> {
@Inject
PluginMetrics(MetricMaker metricMaker) {
Field<String> pluginNameField = Field.ofString("plugin_name").build();
Field<String> classNameField = Field.ofString("class_name").build();
Field<String> exportNameField = Field.ofString("export_name").build();
this.latency =
metricMaker.newTimer(
"plugin/latency",
new Description("Latency for plugin invocation")
.setCumulative()
.setUnit(Units.MILLISECONDS),
Field.ofString("plugin_name"),
Field.ofString("class_name"),
Field.ofString("export_name"));
pluginNameField,
classNameField,
exportNameField);
this.errorCount =
metricMaker.newCounter(
"plugin/error_count",
new Description("Number of plugin errors").setCumulative().setUnit("errors"),
Field.ofString("plugin_name"),
Field.ofString("class_name"),
Field.ofString("export_name"));
pluginNameField,
classNameField,
exportNameField);
}
Timer3.Context startLatency(Extension<?> extension) {

2
java/com/google/gerrit/server/project/ProjectState.java
View File

@@ -135,7 +135,7 @@ public class ProjectState {
new Description("Latency for access computations in ProjectState")
.setCumulative()
.setUnit(Units.NANOSECONDS),
Field.ofString("method"));
Field.ofString("method").build());
if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) {
localOwners = Collections.emptySet();

6
java/com/google/gerrit/server/update/RetryHelper.java
View File

@@ -111,7 +111,7 @@ public class RetryHelper {
@Inject
Metrics(MetricMaker metricMaker) {
Field<ActionType> view = Field.ofEnum(ActionType.class, "action_type");
Field<ActionType> actionTypeField = Field.ofEnum(ActionType.class, "action_type").build();
attemptCounts =
metricMaker.newCounter(
"action/retry_attempt_count",
@@ -120,7 +120,7 @@ public class RetryHelper {
+ " (0 == single attempt, no retry)")
.setCumulative()
.setUnit("attempts"),
view);
actionTypeField);
timeoutCount =
metricMaker.newCounter(
"action/retry_timeout_count",
@@ -128,7 +128,7 @@ public class RetryHelper {
"Number of action executions of RetryHelper that ultimately timed out")
.setCumulative()
.setUnit("timeouts"),
view);
actionTypeField);
}
}

6
javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
View File

@@ -80,7 +80,9 @@ public class ProcMetricModuleTest {
public void counter1() {
Counter1<String> cntr =
metrics.newCounter(
"test/count", new Description("simple test").setCumulative(), Field.ofString("action"));
"test/count",
new Description("simple test").setCumulative(),
Field.ofString("action").build());
Counter total = get("test/count_total", Counter.class);
assertThat(total.getCount()).isEqualTo(0);
@@ -105,7 +107,7 @@ public class ProcMetricModuleTest {
new Description("simple test")
.setCumulative()
.setFieldOrdering(FieldOrdering.PREFIX_FIELDS_BASENAME),
Field.ofString("action"));
Field.ofString("action").build());
Counter total = get("test/count_total", Counter.class);
assertThat(total.getCount()).isEqualTo(0);

50
javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
View File

@@ -230,24 +230,24 @@ public class PerformanceLogContextTest {
metricMaker.newTimer(
"test2/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"));
Field.ofString("foo").build());
timer1.start("value1").close();
Timer2<String, String> timer2 =
metricMaker.newTimer(
"test3/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"));
Field.ofString("foo").build(),
Field.ofString("bar").build());
timer2.start("value1", "value2").close();
Timer3<String, String, String> timer3 =
metricMaker.newTimer(
"test4/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"),
Field.ofString("baz"));
Field.ofString("foo").build(),
Field.ofString("bar").build(),
Field.ofString("baz").build());
timer3.start("value1", "value2", "value3").close();
assertThat(LoggingContext.getInstance().getPerformanceLogRecords()).hasSize(4);
@@ -289,24 +289,24 @@ public class PerformanceLogContextTest {
metricMaker.newTimer(
"test1/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"));
Field.ofString("foo").build());
timer1.start(null).close();
Timer2<String, String> timer2 =
metricMaker.newTimer(
"test2/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"));
Field.ofString("foo").build(),
Field.ofString("bar").build());
timer2.start(null, null).close();
Timer3<String, String, String> timer3 =
metricMaker.newTimer(
"test3/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"),
Field.ofString("baz"));
Field.ofString("foo").build(),
Field.ofString("bar").build(),
Field.ofString("baz").build());
timer3.start(null, null, null).close();
assertThat(LoggingContext.getInstance().getPerformanceLogRecords()).hasSize(3);
@@ -345,24 +345,26 @@ public class PerformanceLogContextTest {
Timer1<String> timer1 =
metricMaker.newTimer(
"test2/latency", new Description("Latency metric for testing"), Field.ofString("foo"));
"test2/latency",
new Description("Latency metric for testing"),
Field.ofString("foo").build());
timer1.start("value1").close();
Timer2<String, String> timer2 =
metricMaker.newTimer(
"test3/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"));
Field.ofString("foo").build(),
Field.ofString("bar").build());
timer2.start("value1", "value2").close();
Timer3<String, String, String> timer3 =
metricMaker.newTimer(
"test4/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"),
Field.ofString("baz"));
Field.ofString("foo").build(),
Field.ofString("bar").build(),
Field.ofString("baz").build());
timer3.start("value1", "value2", "value3").close();
assertThat(LoggingContext.getInstance().isPerformanceLogging()).isFalse();
@@ -391,24 +393,24 @@ public class PerformanceLogContextTest {
metricMaker.newTimer(
"test2/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"));
Field.ofString("foo").build());
timer1.start("value1").close();
Timer2<String, String> timer2 =
metricMaker.newTimer(
"test3/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"));
Field.ofString("foo").build(),
Field.ofString("bar").build());
timer2.start("value1", "value2").close();
Timer3<String, String, String> timer3 =
metricMaker.newTimer(
"test4/latency",
new Description("Latency metric for testing"),
Field.ofString("foo"),
Field.ofString("bar"),
Field.ofString("baz"));
Field.ofString("foo").build(),
Field.ofString("bar").build(),
Field.ofString("baz").build());
timer3.start("value1", "value2", "value3").close();
assertThat(LoggingContext.getInstance().getPerformanceLogRecords()).isEmpty();

2
plugins/hooks

Submodule plugins/hooks updated: 12447394c9...8c6baf54bc

2
plugins/replication

Submodule plugins/replication updated: 1e7930decf...88c979d99a