Merge pull request #8306 from vespa-engine/bjorncs/vespa-http-client

Add client version as header to all http requests (MERGEOK)

5 files changed, 57 insertions, 31 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
index a445230769b..f72f0691deb 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
@@ -4,7 +4,6 @@ package com.yahoo.jdisc.http.server.jetty;
 import com.google.common.base.Objects;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.AccessLogEntry;
-
 import com.yahoo.jdisc.http.servlet.ServletRequest;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.RequestLog;
@@ -12,14 +11,9 @@ import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
 
 import javax.servlet.http.HttpServletRequest;
-
-import java.io.UnsupportedEncodingException;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
 import java.security.Principal;
 import java.security.cert.X509Certificate;
+import java.util.List;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -42,6 +36,9 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
     private static final String HEADER_NAME_YAHOOREMOTEIP = "yahooremoteip";
     private static final String HEADER_NAME_CLIENT_IP = "client-ip";
 
+    // HTTP headers that are logged as extra key-value-pairs in access log entries
+    private static final List<String> LOGGED_REQUEST_HEADERS = List.of("Vespa-Client-Version");
+
     private final AccessLog accessLog;
 
     public AccessLogRequestLog(final AccessLog accessLog) {
@@ -68,6 +65,13 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
             accessLogEntry.setReturnedContentSize(response.getHttpChannel().getBytesWritten());
             accessLogEntry.setStatusCode(response.getCommittedMetaData().getStatus());
 
+            LOGGED_REQUEST_HEADERS.forEach(header -> {
+                String value = request.getHeader(header);
+                if (value != null) {
+                    accessLogEntry.addKeyValue(header, value);
+                }
+            });
+
             accessLog.log(accessLogEntry);
         } catch (Exception e) {
             // Catching any exceptions here as it is unclear how Jetty handles exceptions from a RequestLog.
@@ -81,6 +85,8 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
      * time rather than at logging time. We may, for example, want to set things such as http headers and ip
      * addresses up-front and make it illegal for request handlers to modify these later.
      */
+    // TODO Move populating of access log entry to log(). populateAccessLogEntryFromHttpServletRequest() is not guaranteed
+    //  to be called if request is failed early (e.g invalid uri or header encoding).
     public static void populateAccessLogEntryFromHttpServletRequest(
             final HttpServletRequest request,
             final AccessLogEntry accessLogEntry) {
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Headers.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Headers.java
index 3791ddc5462..9cb42356209 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Headers.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Headers.java
@@ -11,6 +11,8 @@ public final class Headers {
     private Headers() {
     }
 
+    public static final String CLIENT_VERSION = "Vespa-Client-Version";
+
     public static final String TIMEOUT = "X-Yahoo-Feed-Timeout";
     public static final String DRAIN = "X-Yahoo-Feed-Drain";
     public static final String ROUTE = "X-Yahoo-Feed-Route";
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index 57007743b1b..fd228fb99c5 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -26,6 +26,7 @@ import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.message.BasicHeader;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -411,7 +412,8 @@ class ApacheGatewayConnection implements GatewayConnection {
                 clientBuilder.setConnectionManager(connMgr);
 
             }
-            clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.currentVersion));
+            clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.currentVersion.toFullString()));
+            clientBuilder.setDefaultHeaders(List.of(new BasicHeader(Headers.CLIENT_VERSION, Vtag.currentVersion.toFullString())));
             clientBuilder.setMaxConnPerRoute(1);
             clientBuilder.setMaxConnTotal(1);
             clientBuilder.disableContentCompression();
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java
index 28fcd027d2b..4dbe1381e34 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentApiMetrics.java
@@ -1,6 +1,8 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.documentapi.metrics;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.yahoo.metrics.simple.Counter;
 import com.yahoo.metrics.simple.Gauge;
 import com.yahoo.metrics.simple.MetricReceiver;
