Collect metrics for parsing failures

5 files changed, 77 insertions, 51 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/ClientFeederV3.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/ClientFeederV3.java
index 35ff2e33b5d..c3e42a585e5 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/ClientFeederV3.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/ClientFeederV3.java
@@ -187,6 +187,8 @@ class ClientFeederV3 {
                 if (log.isLoggable(LogLevel.DEBUG)) {
                     log.log(LogLevel.DEBUG, Exceptions.toMessageString(e), e);
                 }
+                metric.add(MetricNames.PARSE_ERROR, 1, null);
+
                 repliesFromOldMessages.add(new OperationStatus(
                         Exceptions.toMessageString(e), operationId.get(), ErrorCode.ERROR, false, ""));
 
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 39f5808d2dd..6b52a81323d 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
@@ -18,6 +18,7 @@ public final class MetricNames {
     public static final String OPERATIONS_PER_SEC = PREFIX + "ops_per_sec";
     public static final String LATENCY = PREFIX + "latency";
     public static final String FAILED = PREFIX + "failed";
+    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/feedhandler/v3/CollectingMetric.java b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/CollectingMetric.java
new file mode 100644
index 00000000000..fb04172eb79
--- /dev/null
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/CollectingMetric.java
@@ -0,0 +1,38 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.feedhandler.v3;
+
+import com.yahoo.jdisc.Metric;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * @author ollivir
+ */
+public final class CollectingMetric implements Metric {
+    private final Context DUMMY_CONTEXT = new Context() {};
+    private final Map<String, AtomicLong> values = new ConcurrentHashMap<>();
+
+    public CollectingMetric() {}
+
+    @Override
+    public void set(String key, Number val, Context ctx) {
+        values.computeIfAbsent(key, ignored -> new AtomicLong(0)).set(val.longValue());
+    }
+
+    @Override
+    public void add(String key, Number val, Context ctx) {
+        values.computeIfAbsent(key, ignored -> new AtomicLong(0)).addAndGet(val.longValue());
+    }
+
+    public long get(String key) {
+        return Optional.ofNullable(values.get(key)).map(AtomicLong::get).orElse(0L);
+    }
+
+    @Override
+    public Context createContext(Map<String, ?> properties) {
+        return DUMMY_CONTEXT;
+    }
+}
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 4f58e628974..6452dafc38c 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
@@ -2,6 +2,7 @@
 package com.yahoo.feedhandler.v3;
 
 import com.google.common.base.Splitter;
+import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.messagebus.SessionCache;
 import com.yahoo.container.logging.AccessLog;
@@ -12,6 +13,7 @@ import com.yahoo.document.config.DocumentmanagerConfig;
 import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage;
 import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.jdisc.ReferencedResource;
+import com.yahoo.messagebus.Result;
 import com.yahoo.messagebus.SourceSessionParams;
 import com.yahoo.messagebus.shared.SharedSourceSession;
 import com.yahoo.metrics.simple.MetricReceiver;
@@ -20,10 +22,12 @@ import com.yahoo.vespa.http.client.config.FeedParams;
 import com.yahoo.vespa.http.client.core.ErrorCode;
 import com.yahoo.vespa.http.client.core.Headers;
 import com.yahoo.vespa.http.client.core.OperationStatus;
-import com.yahoo.vespa.http.server.ReplyContext;
 import com.yahoo.vespa.http.server.FeedHandlerV3;
+import com.yahoo.vespa.http.server.MetricNames;
+import com.yahoo.vespa.http.server.ReplyContext;
 import org.junit.Test;
-import com.yahoo.container.jdisc.HttpRequest;
+import org.mockito.Mockito;
+import org.mockito.stubbing.Answer;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -32,15 +36,14 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.startsWith;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.anyObject;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
-import com.yahoo.messagebus.Result;
-import org.mockito.Mockito;
-import org.mockito.stubbing.Answer;
 
 public class FeedTesterV3 {
+    final CollectingMetric metric = new CollectingMetric();
 
     @Test
     public void feedOneDocument() throws Exception {
@@ -50,7 +53,17 @@ public class FeedTesterV3 {
         httpResponse.render(outStream);
         assertThat(httpResponse.getContentType(), is("text/plain"));
         assertThat(Utf8.toString(outStream.toByteArray()), is("1230 OK message trace\n"));
+    }
 
+    @Test
+    public void feedOneBrokenDocument() throws Exception {
+        final FeedHandlerV3 feedHandlerV3 = setupFeederHandler();
+        HttpResponse httpResponse = feedHandlerV3.handle(createBrokenRequest());
+        ByteArrayOutputStream outStream = new ByteArrayOutputStream();
+        httpResponse.render(outStream);
+        assertThat(httpResponse.getContentType(), is("text/plain"));
+        assertThat(Utf8.toString(outStream.toByteArray()), startsWith("1230 ERROR "));
+        assertThat(metric.get(MetricNames.PARSE_ERROR), is(1L));
     }
 
     @Test
@@ -64,7 +77,7 @@ public class FeedTesterV3 {
         assertThat(Splitter.on("\n").splitToList(result).size(), is(101));
     }
 
-    DocumentTypeManager createDoctypeManager() {
+    private static DocumentTypeManager createDoctypeManager() {
         DocumentTypeManager docTypeManager = new DocumentTypeManager();
         DocumentType documentType = new DocumentType("testdocument");
         documentType.addField("title", DataType.STRING);
@@ -73,34 +86,41 @@ public class FeedTesterV3 {
         return docTypeManager;
     }
 
-    HttpRequest createRequest(int numberOfDocs) {
-        String clientId = "client123";
+    private static HttpRequest createRequest(int numberOfDocs) {
         StringBuilder wireData = new StringBuilder();
         for (int x = 0; x < numberOfDocs; x++) {
             String docData = "[{\"put\": \"id:testdocument:testdocument::c\", \"fields\": { \"title\": \"fooKey\", \"body\": \"value\"}}]";
             String operationId = "123" + x;
             wireData.append(operationId + " " + Integer.toHexString(docData.length()) + "\n" + docData);
         }
-        InputStream inputStream =  new ByteArrayInputStream(wireData.toString().getBytes());
-        HttpRequest request = HttpRequest.createTestRequest(
-                "http://dummyhostname:19020/reserved-for-internal-use/feedapi",
-                com.yahoo.jdisc.http.HttpRequest.Method.POST,
-                inputStream);
+        return createRequestWithPayload(wireData.toString());
+    }
+
+    private static HttpRequest createBrokenRequest() {
+        String docData = "[{\"put oops I broke it]";
+        String wireData = "1230 " + Integer.toHexString(docData.length()) + "\n" + docData;
+        return createRequestWithPayload(wireData);
+    }
+
+    private static HttpRequest createRequestWithPayload(String payload) {
+        InputStream inputStream = new ByteArrayInputStream(payload.getBytes());
+        HttpRequest request = HttpRequest.createTestRequest("http://dummyhostname:19020/reserved-for-internal-use/feedapi",
+                com.yahoo.jdisc.http.HttpRequest.Method.POST, inputStream);
         request.getJDiscRequest().headers().add(Headers.VERSION, "3");
         request.getJDiscRequest().headers().add(Headers.DATA_FORMAT, FeedParams.DataFormat.JSON_UTF8.name());
         request.getJDiscRequest().headers().add(Headers.TIMEOUT, "1000000000");
-        request.getJDiscRequest().headers().add(Headers.CLIENT_ID, clientId);
+        request.getJDiscRequest().headers().add(Headers.CLIENT_ID, "client123");
         request.getJDiscRequest().headers().add(Headers.PRIORITY, "LOWEST");
         request.getJDiscRequest().headers().add(Headers.TRACE_LEVEL, "4");
         request.getJDiscRequest().headers().add(Headers.DRAIN, "true");
         return request;
     }
 
-    FeedHandlerV3 setupFeederHandler() throws Exception {
+    private FeedHandlerV3 setupFeederHandler() throws Exception {
         Executor threadPool = Executors.newCachedThreadPool();
         DocumentmanagerConfig docMan = new DocumentmanagerConfig(new DocumentmanagerConfig.Builder().enablecompression(true));
         FeedHandlerV3 feedHandlerV3 = new FeedHandlerV3(
-                new FeedHandlerV3.Context(threadPool, AccessLog.voidAccessLog(), new NullFeedMetric(true)),
+                new FeedHandlerV3.Context(threadPool, AccessLog.voidAccessLog(), metric),
                 docMan,
                 null /* session cache */,
                 null /* thread pool config */,
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/NullFeedMetric.java b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/NullFeedMetric.java
deleted file mode 100644
index 4777c6c7b99..00000000000
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/NullFeedMetric.java
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.feedhandler.v3;
-
-import com.yahoo.jdisc.Metric;
-import java.util.Map;
-
-/**
- * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
- * @since 5.1.20
- */
-public final class NullFeedMetric implements Metric {
-
-    public NullFeedMetric(boolean flag) {
-        if (!flag) {
-            throw new IllegalArgumentException("must set flag allowing to throw away metrics");
-        }
-    }
-
-    @Override
-    public void set(String key, Number val, Context ctx) {
-    }
-
-    @Override
-    public void add(String key, Number val, Context ctx) {
-    }
-
-    @Override
-    public Context createContext(Map<String, ?> properties) {
-        return NullFeedContext.INSTANCE;
-    }
-
-    private static class NullFeedContext implements Context {
-        private static final NullFeedContext INSTANCE = new NullFeedContext();
-    }
-}