Revert "Revert "Access log optimizations [run-systemtest]""

1 files changed, 47 insertions, 33 deletions

diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java
index a4fd7c9bc5f..c9b466517b3 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java
@@ -1,8 +1,9 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http.server.jetty;
 
-import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.AccessLogEntry;
+import com.yahoo.container.logging.RequestLog;
+import com.yahoo.container.logging.RequestLogEntry;
 import com.yahoo.jdisc.http.ServerConfig;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.server.HttpChannel;
@@ -19,6 +20,7 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -30,103 +32,115 @@ import static org.mockito.Mockito.when;
 public class AccessLogRequestLogTest {
     @Test
     public void requireThatQueryWithUnquotedSpecialCharactersIsHandled() {
-        final AccessLogEntry accessLogEntry = new AccessLogEntry();
-        final Request jettyRequest = createRequestMock(accessLogEntry);
+        final Request jettyRequest = createRequestMock();
         when(jettyRequest.getRequestURI()).thenReturn("/search/");
         when(jettyRequest.getQueryString()).thenReturn("query=year:>2010");
 
-        doAccessLoggingOfRequest(jettyRequest);
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
 
-        assertThat(accessLogEntry.getRawPath(), is(not(nullValue())));
-        assertTrue(accessLogEntry.getRawQuery().isPresent());
+        assertThat(entry.rawPath().get(), is(not(nullValue())));
+        assertTrue(entry.rawQuery().isPresent());
     }
 
     @Test
     public void requireThatDoubleQuotingIsNotPerformed() {
-        final AccessLogEntry accessLogEntry = new AccessLogEntry();
-        final Request jettyRequest = createRequestMock(accessLogEntry);
+        final Request jettyRequest = createRequestMock();
         final String path = "/search/";
         when(jettyRequest.getRequestURI()).thenReturn(path);
         final String query = "query=year%252010+%3B&customParameter=something";
         when(jettyRequest.getQueryString()).thenReturn(query);
 
-        doAccessLoggingOfRequest(jettyRequest);
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
 
-        assertThat(accessLogEntry.getRawPath(), is(path));
-        assertThat(accessLogEntry.getRawQuery().get(), is(query));
+        assertThat(entry.rawPath().get(), is(path));
+        assertThat(entry.rawQuery().get(), is(query));
 
     }
 
     @Test
     public void raw_path_and_query_are_set_from_request() {
-        AccessLogEntry accessLogEntry = new AccessLogEntry();
-        Request jettyRequest = createRequestMock(accessLogEntry);
+        Request jettyRequest = createRequestMock();
         String rawPath = "//search/";
         when(jettyRequest.getRequestURI()).thenReturn(rawPath);
         String rawQuery = "q=%%2";
         when(jettyRequest.getQueryString()).thenReturn(rawQuery);
 
-        doAccessLoggingOfRequest(jettyRequest);
-        assertThat(accessLogEntry.getRawPath(), is(rawPath));
-        Optional<String> actualRawQuery = accessLogEntry.getRawQuery();
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
+        assertThat(entry.rawPath().get(), is(rawPath));
+        Optional<String> actualRawQuery = entry.rawQuery();
         assertThat(actualRawQuery.isPresent(), is(true));
         assertThat(actualRawQuery.get(), is(rawQuery));
     }
 
     @Test
     public void verify_x_forwarded_for_precedence () {
-        AccessLogEntry accessLogEntry = new AccessLogEntry();
-        Request jettyRequest = createRequestMock(accessLogEntry);
+        Request jettyRequest = createRequestMock();
         when(jettyRequest.getRequestURI()).thenReturn("//search/");
         when(jettyRequest.getQueryString()).thenReturn("q=%%2");
         when(jettyRequest.getHeader("x-forwarded-for")).thenReturn("1.2.3.4");
         when(jettyRequest.getHeader("y-ra")).thenReturn("2.3.4.5");
 
-        doAccessLoggingOfRequest(jettyRequest);
-        assertThat(accessLogEntry.getRemoteAddress(), is("1.2.3.4"));
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
+        assertThat(entry.remoteAddress().get(), is("1.2.3.4"));
     }
 
     @Test
     public void verify_x_forwarded_port_precedence () {
-        AccessLogEntry accessLogEntry = new AccessLogEntry();
-        Request jettyRequest = createRequestMock(accessLogEntry);
+        Request jettyRequest = createRequestMock();
         when(jettyRequest.getRequestURI()).thenReturn("//search/");
         when(jettyRequest.getQueryString()).thenReturn("q=%%2");
         when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("80");
         when(jettyRequest.getHeader("y-rp")).thenReturn("8080");
 
-        doAccessLoggingOfRequest(jettyRequest);
-        assertThat(accessLogEntry.getRemotePort(), is(80));
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
+        assertThat(entry.remotePort().getAsInt(), is(80));
     }
 
     @Test
     public void defaults_to_peer_port_if_remote_port_header_is_invalid() {
-        final AccessLogEntry accessLogEntry = new AccessLogEntry();
-        final Request jettyRequest = createRequestMock(accessLogEntry);
+        final Request jettyRequest = createRequestMock();
         when(jettyRequest.getRequestURI()).thenReturn("/search/");
         when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("8o8o");
         when(jettyRequest.getRemotePort()).thenReturn(80);
 
-        doAccessLoggingOfRequest(jettyRequest);
-        assertThat(accessLogEntry.getRemotePort(), is(0));
-        assertThat(accessLogEntry.getPeerPort(), is(80));
+        InMemoryRequestLog requestLog = new InMemoryRequestLog();
+        doAccessLoggingOfRequest(requestLog, jettyRequest);
+        RequestLogEntry entry = requestLog.entries().get(0);
+        assertFalse(entry.remotePort().isPresent());
+        assertThat(entry.peerPort().getAsInt(), is(80));
     }
 
-    private void doAccessLoggingOfRequest(Request jettyRequest) {
+    private void doAccessLoggingOfRequest(RequestLog requestLog, Request jettyRequest) {
         ServerConfig.AccessLog config = new ServerConfig.AccessLog(
                 new ServerConfig.AccessLog.Builder()
                         .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra"))
                         .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp")));
-        new AccessLogRequestLog(mock(AccessLog.class), config).log(jettyRequest, createResponseMock());
+        new AccessLogRequestLog(requestLog, config).log(jettyRequest, createResponseMock());
     }
 
-    private static Request createRequestMock(AccessLogEntry entry) {
+    private static Request createRequestMock() {
         ServerConnector serverConnector = mock(ServerConnector.class);
         when(serverConnector.getLocalPort()).thenReturn(1234);
         HttpConnection httpConnection = mock(HttpConnection.class);
         when(httpConnection.getConnector()).thenReturn(serverConnector);
         Request request = mock(Request.class);
-        when(request.getAttribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY)).thenReturn(entry);
+        when(request.getMethod()).thenReturn("GET");
+        when(request.getRemoteAddr()).thenReturn("localhost");
+        when(request.getRemotePort()).thenReturn(12345);
+        when(request.getProtocol()).thenReturn("HTTP/1.1");
+        when(request.getScheme()).thenReturn("http");
+        when(request.getTimeStamp()).thenReturn(0L);
+        when(request.getAttribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY)).thenReturn(new AccessLogEntry());
         when(request.getAttribute("org.eclipse.jetty.server.HttpConnection")).thenReturn(httpConnection);
         return request;
     }