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); }