Improve metrics for /document/v1

Make more consistent with legacy feed api

1 files changed, 45 insertions, 24 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
index 10040ab6d0f..5b28f975564 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
@@ -462,7 +462,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                 DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                         .withResponseHandler(response -> {
                             outstanding.decrementAndGet();
-                            updatePutMetrics(response.outcome());
+                            updatePutMetrics(response.outcome(), latencyOf(request));
                             handleFeedOperation(path, put.fullyApplied(), handler, response);
                         });
                 return () -> dispatchOperation(() -> asyncSession.put((DocumentPut)put.operation(), parameters));
@@ -486,7 +486,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                 DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                         .withResponseHandler(response -> {
                             outstanding.decrementAndGet();
-                            updateUpdateMetrics(response.outcome(), update.getCreateIfNonExistent());
+                            updateUpdateMetrics(response.outcome(), latencyOf(request), update.getCreateIfNonExistent());
                             handleFeedOperation(path, parsed.fullyApplied(), handler, response);
                         });
                 return () -> dispatchOperation(() -> asyncSession.update(update, parameters));
@@ -507,7 +507,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
             DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                     .withResponseHandler(response -> {
                         outstanding.decrementAndGet();
-                        updateRemoveMetrics(response.outcome());
+                        updateRemoveMetrics(response.outcome(), latencyOf(request));
                         handleFeedOperation(path, true, handler, response);
                     });
             return () -> dispatchOperation(() -> asyncSession.remove(remove, parameters));
@@ -1090,32 +1090,50 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
         handle(path, null, handler, response, (document, jsonResponse) -> jsonResponse.commit(Response.Status.OK, fullyApplied));
     }
 
-    private void updatePutMetrics(Outcome outcome) {
+    private static double latencyOf(HttpRequest r) { return (System.nanoTime() - r.relativeCreatedAtNanoTime()) / 1e+9d; }
+
+    private void updatePutMetrics(Outcome outcome, double latency) {
+        incrementMetricNumOperations(); incrementMetricNumPuts(); sampleLatency(latency);
         switch (outcome) {
-            case SUCCESS -> metric.add(MetricNames.SUCCEEDED, 1, null);
-            case CONDITION_FAILED -> metric.add(MetricNames.CONDITION_NOT_MET, 1, null);
-            default -> metric.add(MetricNames.FAILED, 1, null);
+            case SUCCESS -> incrementMetricSucceeded();
+            case CONDITION_FAILED -> { incrementMetricSucceeded(); incrementMetricConditionNotMet(); }
+            default -> incrementMetricFailed();
         }
     }
 
-    private void updateUpdateMetrics(Outcome outcome, boolean create) {
+    private void updateUpdateMetrics(Outcome outcome, double latency, boolean create) {
         if (create && outcome == Outcome.NOT_FOUND) outcome = Outcome.SUCCESS; // >_<
+        incrementMetricNumOperations(); incrementMetricNumUpdates(); sampleLatency(latency);
         switch (outcome) {
-            case SUCCESS -> metric.add(MetricNames.SUCCEEDED, 1, null);
-            case NOT_FOUND -> metric.add(MetricNames.NOT_FOUND, 1, null);
-            case CONDITION_FAILED -> metric.add(MetricNames.CONDITION_NOT_MET, 1, null);
-            default -> metric.add(MetricNames.FAILED, 1, null);
+            case SUCCESS -> incrementMetricSucceeded();
+            case NOT_FOUND -> { incrementMetricSucceeded(); incrementMetricNotFound(); }
+            case CONDITION_FAILED -> { incrementMetricSucceeded(); incrementMetricConditionNotMet(); }
+            default -> incrementMetricFailed();
         }
     }
 
-    private void updateRemoveMetrics(Outcome outcome) {
+    private void updateRemoveMetrics(Outcome outcome, double latency) {
+        incrementMetricNumOperations(); incrementMetricNumRemoves(); sampleLatency(latency);
         switch (outcome) {
-            case SUCCESS, NOT_FOUND -> metric.add(MetricNames.SUCCEEDED, 1, null);
-            case CONDITION_FAILED -> metric.add(MetricNames.CONDITION_NOT_MET, 1, null);
-            case INSUFFICIENT_STORAGE, TIMEOUT, ERROR -> metric.add(MetricNames.FAILED, 1, null);
+            case SUCCESS -> incrementMetricSucceeded();
+            case NOT_FOUND -> { incrementMetricSucceeded(); incrementMetricNotFound(); }
+            case CONDITION_FAILED -> { incrementMetricSucceeded(); incrementMetricConditionNotMet(); }
+            default -> incrementMetricFailed();
         }
     }
 
+    private void sampleLatency(double latency) { setMetric(MetricNames.LATENCY, latency); }
+    private void incrementMetricNumOperations() { incrementMetric(MetricNames.NUM_OPERATIONS); }
+    private void incrementMetricNumPuts() { incrementMetric(MetricNames.NUM_PUTS); }
+    private void incrementMetricNumRemoves() { incrementMetric(MetricNames.NUM_REMOVES); }
+    private void incrementMetricNumUpdates() { incrementMetric(MetricNames.NUM_UPDATES); }
+    private void incrementMetricFailed() { incrementMetric(MetricNames.FAILED); }
+    private void incrementMetricConditionNotMet() { incrementMetric(MetricNames.CONDITION_NOT_MET); }
+    private void incrementMetricSucceeded() { incrementMetric(MetricNames.SUCCEEDED); }
+    private void incrementMetricNotFound() { incrementMetric(MetricNames.NOT_FOUND); }
+    private void incrementMetric(String n) { metric.add(n, 1, null); }
+    private void setMetric(String n, Number v) { metric.set(n, v, null); }
+
     // ------------------------------------------------- Visits ------------------------------------------------
 
     private VisitorParameters parseGetParameters(HttpRequest request, DocumentPath path, boolean streamed) {
@@ -1446,17 +1464,20 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
 
         @Override
         public ContentChannel handleResponse(Response response) {
-            var statusCodeGroup = response.getStatus() / 100;
-            // Status code 412 - condition not met - is considered OK
-            if (statusCodeGroup == 2 || response.getStatus() == 412)
-                metrics.reportSuccessful(type, start);
-            else if (statusCodeGroup == 4)
-                metrics.reportFailure(type, DocumentOperationStatus.REQUEST_ERROR);
-            else if (statusCodeGroup == 5)
-                metrics.reportFailure(type, DocumentOperationStatus.SERVER_ERROR);
+            switch (response.getStatus()) {
+                case 200 -> report(DocumentOperationStatus.OK);
+                case 400 -> report(DocumentOperationStatus.REQUEST_ERROR);
+                case 404 -> report(DocumentOperationStatus.OK, DocumentOperationStatus.NOT_FOUND);
+                case 412 -> report(DocumentOperationStatus.OK, DocumentOperationStatus.CONDITION_FAILED);
+                case 429 -> report(DocumentOperationStatus.TOO_MANY_REQUESTS);
+                case 500,502,503,504,507 -> report(DocumentOperationStatus.SERVER_ERROR);
+                default -> throw new IllegalStateException("Unexpected status code '%s'".formatted(response.getStatus()));
+            }
             return delegate.handleResponse(response);
         }
 
+        private void report(DocumentOperationStatus... status) { metrics.report(type, start, status); }
+
     }
 
     static class StorageCluster {