Code review fixes

8 files changed, 44 insertions, 41 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java
index 218d1779bb5..dbcb55cfd06 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java
@@ -16,7 +16,7 @@ import com.yahoo.documentapi.VisitorParameters;
 import com.yahoo.documentapi.VisitorSession;
 import com.yahoo.documentapi.messagebus.MessageBusSyncSession;
 import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.documentapi.metrics.DocumentOperationStatus;
 import com.yahoo.documentapi.metrics.DocumentOperationType;
 import com.yahoo.messagebus.StaticThrottlePolicy;
@@ -32,6 +32,7 @@ import org.apache.commons.lang3.exception.ExceptionUtils;
 
 import java.io.ByteArrayOutputStream;
 import java.nio.charset.StandardCharsets;
+import java.time.Instant;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -49,7 +50,7 @@ public class OperationHandlerImpl implements OperationHandler {
 
     public static final int VISIT_TIMEOUT_MS = 120000;
     private final DocumentAccess documentAccess;
-    private final DocumentApiMetricsHelper metricsHelper;
+    private final DocumentApiMetrics metricsHelper;
     private final ClusterEnumerator clusterEnumerator;
 
     private static final class SyncSessionFactory extends ResourceFactory<SyncSession> {
@@ -73,7 +74,7 @@ public class OperationHandlerImpl implements OperationHandler {
         this.documentAccess = documentAccess;
         this.clusterEnumerator = clusterEnumerator;
         syncSessions = new ConcurrentResourcePool<>(new SyncSessionFactory(documentAccess));
-        metricsHelper = new DocumentApiMetricsHelper(metricReceiver, "documentV1");
+        metricsHelper = new DocumentApiMetrics(metricReceiver, "documentV1");
     }
 
     @Override
@@ -188,7 +189,7 @@ public class OperationHandlerImpl implements OperationHandler {
         SyncSession syncSession = syncSessions.alloc();
         Response response;
         try {
-            long startTime = System.currentTimeMillis();
+            Instant startTime = Instant.now();
             DocumentPut put = new DocumentPut(data.getDocument());
             put.setCondition(data.getCondition());
             setRoute(syncSession, route);
@@ -212,7 +213,7 @@ public class OperationHandlerImpl implements OperationHandler {
         SyncSession syncSession = syncSessions.alloc();
         Response response;
         try {
-            long startTime = System.currentTimeMillis();
+            Instant startTime = Instant.now();
             setRoute(syncSession, route);
             syncSession.update(data.getDocumentUpdate());
             metricsHelper.reportSuccessful(DocumentOperationType.UPDATE, startTime);
@@ -234,7 +235,7 @@ public class OperationHandlerImpl implements OperationHandler {
         SyncSession syncSession = syncSessions.alloc();
         Response response;
         try {
-            long startTime = System.currentTimeMillis();
+            Instant startTime = Instant.now();
             DocumentId id = new DocumentId(restUri.generateFullId());
             DocumentRemove documentRemove = new DocumentRemove(id);
             setRoute(syncSession, route);
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetricsHelper.java b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java
index 154f473630e..13c007a075e 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetricsHelper.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java
@@ -6,19 +6,23 @@ import com.yahoo.metrics.simple.Gauge;
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.metrics.simple.Point;
 
+import java.time.Duration;
+import java.time.Instant;
 import java.util.HashMap;
 import java.util.Map;
 
 
 /**
+ * This class reports metrics for feed operations by APIs that use documentapi.
+ *
  * @author freva
  */
-public class DocumentApiMetricsHelper {
+public class DocumentApiMetrics {
     private final Counter feeds;
     private final Gauge feedLatency;
     private final Map<DocumentOperationStatus, Map<DocumentOperationType, Point>> points = new HashMap<>();
 
-    public DocumentApiMetricsHelper(MetricReceiver metricReceiver, String apiName) {
+    public DocumentApiMetrics(MetricReceiver metricReceiver, String apiName) {
         Map<String, String> dimensions = new HashMap<>();
         dimensions.put("api", apiName);
         for (DocumentOperationStatus status : DocumentOperationStatus.values()) {
@@ -34,15 +38,15 @@ public class DocumentApiMetricsHelper {
         feedLatency = metricReceiver.declareGauge("feed.latency");
     }
 
-    public void reportSuccessful(DocumentOperationType documentOperationType, double latency) {
+    public void reportSuccessful(DocumentOperationType documentOperationType, double latencyInSeconds) {
         Point point = points.get(DocumentOperationStatus.OK).get(documentOperationType);
 
-        feedLatency.sample(latency, point);
+        feedLatency.sample(latencyInSeconds, point);
         feeds.add(point);
     }
 
-    public void reportSuccessful(DocumentOperationType documentOperationType, long startTime) {
-        final double latency = (System.currentTimeMillis() - startTime) / 1000.0d;
+    public void reportSuccessful(DocumentOperationType documentOperationType, Instant startTime) {
+        final double latency = Duration.between(startTime, Instant.now()).toMillis() / 1000.0d;
         reportSuccessful(documentOperationType, latency);
     }
 
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java
index cdbaf645b6e..1513833eee8 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java
@@ -6,24 +6,24 @@ import com.yahoo.document.restapi.OperationHandlerImpl;
 import java.util.Set;
 
 /**
+ * Enum with possible outcomes of a single document feeding operation:
+ * <ul>
+ * <li>OK: Document was successfully added/updated/removed</li>
+ * <li>REQUEST_ERROR: User-made error, for example invalid document format</li>
+ * <li>SERVER_ERROR: Server-made error, for example insufficient disk space</li>
+ * </ul>
+ *
  * @author freva
  */
-public enum  DocumentOperationStatus {
+public enum DocumentOperationStatus {
     OK, REQUEST_ERROR, SERVER_ERROR;
 
     public static DocumentOperationStatus fromHttpStatusCode(int httpStatus) {
         switch (httpStatus / 100) {
-            case 2:
-                return OK;
-
-            case 4:
-                return REQUEST_ERROR;
-
-            case 5:
-                return SERVER_ERROR;
-
-            default:
-                return null;
+            case 2: return OK;
+            case 4: return REQUEST_ERROR;
+            case 5: return SERVER_ERROR;
+            default: return null;
         }
     }
 
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
index acb12d291ba..0412f630469 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
@@ -11,7 +11,7 @@ import com.yahoo.container.jdisc.messagebus.SessionCache;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.http.HttpResponse.Status;
 import com.yahoo.log.LogLevel;
@@ -69,7 +69,7 @@ public class FeedHandler extends LoggingRequestHandler {
             ThreadpoolConfig threadpoolConfig,
             MetricReceiver metricReceiver) throws Exception {
         super(executor, accessLog);
-        DocumentApiMetricsHelper metricsHelper = new DocumentApiMetricsHelper(metricReceiver, "vespa.http.server");
+        DocumentApiMetrics metricsHelper = new DocumentApiMetrics(metricReceiver, "vespa.http.server");
         feedHandlerV3 = new FeedHandlerV3(executor, documentManagerConfig, sessionCache, metric, accessLog, threadpoolConfig, metricsHelper);
         docTypeManager = createDocumentManager(documentManagerConfig);
         clients = new HashMap<>();
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
index 134387f0114..2227d0ab41f 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
@@ -10,7 +10,7 @@ import com.yahoo.container.jdisc.messagebus.SessionCache;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.ReferencedResource;
 import com.yahoo.log.LogLevel;
@@ -55,7 +55,7 @@ public class FeedHandlerV3 extends LoggingRequestHandler {
             Metric metric,
             AccessLog accessLog,
             ThreadpoolConfig threadpoolConfig,
-            DocumentApiMetricsHelper metricsHelper) throws Exception {
+            DocumentApiMetrics metricsHelper) throws Exception {
         super(executor, accessLog);
         docTypeManager = new DocumentTypeManager(documentManagerConfig);
         this.sessionCache = sessionCache;
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java
index eccc1cb17e7..84a46484cfe 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java
@@ -4,9 +4,8 @@ package com.yahoo.vespa.http.server;
 import java.util.Set;
 import java.util.logging.Logger;
 
-import com.yahoo.documentapi.messagebus.MessageBusAsyncSession;
 import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.documentapi.metrics.DocumentOperationStatus;
 import com.yahoo.documentapi.metrics.DocumentOperationType;
 import com.yahoo.jdisc.Metric;
@@ -26,9 +25,9 @@ public class FeedReplyReader implements ReplyHandler {
 
     private static final Logger log = Logger.getLogger(FeedReplyReader.class.getName());
     private final Metric metric;
-    private final DocumentApiMetricsHelper metricsHelper;
+    private final DocumentApiMetrics metricsHelper;
 
-    public FeedReplyReader(Metric metric, DocumentApiMetricsHelper metricsHelper) {
+    public FeedReplyReader(Metric metric, DocumentApiMetrics metricsHelper) {
         this.metric = metric;
         this.metricsHelper = metricsHelper;
     }
@@ -40,18 +39,18 @@ public class FeedReplyReader implements ReplyHandler {
             return;
         }
         ReplyContext context = (ReplyContext) o;
-        final double latency = (System.currentTimeMillis() - context.creationTime) / 1000.0d;
-        metric.set(MetricNames.LATENCY, latency, null);
+        final double latencyInSeconds = (System.currentTimeMillis() - context.creationTime) / 1000.0d;
+        metric.set(MetricNames.LATENCY, latencyInSeconds, null);
 
         if (reply.hasErrors()) {
-            Set<Integer> errorCodes = MessageBusAsyncSession.getErrorCodes(reply);
+            Set<Integer> errorCodes = reply.getErrorCodes();
             metricsHelper.reportFailure(DocumentOperationType.fromMessage(reply.getMessage()),
                     DocumentOperationStatus.fromMessageBusErrorCodes(errorCodes));
             metric.add(MetricNames.FAILED, 1, null);
             enqueue(context, reply.getError(0).getMessage(), ErrorCode.ERROR,
                     reply.getError(0).getCode() == DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED, reply.getTrace());
         } else {
-            metricsHelper.reportSuccessful(DocumentOperationType.fromMessage(reply.getMessage()), latency);
+            metricsHelper.reportSuccessful(DocumentOperationType.fromMessage(reply.getMessage()), latencyInSeconds);
             metric.add(MetricNames.SUCCEEDED, 1, null);
             enqueue(context, "Document processed.", ErrorCode.OK, false, reply.getTrace());
         }
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
index 2a72ead0a5a..9fb7dba7257 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
@@ -10,7 +10,7 @@ import com.yahoo.document.DocumentType;
 import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
 import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.feedhandler.NullFeedMetric;
 import com.yahoo.jdisc.ReferencedResource;
 import com.yahoo.messagebus.SourceSessionParams;
@@ -39,7 +39,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import com.yahoo.messagebus.Result;
 import org.mockito.Mockito;
-import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
 public class FeedTesterV3 {
@@ -102,7 +101,7 @@ public class FeedTesterV3 {
         Executor threadPool = Executors.newCachedThreadPool();
         DocumentmanagerConfig docMan = new DocumentmanagerConfig(new DocumentmanagerConfig.Builder().enablecompression(true));
         FeedHandlerV3 feedHandlerV3 = new FeedHandlerV3(
-                threadPool, docMan, null /* session cache */ , new NullFeedMetric(), AccessLog.voidAccessLog(), null, new DocumentApiMetricsHelper(MetricReceiver.nullImplementation, "test")) {
+                threadPool, docMan, null /* session cache */ , new NullFeedMetric(), AccessLog.voidAccessLog(), null, new DocumentApiMetrics(MetricReceiver.nullImplementation, "test")) {
             @Override
             protected ReferencedResource<SharedSourceSession> retainSource(
                     SessionCache sessionCache, SourceSessionParams sessionParams)  {
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/V3CongestionTestCase.java b/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/V3CongestionTestCase.java
index 47142a49050..915a8635863 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/V3CongestionTestCase.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/V3CongestionTestCase.java
@@ -3,7 +3,7 @@ package com.yahoo.vespa.http.server;
 
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.document.DocumentTypeManager;
-import com.yahoo.documentapi.metrics.DocumentApiMetricsHelper;
+import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.ReferencedResource;
 import com.yahoo.jdisc.References;
@@ -88,7 +88,7 @@ public class V3CongestionTestCase {
                 null /*DocTypeManager*/,
                 "clientID",
                 null/*metric*/,
-                new FeedReplyReader(null/*metric*/, new DocumentApiMetricsHelper(MetricReceiver.nullImplementation, "tester")),
+                new FeedReplyReader(null/*metric*/, new DocumentApiMetrics(MetricReceiver.nullImplementation, "tester")),
                 threadsAvail);
     }