Add metric for update-doc-not-found, and update httpapi metrics from /doc/v1

5 files changed, 70 insertions, 22 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 a102e8fffd4..2a7e69a0bf4 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
@@ -34,6 +34,7 @@ import com.yahoo.documentapi.DocumentAccess;
 import com.yahoo.documentapi.DocumentOperationParameters;
 import com.yahoo.documentapi.DocumentResponse;
 import com.yahoo.documentapi.ProgressToken;
+import com.yahoo.documentapi.Response.Outcome;
 import com.yahoo.documentapi.Result;
 import com.yahoo.documentapi.VisitorControlHandler;
 import com.yahoo.documentapi.VisitorDataHandler;
@@ -65,6 +66,7 @@ import com.yahoo.restapi.Path;
 import com.yahoo.search.query.ParameterParser;
 import com.yahoo.text.Text;
 import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig;
+import com.yahoo.vespa.http.server.MetricNames;
 import com.yahoo.yolean.Exceptions;
 import com.yahoo.yolean.Exceptions.RunnableThrowingIOException;
 
@@ -93,8 +95,6 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Phaser;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
@@ -450,7 +450,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                 DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                         .withResponseHandler(response -> {
                             outstanding.decrementAndGet();
-                            handle(path, handler, response);
+                            updatePutMetrics(response.outcome());
+                            handleFeedOperation(path, handler, response);
                         });
                 return () -> dispatchOperation(() -> asyncSession.put(put, parameters));
             });
@@ -467,7 +468,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                 DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                         .withResponseHandler(response -> {
                             outstanding.decrementAndGet();
-                            handle(path, handler, response);
+                            updateUpdateMetrics(response.outcome());
+                            handleFeedOperation(path, handler, response);
                         });
                 return () -> dispatchOperation(() -> asyncSession.update(update, parameters));
             });
@@ -482,7 +484,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
             DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE)
                     .withResponseHandler(response -> {
                         outstanding.decrementAndGet();
-                        handle(path, handler, response);
+                        updateRemoveMetrics(response.outcome());
+                        handleFeedOperation(path, handler, response);
                     });
             return () -> dispatchOperation(() -> asyncSession.remove(remove, parameters));
         });
@@ -1032,10 +1035,35 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
         }
     }
 
