Merge pull request #8198 from vespa-engine/7

7 MERGEOK

5 files changed, 3 insertions, 119 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 26db07f9ed7..a445230769b 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
@@ -28,13 +28,14 @@ import java.util.logging.Logger;
  * This class is a bridge between Jetty's {@link org.eclipse.jetty.server.handler.RequestLogHandler}
  * and our own configurable access logging in different formats provided by {@link AccessLog}.
  *
- * @author bakksjo
+ * @author Oyvind Bakksjo
  * @author bjorncs
  */
 public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog {
 
     private static final Logger logger = Logger.getLogger(AccessLogRequestLog.class.getName());
 
+    // TODO These hardcoded headers should be provided by config instead
     private static final String HEADER_NAME_X_FORWARDED_FOR = "x-forwarded-for";
     private static final String HEADER_NAME_Y_RA = "y-ra";
     private static final String HEADER_NAME_Y_RP = "y-rp";
@@ -83,7 +84,6 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
     public static void populateAccessLogEntryFromHttpServletRequest(
             final HttpServletRequest request,
             final AccessLogEntry accessLogEntry) {
-        setUriFromRequest(request, accessLogEntry);
 
         accessLogEntry.setRawPath(request.getRequestURI());
         String queryString = request.getQueryString();
@@ -135,53 +135,4 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
                 .map(Integer::valueOf)
                 .orElseGet(request::getRemotePort);
     }
-
-    @SuppressWarnings("deprecation")
-    private static void setUriFromRequest(HttpServletRequest request, AccessLogEntry accessLogEntry) {
-        tryCreateUriFromRequest(request)
-                .ifPresent(accessLogEntry::setURI); // setURI is deprecated
-    }
-
-    // This is a mess and does not work correctly
-    private static Optional<URI> tryCreateUriFromRequest(HttpServletRequest request) {
-        final String quotedQuery = request.getQueryString();
-        final String quotedPath = request.getRequestURI();
-        try {
-            final StringBuilder uriBuffer = new StringBuilder();
-            uriBuffer.append(quotedPath);
-            if (quotedQuery != null) {
-                uriBuffer.append('?').append(quotedQuery);
-            }
-            return Optional.of(new URI(uriBuffer.toString()));
-        } catch (URISyntaxException e) {
-            return setUriFromMalformedInput(quotedPath, quotedQuery);
-        }
-    }
-
-    private static Optional<URI> setUriFromMalformedInput(final String quotedPath, final String quotedQuery) {
-        try {
-            final String scheme = null;
-            final String authority = null;
-            final String fragment = null;
-            return Optional.of(new URI(scheme, authority, unquote(quotedPath), unquote(quotedQuery), fragment));
-        } catch (URISyntaxException e) {
-            // I have no idea how this can happen here now...
-            logger.log(Level.WARNING, "Could not convert String URI to URI object", e);
-            return Optional.empty();
-        }
-    }
-
-    private static String unquote(final String quotedQuery) {
-        if (quotedQuery == null) {
-            return null;
-        }
-        try {
-            // inconsistent handling of semi-colon added here...
-            return URLDecoder.decode(quotedQuery, StandardCharsets.UTF_8.name());
-        } catch (IllegalArgumentException e) {
-            return quotedQuery;
-        } catch (UnsupportedEncodingException e) {
-            throw new RuntimeException(e); // should not happen
-        }
-    }
 }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
