Add metrics for hook execution
- Execution latency
- Execution count
- Error count
- Timeout count
Change-Id: I7ed95e08608b436ab533929322f951e85a45b7f9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookArgs.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookArgs.java
index 7797e05..f68395c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookArgs.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookArgs.java
@@ -30,14 +30,21 @@
class HookArgs {
private final String anonymousCowardName;
private final Provider<String> urlProvider;
+ private final HookMetrics metrics;
private final List<String> args;
- HookArgs(String anonymousCowardName, Provider<String> urlProvider) {
+ HookArgs(String anonymousCowardName, Provider<String> urlProvider,
+ HookMetrics metrics) {
this.anonymousCowardName = anonymousCowardName;
this.urlProvider = urlProvider;
+ this.metrics = metrics;
this.args = new ArrayList<>();
}
+ public HookMetrics metrics() {
+ return metrics;
+ }
+
public ImmutableList<String> get() {
return ImmutableList.copyOf(args);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookFactory.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookFactory.java
index 3ec1d8c..61207b8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookFactory.java
@@ -36,6 +36,7 @@
private final HookExecutor syncHookExecutor;
private final Config config;
private final String anonymousCowardName;
+ private final HookMetrics metrics;
private final Provider<String> urlProvider;
private final Path hooksPath;
@@ -45,11 +46,13 @@
@GerritServerConfig Config config,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
+ HookMetrics metrics,
SitePaths sitePaths) {
this.queue = queue;
this.syncHookExecutor = syncHookExecutor;
this.config = config;
this.anonymousCowardName = anonymousCowardName;
+ this.metrics = metrics;
this.urlProvider = urlProvider;
String v = config.getString("hooks", null, "path");
@@ -75,6 +78,6 @@
}
public HookArgs createArgs() {
- return new HookArgs(anonymousCowardName, urlProvider);
+ return new HookArgs(anonymousCowardName, urlProvider, metrics);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookMetrics.java
new file mode 100644
index 0000000..066def0
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookMetrics.java
@@ -0,0 +1,69 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// 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 com.googlesource.gerrit.plugins.hooks;
+
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer1;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+@Singleton
+public class HookMetrics {
+ private final Timer1<String> latency;
+ private final Counter1<String> count;
+ private final Counter1<String> error;
+ private final Counter1<String> timeout;
+
+ @Inject
+ HookMetrics(MetricMaker metricMaker) {
+ Field<String> field = Field.ofString("hook");
+ latency = metricMaker.newTimer("latency",
+ new Description("Time spent executing a hook")
+ .setCumulative()
+ .setUnit(Description.Units.MILLISECONDS),
+ field);
+ count = metricMaker.newCounter("count",
+ new Description("Hook executions")
+ .setRate(),
+ field);
+ error = metricMaker.newCounter("error",
+ new Description("Hook execution errors")
+ .setRate(),
+ field);
+ timeout = metricMaker.newCounter("timeout",
+ new Description("Hook execution timeouts")
+ .setRate(),
+ field);
+ }
+
+ public Timer1.Context start(String name) {
+ return latency.start(name);
+ }
+
+ public void count(String name) {
+ count.increment(name);
+ }
+
+ public void error(String name) {
+ error.increment(name);
+ }
+
+ public void timeout(String name) {
+ timeout.increment(name);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
index f722034..6e87d3d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
@@ -17,6 +17,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.io.ByteStreams;
+import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -42,6 +43,7 @@
private final String projectName;
private final Path hook;
private final List<String> args;
+ private final HookMetrics metrics;
private StringWriter output;
private Process ps;
@@ -79,6 +81,7 @@
this.projectName = projectName;
this.hook = hook;
this.args = args.get();
+ this.metrics = args.metrics();
}
public void cancel() {
@@ -95,7 +98,9 @@
public HookResult runHook() {
HookResult result = null;
- try {
+ String name = getName();
+ try (Timer1.Context timer = metrics.start(name)) {
+ metrics.count(name);
List<String> argv = new ArrayList<>(1 + args.size());
argv.add(hook.toAbsolutePath().toString());
argv.addAll(args);
@@ -122,7 +127,9 @@
result = new HookResult(ps.exitValue(), output);
} catch (InterruptedException iex) {
// InterruptedException - timeout or cancel
+ metrics.timeout(name);
} catch (Throwable err) {
+ metrics.error(name);
log.error("Error running hook " + hook.toAbsolutePath(), err);
}