-    private static void handle(DocumentPath path, ResponseHandler handler, com.yahoo.documentapi.Response response) {
+    private static void handleFeedOperation(DocumentPath path, ResponseHandler handler, com.yahoo.documentapi.Response response) {
         handle(path, handler, response, (document, jsonResponse) -> jsonResponse.commit(Response.Status.OK));
     }
 
+    private void updatePutMetrics(Outcome outcome) {
+        switch (outcome) {
+            case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break;
+            case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break;
+            default: metric.add(MetricNames.FAILED, 1, null); break;
+        }
+    }
+
+    private void updateUpdateMetrics(Outcome outcome) {
+        switch (outcome) {
+            case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break;
+            case NOT_FOUND: metric.add(MetricNames.NOT_FOUND, 1, null); break;
+            case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break;
+            default: metric.add(MetricNames.FAILED, 1, null); break;
+        }
+    }
+
+    private void updateRemoveMetrics(Outcome outcome) {
+        switch (outcome) {
+            case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break;
+            case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break;
+            default: metric.add(MetricNames.FAILED, 1, null); break;
+        }
+    }
+
     // ------------------------------------------------- Visits ------------------------------------------------
 
     private VisitorParameters parseGetParameters(HttpRequest request, DocumentPath path, boolean streamed) {
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 522652212a9..97e5e9e63d8 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
@@ -19,15 +19,6 @@ 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;
-        }
-    }
-
     public static DocumentOperationStatus fromMessageBusErrorCodes(Set<Integer> errorCodes) {
         if (errorCodes.size() == 1 && errorCodes.contains(DocumentProtocol.ERROR_NO_SPACE))
             return SERVER_ERROR;
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 50f79c0a828..b71b5d79520 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.http.server;
 
 import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol;
+import com.yahoo.documentapi.messagebus.protocol.UpdateDocumentReply;
 import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.documentapi.metrics.DocumentOperationStatus;
 import com.yahoo.documentapi.metrics.DocumentOperationType;
@@ -54,14 +55,20 @@ public class FeedReplyReader implements ReplyHandler {
         } else {
             metricsHelper.reportSuccessful(type, latencyInSeconds);
             metric.add(MetricNames.SUCCEEDED, 1, null);
-            if (!conditionMet)
+            if ( ! conditionMet)
                 metric.add(MetricNames.CONDITION_NOT_MET, 1, testAndSetMetricCtx);
+            if ( ! updateNotFound(reply))
+                metric.add(MetricNames.NOT_FOUND, 1, null);
             enqueue(context, "Document processed.", ErrorCode.OK, !conditionMet, reply.getTrace());
         }
     }
 
     private static boolean conditionMet(Reply reply) {
-        return !reply.hasErrors() || reply.getError(0).getCode() != DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED;
+        return ! reply.hasErrors() || reply.getError(0).getCode() != DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED;
+    }
+
+    private static boolean updateNotFound(Reply reply) {
+        return reply instanceof UpdateDocumentReply && ! ((UpdateDocumentReply) reply).wasFound();
     }
 
     private void enqueue(ReplyContext context, String message, ErrorCode status, boolean isConditionNotMet, Trace trace) {
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java
index 6ded410ff68..a5987f2398e 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java
@@ -19,6 +19,7 @@ public final class MetricNames {
     public static final String LATENCY = PREFIX + "latency";
     public static final String FAILED = PREFIX + "failed";
     public static final String CONDITION_NOT_MET = PREFIX + "condition_not_met";
+    public static final String NOT_FOUND = PREFIX + "not_found";
     public static final String PARSE_ERROR = PREFIX + "parse_error";
     public static final String SUCCEEDED = PREFIX + "succeeded";
     public static final String PENDING = PREFIX + "pending";
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
index de907f70c19..232d49129d7 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
@@ -3,7 +3,6 @@ package com.yahoo.document.restapi.resource;
 
 import com.yahoo.cloud.config.ClusterListConfig;
 import com.yahoo.container.jdisc.RequestHandlerTestDriver;
-import com.yahoo.docproc.jdisc.metric.NullMetric;
 import com.yahoo.document.BucketId;
 import com.yahoo.document.Document;
 import com.yahoo.document.DocumentId;
@@ -44,6 +43,7 @@ import com.yahoo.documentapi.VisitorResponse;
 import com.yahoo.documentapi.VisitorSession;
 import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage;
 import com.yahoo.jdisc.Metric;
+import com.yahoo.jdisc.test.MockMetric;
 import com.yahoo.messagebus.StaticThrottlePolicy;
 import com.yahoo.messagebus.Trace;
 import com.yahoo.messagebus.TraceNode;
@@ -126,7 +126,7 @@ public class DocumentV1ApiTest {
                                                                                       Map.of("music", "default")));
     ManualClock clock;
     MockDocumentAccess access;
-    Metric metric;
+    MockMetric metric;
     MetricReceiver metrics;
     DocumentV1ApiHandler handler;
 
@@ -134,7 +134,7 @@ public class DocumentV1ApiTest {
     public void setUp() {
         clock = new ManualClock();
         access = new MockDocumentAccess(docConfig);
-        metric = new NullMetric();
+        metric = new MockMetric();
         metrics = new MetricReceiver.MockReceiver();
         handler = new DocumentV1ApiHandler(clock, Duration.ofMillis(1), metric, metrics, access, docConfig,
                                            executorConfig, clusterConfig, bucketConfig);
@@ -614,6 +614,23 @@ public class DocumentV1ApiTest {
                        "}", response.readAll());
         assertEquals(400, response.getStatus());
 
+        // PUT on document which is not found is a 200
+        access.session.expect((update, parameters) -> {
+            parameters.responseHandler().get().handleResponse(new UpdateResponse(0, false));
+            return new Result(Result.ResultType.SUCCESS, null);
+        });
+        response = driver.sendRequest("http://localhost/document/v1/space/music/docid/sonny", PUT,
+                                      "{" +
+                                      "  \"fields\": {" +
+                                      "    \"artist\": { \"assign\": \"The Shivers\" }" +
+                                      "  }" +
+                                      "}");
+        assertSameJson("{" +
+                       "  \"pathId\": \"/document/v1/space/music/docid/sonny\"," +
+                       "  \"id\": \"id:space:music::sonny\"" +
+                       "}", response.readAll());
+        assertEquals(200, response.getStatus());
+
         // DELETE with full document ID is a document remove operation.
         access.session.expect((remove, parameters) -> {
             DocumentRemove expectedRemove = new DocumentRemove(doc2.getId());
@@ -667,7 +684,7 @@ public class DocumentV1ApiTest {
             parameters.responseHandler().get().handleResponse(new Response(0, "disk full", Response.Outcome.INSUFFICIENT_STORAGE));
             return new Result(Result.ResultType.SUCCESS, null);
         });
-        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two");
+        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE);
         assertSameJson("{" +
                        "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
                        "  \"id\": \"id:space:music:n=1:two\"," +
@@ -680,7 +697,7 @@ public class DocumentV1ApiTest {
             parameters.responseHandler().get().handleResponse(new Response(0, "no dice", Response.Outcome.CONDITION_FAILED));
             return new Result(Result.ResultType.SUCCESS, null);
         });
-        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two");
+        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE);
         assertSameJson("{" +
                        "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
                        "  \"id\": \"id:space:music:n=1:two\"," +
@@ -746,6 +763,10 @@ public class DocumentV1ApiTest {
                 handler.get().handleResponse(new Response(0));  // response may eventually arrive, but too late.
         }
 
+        assertEquals(3, metric.metrics().get("httpapi_succeeded").get(Map.of()), 0);
+        assertEquals(1, metric.metrics().get("httpapi_condition_not_met").get(Map.of()), 0);
+        assertEquals(1, metric.metrics().get("httpapi_not_found").get(Map.of()), 0);
+        assertEquals(1, metric.metrics().get("httpapi_failed").get(Map.of()), 0);
         driver.close();
     }