index 95f26e8bc1b..617e081bd24 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
@@ -38,7 +38,7 @@ class HttpRequestFactory {
     }
 
     public static URI getUri(HttpServletRequest servletRequest) {
-        String query = extraQuote(servletRequest.getQueryString());
+        String query = servletRequest.getQueryString();
         try {
             return URI.create(servletRequest.getRequestURL() + (query != null ? '?' + query : ""));
         } catch (IllegalArgumentException e) {
@@ -59,54 +59,6 @@ class HttpRequestFactory {
         }
     }
 
-    // TODO Remove this ugly, non-complete escaping in Vespa 7
-    private static String extraQuote(String queryString) {
-        // TODO: Use an URI builder
-        if (queryString == null) return null;
-
-        int toAndIncluding = -1;
-        for (int i = 0; i < queryString.length(); ++i) {
-            if (quote(queryString.charAt(i)) != null) {
-                break;
-            }
-            toAndIncluding = i;
-        }
-
-        String washed;
-        if (toAndIncluding != (queryString.length() - 1)) {
-            StringBuilder w = new StringBuilder(queryString.substring(0, toAndIncluding + 1));
-            for (int i = toAndIncluding + 1; i < queryString.length(); ++i) {
-                String s = quote(queryString.charAt(i));
-                if (s == null) {
-                    w.append(queryString.charAt(i));
-                } else {
-                    w.append(s);
-                }
-            }
-            washed = w.toString();
-        } else {
-            washed = queryString;
-        }
-        return washed;
-    }
-
-    private static String quote(char c) {
-        switch(c) {
-        case '\\':
-            return "%5C";
-        case '^':
-            return "%5E";
-        case '{':
-            return "%7B";
-        case '|':
-            return "%7C";
-        case '}':
-            return "%7D";
-        default:
-            return null;
-        }
-    }
-
     private static X509Certificate[] getCertChain(HttpServletRequest servletRequest) {
         return (X509Certificate[]) servletRequest.getAttribute("javax.servlet.request.X509Certificate");
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
index 2f5fe7612c8..20c8f945b82 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
@@ -84,7 +84,6 @@ class JDiscHttpServlet extends HttpServlet {
      * Override to set connector attribute before the request becomes an upgrade request in the web socket case.
      * (After the upgrade, the HttpConnection is no longer available.)
      */
-    @SuppressWarnings("deprecation")
     @Override
     protected void service(HttpServletRequest request, HttpServletResponse response) 
             throws ServletException, IOException {
@@ -93,7 +92,6 @@ class JDiscHttpServlet extends HttpServlet {
         Metric.Context metricContext = getMetricContext(request);
         context.metric.add(JettyHttpServer.Metrics.NUM_REQUESTS, 1, metricContext);
         context.metric.add(JettyHttpServer.Metrics.JDISC_HTTP_REQUESTS, 1, metricContext);
-        context.metric.add(JettyHttpServer.Metrics.MANHATTAN_NUM_REQUESTS, 1, metricContext);
 
         if (JETTY_UNSUPPORTED_METHODS.contains(request.getMethod().toUpperCase())) {
             dispatchHttpRequest(request, response);
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 40be93f2111..07d3d77dff2 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -66,7 +66,6 @@ import java.util.stream.Collectors;
  * @author bjorncs
  */
 @Beta
-// TODO Vespa 7: Remove unused Manhattan metrics
 public class JettyHttpServer extends AbstractServerProvider {
 
     public interface Metrics {
@@ -83,15 +82,12 @@ public class JettyHttpServer extends AbstractServerProvider {
 
         String NUM_BYTES_RECEIVED = "serverBytesReceived";
         String NUM_BYTES_SENT     = "serverBytesSent";
-        @Deprecated String MANHATTAN_NUM_BYTES_RECEIVED = "http.in.bytes";
-        @Deprecated String MANHATTAN_NUM_BYTES_SENT     = "http.out.bytes";
 
         String NUM_CONNECTIONS = "serverNumConnections";
 
         /* For historical reasons, these are all aliases for the same metric. 'jdisc.http' should ideally be the only one. */
         String JDISC_HTTP_REQUESTS = "jdisc.http.requests";
         String NUM_REQUESTS = "serverNumRequests";
-        @Deprecated String MANHATTAN_NUM_REQUESTS = "http.requests";
 
         String NUM_SUCCESSFUL_RESPONSES = "serverNumSuccessfulResponses";
         String NUM_FAILED_RESPONSES = "serverNumFailedResponses";
@@ -99,10 +95,8 @@ public class JettyHttpServer extends AbstractServerProvider {
         String NUM_FAILED_WRITES = "serverNumFailedResponseWrites";
 
         String TOTAL_SUCCESSFUL_LATENCY = "serverTotalSuccessfulResponseLatency";
-        @Deprecated String MANHATTAN_TOTAL_SUCCESSFUL_LATENCY = "http.latency";
         String TOTAL_FAILED_LATENCY = "serverTotalFailedResponseLatency";
         String TIME_TO_FIRST_BYTE = "serverTimeToFirstByte";
-        @Deprecated String MANHATTAN_TIME_TO_FIRST_BYTE = "http.out.firstbytetime";
 
         String RESPONSES_1XX = "http.status.1xx";
         String RESPONSES_2XX = "http.status.2xx";
@@ -113,7 +107,6 @@ public class JettyHttpServer extends AbstractServerProvider {
         String RESPONSES_403 = "http.status.403";
 
         String STARTED_MILLIS = "serverStartedMillis";
-        @Deprecated String MANHATTAN_STARTED_MILLIS = "proc.uptime";
 
         String URI_LENGTH = "jdisc.http.request.uri_length";
         String CONTENT_SIZE = "jdisc.http.request.content_size";
@@ -350,11 +343,9 @@ public class JettyHttpServer extends AbstractServerProvider {
 
     }
 
-    @SuppressWarnings("deprecation")
     private void setServerMetrics(HttpResponseStatisticsCollector statisticsCollector) {
         long timeSinceStarted = System.currentTimeMillis() - timeStarted;
         metric.set(Metrics.STARTED_MILLIS, timeSinceStarted, null);
-        metric.set(Metrics.MANHATTAN_STARTED_MILLIS, timeSinceStarted, null);
 
         addResponseMetrics(statisticsCollector);
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricReporter.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricReporter.java
index 4b01a475842..21a64792731 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricReporter.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricReporter.java
@@ -29,22 +29,18 @@ public class MetricReporter {
         this.requestStartTime = requestStartTime;
     }
 
-    @SuppressWarnings("deprecation")
     public void successfulWrite(int numBytes) {
         setTimeToFirstByteFirstTime();
 
         metric.add(Metrics.NUM_SUCCESSFUL_WRITES, 1, context);
         metric.set(Metrics.NUM_BYTES_SENT, numBytes, context);
-        metric.set(Metrics.MANHATTAN_NUM_BYTES_SENT, numBytes, context);
     }
 
-    @SuppressWarnings("deprecation")
     private void setTimeToFirstByteFirstTime() {
         boolean isFirstWrite = firstSetOfTimeToFirstByte.getAndSet(false);
         if (isFirstWrite) {
             long timeToFirstByte = getRequestLatency();
             metric.set(Metrics.TIME_TO_FIRST_BYTE, timeToFirstByte, context);
-            metric.set(Metrics.MANHATTAN_TIME_TO_FIRST_BYTE, timeToFirstByte, context);
         }
     }
 
@@ -52,14 +48,12 @@ public class MetricReporter {
         metric.add(Metrics.NUM_FAILED_WRITES, 1, context);
     }
 
-    @SuppressWarnings("deprecation")
     public void successfulResponse() {
         setTimeToFirstByteFirstTime();
 
         long requestLatency = getRequestLatency();
 
         metric.set(Metrics.TOTAL_SUCCESSFUL_LATENCY, requestLatency, context);
-        metric.set(Metrics.MANHATTAN_TOTAL_SUCCESSFUL_LATENCY, requestLatency, context);
 
         metric.add(Metrics.NUM_SUCCESSFUL_RESPONSES, 1, context);
     }
@@ -75,10 +69,8 @@ public class MetricReporter {
         metric.add(Metrics.NUM_PREMATURELY_CLOSED_CONNECTIONS, 1, context);
     }
 
-    @SuppressWarnings("deprecation")
     public void successfulRead(int bytes_received) {
         metric.set(JettyHttpServer.Metrics.NUM_BYTES_RECEIVED, bytes_received, context);
-        metric.set(JettyHttpServer.Metrics.MANHATTAN_NUM_BYTES_RECEIVED, bytes_received, context);
     }
 
     private long getRequestLatency() {