Add metrics for ssh requests
- break them down by ssh command name
- include aggregate counts and errors by exception type
Release-Notes: Add metrics for ssh requests
Change-Id: I92ad23c67284f8215b21f57345ba2051f9eb86ab
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index a510e25..903c635 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -321,6 +321,17 @@
** `view`:
view implementation class
+=== SSH
+
+* `ssh/success_count`: Rate of successful SSH requests
+** `command_name`:
+ Name of the SSH command
+* `ssh/error_count`: Rate of SSH error responses
+** `command_name`:
+ Name of the SSH command
+** `exception`:
+ Name of the exception which has caused the request to fail.
+
=== Query
* `query/query_latency`: Successful query latency, accumulated over the life
diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index abcc108..8907fae 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -324,7 +324,7 @@
.orElseThrow(
() -> new RuntimeException(String.format("project %s not found", req.project)));
- AsyncReceiveCommits arc = factory.create(projectState, identifiedUser, db, null);
+ AsyncReceiveCommits arc = factory.create(projectState, identifiedUser, db, null, null);
if (arc.canUpload() != Capable.OK) {
throw new ServiceNotAuthorizedException();
}
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 1537655..b90ef35 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -534,7 +534,7 @@
}
AsyncReceiveCommits arc =
- factory.create(state, userProvider.get().asIdentifiedUser(), db, null);
+ factory.create(state, userProvider.get().asIdentifiedUser(), db, null, null);
ReceivePack rp = arc.getReceivePack();
req.setAttribute(ATT_ARC, arc);
return rp;
diff --git a/java/com/google/gerrit/server/RequestCounter.java b/java/com/google/gerrit/server/RequestCounter.java
new file mode 100644
index 0000000..444521a
--- /dev/null
+++ b/java/com/google/gerrit/server/RequestCounter.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2024 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.google.gerrit.server;
+
+import com.google.gerrit.common.Nullable;
+
+public interface RequestCounter {
+ /**
+ * Count a request.
+ *
+ * @param requestInfo information about the request
+ * @param error The exception which caused the request to fail, or null if the request was
+ * successful.
+ */
+ void countRequest(RequestInfo requestInfo, @Nullable Throwable error);
+}
diff --git a/java/com/google/gerrit/server/RequestInfo.java b/java/com/google/gerrit/server/RequestInfo.java
index 927985d8..6457b38 100644
--- a/java/com/google/gerrit/server/RequestInfo.java
+++ b/java/com/google/gerrit/server/RequestInfo.java
@@ -85,6 +85,13 @@
return requestUri().map(RequestInfo::redactRequestUri);
}
+ /**
+ * The command name of the SSH command.
+ *
+ * <p>Only set if request type is {@link RequestType#SSH}.
+ */
+ public abstract Optional<String> commandName();
+
/** The user that has sent the request. */
public abstract CurrentUser callingUser();
@@ -164,6 +171,18 @@
return builder().requestType(requestType).callingUser(callingUser).traceContext(traceContext);
}
+ public static RequestInfo.Builder builder(
+ RequestType requestType,
+ String commandName,
+ CurrentUser callingUser,
+ TraceContext traceContext) {
+ return builder()
+ .requestType(requestType)
+ .commandName(commandName)
+ .callingUser(callingUser)
+ .traceContext(traceContext);
+ }
+
@UsedAt(UsedAt.Project.GOOGLE)
public static RequestInfo.Builder builder() {
return new AutoValue_RequestInfo.Builder();
@@ -191,6 +210,8 @@
return this;
}
+ public abstract Builder commandName(String commandName);
+
public abstract Builder callingUser(CurrentUser callingUser);
public abstract Builder traceContext(TraceContext traceContext);
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index cb1af07..1d8aa05 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -36,6 +36,7 @@
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PublishCommentsOp;
+import com.google.gerrit.server.RequestCounter;
import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ConfigUtil;
@@ -102,7 +103,8 @@
ProjectState projectState,
IdentifiedUser user,
Repository repository,
- @Nullable MessageSender messageSender);
+ @Nullable MessageSender messageSender,
+ @Nullable RequestCounter requestCounter);
}
public static class AsyncReceiveCommitsModule extends PrivateModule {
@@ -259,7 +261,8 @@
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted Repository repo,
- @Assisted @Nullable MessageSender messageSender)
+ @Assisted @Nullable MessageSender messageSender,
+ @Assisted @Nullable RequestCounter requestCounter)
throws PermissionBackendException {
this.multiProgressMonitorFactory = multiProgressMonitorFactory;
this.executor = executor;
@@ -305,7 +308,8 @@
projectName,
user.getAccountId()));
receiveCommits =
- factory.create(projectState, user, receivePack, repo, allRefsWatcher, messageSender);
+ factory.create(
+ projectState, user, receivePack, repo, allRefsWatcher, messageSender, requestCounter);
receiveCommits.init();
QuotaResponse.Aggregated availableTokens =
quotaBackend.user(user).project(projectName).availableTokens(REPOSITORY_SIZE_GROUP);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 7544f95..19e2253 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -119,6 +119,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.PublishCommentsOp;
+import com.google.gerrit.server.RequestCounter;
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.Sequences;
@@ -288,7 +289,8 @@
ReceivePack receivePack,
Repository repository,
AllRefsWatcher allRefsWatcher,
- MessageSender messageSender);
+ @Nullable MessageSender messageSender,
+ @Nullable RequestCounter requestCounter);
}
private class ReceivePackMessageSender implements MessageSender {
@@ -430,6 +432,7 @@
private final SetHashtagsOp.Factory hashtagsFactory;
private final SetTopicOp.Factory setTopicFactory;
private final ServiceUserClassifier serviceUserClassifier;
+ private final RequestCounter requestCounter;
private final ImmutableList<SubmissionListener> superprojectUpdateSubmissionListeners;
private final TagCache tagCache;
private final ProjectConfig.Factory projectConfigFactory;
@@ -535,7 +538,8 @@
@Assisted ReceivePack rp,
@Assisted Repository repository,
@Assisted AllRefsWatcher allRefsWatcher,
- @Nullable @Assisted MessageSender messageSender)
+ @Assisted @Nullable MessageSender messageSender,
+ @Assisted @Nullable RequestCounter requestCounter)
throws IOException {
// Injected fields.
this.accountResolver = accountResolver;
@@ -579,6 +583,7 @@
this.receiveConfig = receiveConfig;
this.refValidatorsFactory = refValidatorsFactory;
this.replaceOpFactory = replaceOpFactory;
+ this.requestCounter = requestCounter;
this.requestListeners = requestListeners;
this.retryHelper = retryHelper;
this.requestScopePropagator = requestScopePropagator;
@@ -705,7 +710,8 @@
TraceTimer traceTimer =
newTimer("processCommands", Metadata.builder().resourceCount(commandCount))) {
RequestInfo requestInfo =
- RequestInfo.builder(RequestInfo.RequestType.GIT_RECEIVE, user, traceContext)
+ RequestInfo.builder(
+ RequestInfo.RequestType.GIT_RECEIVE, "git-receive-pack", user, traceContext)
.project(project.getNameKey())
.build();
requestListeners.runEach(l -> l.onRequest(requestInfo));
@@ -721,6 +727,7 @@
commands =
commands.stream().map(c -> wrapReceiveCommand(c, commandProgress)).collect(toList());
+ Throwable error = null;
try (RequestStateContext requestStateContext =
RequestStateContext.open()
.addRequestStateProvider(progress)
@@ -731,9 +738,11 @@
commands,
RejectionReason.create(MetricBucket.INTERNAL_SERVER_ERROR, INTERNAL_SERVER_ERROR));
} catch (InvalidDeadlineException e) {
+ error = e;
rejectRemaining(
commands, RejectionReason.create(MetricBucket.INVALID_DEADLINE, e.getMessage()));
} catch (RuntimeException e) {
+ error = e;
Optional<RequestCancelledException> requestCancelledException =
RequestCancelledException.getFromCausalChain(e);
if (!requestCancelledException.isPresent()) {
@@ -763,6 +772,10 @@
}
rejectRemaining(commands, RejectionReason.create(metricBucket, msg.toString()));
+ } finally {
+ if (requestCounter != null) {
+ requestCounter.countRequest(requestInfo, error);
+ }
}
// This sends error messages before the 'done' string of the progress monitor is sent.
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 33a49ae..60464e5 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -76,6 +76,9 @@
/** The cause of an error. */
public abstract Optional<String> cause();
+ /** The command name of an SSH request. */
+ public abstract Optional<String> commandName();
+
/** Side where the comment is written: <= 0 for parent, 1 for revision. */
public abstract Optional<Integer> commentSide();
@@ -88,6 +91,9 @@
/** The type of an event. */
public abstract Optional<String> eventType();
+ /** The name of an exception which failed an SSH request. */
+ public abstract Optional<String> exception();
+
/** The value of the @Export annotation which was used to register a plugin extension. */
public abstract Optional<String> exportValue();
@@ -192,8 +198,8 @@
* authDomainName=Optional.empty, branchName=Optional.empty, cacheKey=Optional.empty,
* cacheName=Optional.empty, caller=Optional.empty, className=Optional.empty,
* cancellationReason=Optional.empty, changeId=Optional[9212550], changeIdType=Optional.empty,
- * cause=Optional.empty, diffAlgorithm=Optional.empty, eventType=Optional.empty,
- * exportValue=Optional.empty, filePath=Optional.empty, garbageCollectorName=Optional.empty,
+ * cause=Optional.empty, commandName=Optional.empty, diffAlgorithm=Optional.empty, eventType=Optional.empty,
+ * exception=Optional.empty, exportValue=Optional.empty, filePath=Optional.empty, garbageCollectorName=Optional.empty,
* gitOperation=Optional.empty, groupId=Optional.empty, groupName=Optional.empty,
* groupUuid=Optional.empty, httpStatus=Optional.empty, indexName=Optional.empty,
* indexVersion=Optional[0], methodName=Optional.empty, multiple=Optional.empty,
@@ -235,10 +241,12 @@
.add("changeId", changeId().orElse(null))
.add("changeIdType", changeIdType().orElse(null))
.add("cause", cause().orElse(null))
+ .add("commandName", commandName().orElse(null))
.add("commentSide", commentSide().orElse(null))
.add("commit", commit().orElse(null))
.add("diffAlgorithm", diffAlgorithm().orElse(null))
.add("eventType", eventType().orElse(null))
+ .add("exception", exception().orElse(null))
.add("exportValue", exportValue().orElse(null))
.add("filePath", filePath().orElse(null))
.add("garbageCollectorName", garbageCollectorName().orElse(null))
@@ -314,6 +322,8 @@
public abstract Builder cause(@Nullable String cause);
+ public abstract Builder commandName(@Nullable String commandName);
+
public abstract Builder commentSide(int side);
public abstract Builder commit(@Nullable String commit);
@@ -322,6 +332,8 @@
public abstract Builder eventType(@Nullable String eventType);
+ public abstract Builder exception(@Nullable String exception);
+
public abstract Builder exportValue(@Nullable String exportValue);
public abstract Builder filePath(@Nullable String filePath);
diff --git a/java/com/google/gerrit/sshd/SshCommand.java b/java/com/google/gerrit/sshd/SshCommand.java
index a4e427d..359adae 100644
--- a/java/com/google/gerrit/sshd/SshCommand.java
+++ b/java/com/google/gerrit/sshd/SshCommand.java
@@ -46,6 +46,7 @@
@Inject @GerritServerConfig private Config config;
@Inject private DeadlineChecker.Factory deadlineCheckerFactory;
@Inject private CancellationMetrics cancellationMetrics;
+ @Inject private SshMetrics sshMetrics;
@Option(name = "--trace", usage = "enable request tracing")
private boolean trace;
@@ -72,7 +73,9 @@
PerformanceLogContext performanceLogContext =
new PerformanceLogContext(config, performanceLoggers)) {
RequestInfo requestInfo =
- RequestInfo.builder(RequestInfo.RequestType.SSH, user, traceContext).build();
+ RequestInfo.builder(RequestInfo.RequestType.SSH, getName(), user, traceContext)
+ .build();
+ Throwable error = null;
try (RequestStateContext requestStateContext =
RequestStateContext.open()
.addRequestStateProvider(
@@ -80,8 +83,10 @@
requestListeners.runEach(l -> l.onRequest(requestInfo));
SshCommand.this.run();
} catch (InvalidDeadlineException e) {
+ error = e;
stderr.println(e.getMessage());
} catch (RuntimeException e) {
+ error = e;
Optional<RequestCancelledException> requestCancelledException =
RequestCancelledException.getFromCausalChain(e);
if (!requestCancelledException.isPresent()) {
@@ -97,6 +102,8 @@
" (%s)", requestCancelledException.get().getCancellationMessage().get()));
}
stderr.println(msg.toString());
+ } finally {
+ sshMetrics.countRequest(requestInfo, error);
}
} finally {
stdout.flush();
diff --git a/java/com/google/gerrit/sshd/SshMetrics.java b/java/com/google/gerrit/sshd/SshMetrics.java
new file mode 100644
index 0000000..2e1ade3
--- /dev/null
+++ b/java/com/google/gerrit/sshd/SshMetrics.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2024 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.google.gerrit.sshd;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Counter2;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.server.RequestCounter;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+@Singleton
+public class SshMetrics implements RequestCounter {
+ private final Counter1<String> count;
+ private final Counter2<String, String> errorCount;
+
+ @Inject
+ SshMetrics(MetricMaker metrics) {
+ this.count =
+ metrics.newCounter(
+ "ssh/success_count",
+ new Description("Count of successful ssh requests").setRate(),
+ Field.ofString("command_name", Metadata.Builder::commandName)
+ .description("The command name of the request.")
+ .build());
+
+ this.errorCount =
+ metrics.newCounter(
+ "ssh/error_count",
+ new Description("Number of failed requests").setRate(),
+ Field.ofString("command_name", Metadata.Builder::commandName)
+ .description("The command name of the request.")
+ .build(),
+ Field.ofString("exception", Metadata.Builder::exception)
+ .description("Exception that failed the request.")
+ .build());
+ }
+
+ @Override
+ public void countRequest(RequestInfo requestInfo, @Nullable Throwable error) {
+ if (error == null) {
+ count.increment(requestInfo.commandName().get());
+ } else {
+ errorCount.increment(requestInfo.commandName().get(), error.getClass().getSimpleName());
+ }
+ }
+}
diff --git a/java/com/google/gerrit/sshd/commands/Receive.java b/java/com/google/gerrit/sshd/commands/Receive.java
index 3f2e2ad..f34b08b 100644
--- a/java/com/google/gerrit/sshd/commands/Receive.java
+++ b/java/com/google/gerrit/sshd/commands/Receive.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.sshd.AbstractGitCommand;
import com.google.gerrit.sshd.CommandMetaData;
+import com.google.gerrit.sshd.SshMetrics;
import com.google.inject.Inject;
import java.io.IOException;
import org.eclipse.jgit.errors.TooLargeObjectInPackException;
@@ -45,6 +46,7 @@
@Inject private AsyncReceiveCommits.Factory factory;
@Inject private PermissionBackend permissionBackend;
+ @Inject private SshMetrics sshMetrics;
private final SetMultimap<ReviewerStateInternal, Account.Id> reviewers =
MultimapBuilder.hashKeys(2).hashSetValues().build();
@@ -82,7 +84,7 @@
}
AsyncReceiveCommits arc =
- factory.create(projectState, currentUser.asIdentifiedUser(), repo, null);
+ factory.create(projectState, currentUser.asIdentifiedUser(), repo, null, sshMetrics);
try {
Capable r = arc.canUpload();
diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java
index b80b879..7977895 100644
--- a/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/java/com/google/gerrit/sshd/commands/Upload.java
@@ -33,6 +33,7 @@
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.sshd.AbstractGitCommand;
+import com.google.gerrit.sshd.SshMetrics;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.List;
@@ -54,6 +55,7 @@
@Inject private UploadValidators.Factory uploadValidatorsFactory;
@Inject private PermissionBackend permissionBackend;
@Inject private UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook;
+ @Inject private SshMetrics sshMetrics;
private PackStatistics stats;
@@ -93,21 +95,30 @@
try (TraceContext traceContext = TraceContext.open();
TracingHook tracingHook = new TracingHook()) {
RequestInfo requestInfo =
- RequestInfo.builder(RequestInfo.RequestType.GIT_UPLOAD, user, traceContext)
+ RequestInfo.builder(RequestInfo.RequestType.GIT_UPLOAD, getName(), user, traceContext)
.project(projectState.getNameKey())
.build();
- requestListeners.runEach(l -> l.onRequest(requestInfo));
- up.setProtocolV2Hook(tracingHook);
- up.upload(in, out, err);
- session.setPeerAgent(up.getPeerUserAgent());
- stats = up.getStatistics();
- } catch (UploadValidationException e) {
- // UploadValidationException is used by the UploadValidators to
- // stop the uploadPack. We do not want this exception to go beyond this
- // point otherwise it would print a stacktrace in the logs and return an
- // internal server error to the client.
- if (!e.isOutput()) {
- up.sendMessage(e.getMessage());
+ Throwable error = null;
+ try {
+ requestListeners.runEach(l -> l.onRequest(requestInfo));
+ up.setProtocolV2Hook(tracingHook);
+ up.upload(in, out, err);
+ session.setPeerAgent(up.getPeerUserAgent());
+ stats = up.getStatistics();
+ } catch (UploadValidationException e) {
+ error = e;
+ // UploadValidationException is used by the UploadValidators to
+ // stop the uploadPack. We do not want this exception to go beyond this
+ // point otherwise it would print a stacktrace in the logs and return an
+ // internal server error to the client.
+ if (!e.isOutput()) {
+ up.sendMessage(e.getMessage());
+ }
+ } catch (RuntimeException e) {
+ error = e;
+ throw e;
+ } finally {
+ sshMetrics.countRequest(requestInfo, error);
}
}
}