@@ -10,6 +12,7 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 
 
 /**
@@ -21,7 +24,9 @@ public class DocumentApiMetrics {
 
     private final Counter feeds;
     private final Gauge feedLatency;
+    private final Counter feedRequests;
     private final Map<DocumentOperationStatus, Map<DocumentOperationType, Point>> points = new HashMap<>();
+    private final Cache<String, Point> versionPointCache = CacheBuilder.newBuilder().maximumSize(256).build();
 
     public DocumentApiMetrics(MetricReceiver metricReceiver, String apiName) {
         Map<String, String> dimensions = new HashMap<>();
@@ -37,6 +42,7 @@ public class DocumentApiMetrics {
 
         feeds = metricReceiver.declareCounter("feed.operations");
         feedLatency = metricReceiver.declareGauge("feed.latency");
+        feedRequests = metricReceiver.declareCounter("feed.http-requests");
     }
 
     public void reportSuccessful(DocumentOperationType documentOperationType, double latencyInSeconds) {
@@ -56,4 +62,17 @@ public class DocumentApiMetrics {
         feeds.add(point);
     }
 
+    public void reportHttpRequest(String clientVersion) {
+        if (clientVersion != null) {
+            try {
+                Point point = versionPointCache.get(clientVersion, () -> new Point(Map.of("client-version", clientVersion)));
+                feedRequests.add(point);
+            } catch (ExecutionException e) { // When Point constructor throws an exception
+                throw new RuntimeException(e);
+            }
+        } else {
+            feedRequests.add();
+        }
+    }
+
 }
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 bd7d195b48b..6cddba59a6c 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
@@ -2,46 +2,27 @@
 package com.yahoo.vespa.http.server;
 
 import com.yahoo.collections.Tuple2;
-import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.container.handler.ThreadpoolConfig;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.container.jdisc.messagebus.SessionCache;
-import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
 import com.yahoo.documentapi.metrics.DocumentApiMetrics;
-import com.yahoo.jdisc.Metric;
-import com.yahoo.jdisc.http.HttpResponse.Status;
-import com.yahoo.log.LogLevel;
 import com.yahoo.messagebus.ReplyHandler;
-import com.yahoo.messagebus.SourceSessionParams;
 import com.yahoo.metrics.simple.MetricReceiver;
-import com.yahoo.net.HostName;
 import com.yahoo.vespa.http.client.core.Headers;
-import com.yahoo.vespa.http.client.core.OperationStatus;
-import com.yahoo.yolean.Exceptions;
 
 import javax.inject.Inject;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.ThreadLocalRandom;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.zip.GZIPInputStream;
 
 /**
@@ -53,7 +34,9 @@ public class FeedHandler extends LoggingRequestHandler {
 
     protected final ReplyHandler feedReplyHandler;
     private static final List<Integer> serverSupportedVersions = Collections.unmodifiableList(Arrays.asList(3));
+    private static final Pattern USER_AGENT_PATTERN = Pattern.compile("vespa-http-client \\((.+)\\)");
     private final FeedHandlerV3 feedHandlerV3;
+    private final DocumentApiMetrics metricsHelper;
 
     @Inject
     public FeedHandler(
@@ -63,7 +46,7 @@ public class FeedHandler extends LoggingRequestHandler {
             ThreadpoolConfig threadpoolConfig,
             MetricReceiver metricReceiver) throws Exception {
         super(parentCtx);
-        DocumentApiMetrics metricsHelper = new DocumentApiMetrics(metricReceiver, "vespa.http.server");
+        metricsHelper = new DocumentApiMetrics(metricReceiver, "vespa.http.server");
         feedHandlerV3 = new FeedHandlerV3(parentCtx, documentManagerConfig, sessionCache, threadpoolConfig, metricsHelper);
         feedReplyHandler = new FeedReplyReader(parentCtx.getMetric(), metricsHelper);
     }
@@ -121,6 +104,7 @@ public class FeedHandler extends LoggingRequestHandler {
 
     @Override
     public HttpResponse handle(HttpRequest request) {
+        metricsHelper.reportHttpRequest(findClientVersion(request).orElse(null));
         Tuple2<HttpResponse, Integer> protocolVersion = checkProtocolVersion(request);
 
         if (protocolVersion.first != null) {
@@ -129,6 +113,19 @@ public class FeedHandler extends LoggingRequestHandler {
         return feedHandlerV3.handle(request);
     }
 
+    private static Optional<String> findClientVersion(HttpRequest request) {
+        String versionHeader = request.getHeader(Headers.CLIENT_VERSION);
+        if (versionHeader != null) {
+            return Optional.of(versionHeader);
+        }
+        String userAgentHeader = request.getHeader("User-Agent");
+        Matcher matcher = USER_AGENT_PATTERN.matcher(userAgentHeader);
+        if (matcher.matches()) {
+            return Optional.of(matcher.group(1));
+        }
+        return Optional.empty();
+    }
+
     // Protected for testing
     protected static InputStream unzipStreamIfNeeded(InputStream inputStream, HttpRequest httpRequest)
             throws IOException {