diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2021-01-21 11:04:03 +0100 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2021-01-21 13:31:52 +0100 |
commit | 4c514714e3e739c66f0107629d44ef5e5877403a (patch) | |
tree | 1831864072336b2fdc3fd53a27282c9a38800fa5 /jdisc_http_service/src | |
parent | be41683f5efb4146ec1d85e41011d61b73d9c8da (diff) |
Introduce RequestLog interface
Diffstat (limited to 'jdisc_http_service/src')
11 files changed, 92 insertions, 39 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLog.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLog.java index 5c1a549070c..f60c52f7f38 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLog.java +++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLog.java @@ -9,8 +9,9 @@ import com.yahoo.component.provider.ComponentRegistry; * Logs to all the configured access logs. * * @author Tony Vaagenes + * @author bjorncs */ -public class AccessLog { +public class AccessLog implements RequestLog { private ComponentRegistry<AccessLogInterface> implementers; @@ -23,10 +24,16 @@ public class AccessLog { return new AccessLog(new ComponentRegistry<>()); } + @Override public void log(AccessLogEntry accessLogEntry) { for (AccessLogInterface log: implementers.allComponents()) { log.log(accessLogEntry); } } + @Override + public void log(RequestLogEntry entry) { + // TODO Implement + } + } diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLog.java new file mode 100644 index 00000000000..f1036dcb880 --- /dev/null +++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLog.java @@ -0,0 +1,15 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.logging; + +/** + * Access logging for requests + * + * @author bjorncs + */ +public interface RequestLog { + + void log(AccessLogEntry entry); + + void log(RequestLogEntry entry); + +} 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 32790534f86..fb4db5a6307 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,10 +4,10 @@ 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.container.logging.RequestLog; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.servlet.ServletRequest; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.RequestLog; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.component.AbstractLifeCycle; @@ -30,19 +30,19 @@ import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnectorLoca * @author Oyvind Bakksjo * @author bjorncs */ -class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog { +class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty.server.RequestLog { private static final Logger logger = Logger.getLogger(AccessLogRequestLog.class.getName()); // 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; + private final RequestLog requestLog; private final List<String> remoteAddressHeaders; private final List<String> remotePortHeaders; - AccessLogRequestLog(AccessLog accessLog, ServerConfig.AccessLog config) { - this.accessLog = accessLog; + AccessLogRequestLog(RequestLog requestLog, ServerConfig.AccessLog config) { + this.requestLog = requestLog; this.remoteAddressHeaders = config.remoteAddressHeaders(); this.remotePortHeaders = config.remotePortHeaders(); } @@ -126,7 +126,7 @@ class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog { accessLogEntry.setConnectionId(connectionId.toString()); } - accessLog.log(accessLogEntry); + requestLog.log(accessLogEntry); } catch (Exception e) { // Catching any exceptions here as it is unclear how Jetty handles exceptions from a RequestLog. logger.log(Level.SEVERE, "Failed to log access log entry: " + e.getMessage(), e); 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 1e077c32ea1..510c561c10f 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 @@ -7,6 +7,7 @@ import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.ConnectionLog; +import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; @@ -73,7 +74,7 @@ public class JettyHttpServer extends AbstractServerProvider { ComponentRegistry<ConnectorFactory> connectorFactories, ComponentRegistry<ServletHolder> servletHolders, FilterInvoker filterInvoker, - AccessLog accessLog, + RequestLog requestLog, ConnectionLog connectionLog) { super(container); if (connectorFactories.allComponents().isEmpty()) @@ -83,7 +84,7 @@ public class JettyHttpServer extends AbstractServerProvider { server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); - server.setRequestLog(new AccessLogRequestLog(accessLog, serverConfig.accessLog())); + server.setRequestLog(new AccessLogRequestLog(requestLog, serverConfig.accessLog())); setupJmx(server, serverConfig); configureJettyThreadpool(server, serverConfig); JettyConnectionLogger connectionLogger = new JettyConnectionLogger(serverConfig.connectionLog(), connectionLog); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/VoidRequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/VoidRequestLog.java new file mode 100644 index 00000000000..f0ebf562fae --- /dev/null +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/VoidRequestLog.java @@ -0,0 +1,16 @@ +// Copyright Verizon Media. 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.AccessLogEntry; +import com.yahoo.container.logging.RequestLog; +import com.yahoo.container.logging.RequestLogEntry; + +/** + * @author bjorncs + */ +public class VoidRequestLog implements RequestLog { + + @Override public void log(AccessLogEntry entry) {} + @Override public void log(RequestLogEntry entry) {} + +} 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..68ab41e756f 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,8 @@ // 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.jdisc.http.ServerConfig; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.server.HttpChannel; @@ -117,7 +117,7 @@ public class AccessLogRequestLogTest { 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(mock(RequestLog.class), config).log(jettyRequest, createResponseMock()); } private static Request createRequestMock(AccessLogEntry entry) { diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java index 33fbe4e3016..a67656dd5ca 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java @@ -4,6 +4,7 @@ package com.yahoo.jdisc.http.server.jetty; import com.google.inject.AbstractModule; import com.google.inject.util.Modules; import com.yahoo.container.logging.ConnectionLog; +import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.AbstractResource; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.ResourceReference; @@ -556,6 +557,7 @@ public class FilterTestCase { bind(ConnectorConfig.class).toInstance(new ConnectorConfig(new ConnectorConfig.Builder())); bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder())); bind(ConnectionLog.class).toInstance(new VoidConnectionLog()); + bind(RequestLog.class).toInstance(new VoidRequestLog()); } }, new ConnectorFactoryRegistryModule(), diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java index 2f8e35fffa6..5659dfc2d3c 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java @@ -5,6 +5,7 @@ import com.google.inject.AbstractModule; import com.google.inject.Module; import com.google.inject.util.Modules; import com.yahoo.container.logging.ConnectionLog; +import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.jdisc.http.guiceModules.ConnectorFactoryRegistryModule; @@ -776,6 +777,8 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { .toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder())); bind(ConnectionLog.class) .toInstance(new VoidConnectionLog()); + bind(RequestLog.class) + .toInstance(new VoidRequestLog()); } }, new ConnectorFactoryRegistryModule()); diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 81ccb2ccb1c..2e460288cdc 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -3,10 +3,11 @@ package com.yahoo.jdisc.http.server.jetty; import com.google.inject.AbstractModule; import com.google.inject.Module; -import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.container.logging.ConnectionLog; import com.yahoo.container.logging.ConnectionLogEntry; +import com.yahoo.container.logging.RequestLog; +import com.yahoo.container.logging.RequestLogEntry; import com.yahoo.jdisc.References; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; @@ -176,31 +177,35 @@ public class HttpServerTest { @Test public void requireThatAccessLogIsCalledForRequestRejectedByJetty() throws Exception { - AccessLogMock accessLogMock = new AccessLogMock(); + RequestLogMock requestLogMock = new RequestLogMock(); final TestDriver driver = TestDrivers.newConfiguredInstance( mockRequestHandler(), new ServerConfig.Builder(), new ConnectorConfig.Builder().requestHeaderSize(1), - binder -> binder.bind(AccessLog.class).toInstance(accessLogMock)); + binder -> binder.bind(RequestLog.class).toInstance(requestLogMock)); driver.client().get("/status.html") .expectStatusCode(is(REQUEST_URI_TOO_LONG)); assertThat(driver.close(), is(true)); - assertThat(accessLogMock.logEntries.size(), equalTo(1)); - AccessLogEntry accessLogEntry = accessLogMock.logEntries.get(0); + assertThat(requestLogMock.logEntries.size(), equalTo(1)); + AccessLogEntry accessLogEntry = requestLogMock.logEntries.get(0); assertEquals(414, accessLogEntry.getStatusCode()); } - private static class AccessLogMock extends AccessLog { + private static class RequestLogMock implements RequestLog { final List<AccessLogEntry> logEntries = new CopyOnWriteArrayList<>(); - AccessLogMock() { super(null); } - @Override public void log(AccessLogEntry accessLogEntry) { logEntries.add(accessLogEntry); } + + @Override + public void log(RequestLogEntry entry) { + // TODO Implement + } + } @Test @@ -776,9 +781,9 @@ public class HttpServerTest { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - AccessLogMock accessLogMock = new AccessLogMock(); + RequestLogMock requestLogMock = new RequestLogMock(); InMemoryConnectionLog connectionLog = new InMemoryConnectionLog(); - TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, accessLogMock, /*mixedMode*/connectionLog, false); + TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, /*mixedMode*/connectionLog, false); String proxiedRemoteAddress = "192.168.0.100"; int proxiedRemotePort = 12345; @@ -786,9 +791,9 @@ public class HttpServerTest { sendJettyClientRequest(driver, certificateFile, new V2.Tag(proxiedRemoteAddress, proxiedRemotePort)); assertTrue(driver.close()); - assertEquals(2, accessLogMock.logEntries.size()); - assertLogEntryHasRemote(accessLogMock.logEntries.get(0), proxiedRemoteAddress, proxiedRemotePort); - assertLogEntryHasRemote(accessLogMock.logEntries.get(1), proxiedRemoteAddress, proxiedRemotePort); + assertEquals(2, requestLogMock.logEntries.size()); + assertLogEntryHasRemote(requestLogMock.logEntries.get(0), proxiedRemoteAddress, proxiedRemotePort); + assertLogEntryHasRemote(requestLogMock.logEntries.get(1), proxiedRemoteAddress, proxiedRemotePort); Assertions.assertThat(connectionLog.logEntries()).hasSize(2); assertLogEntryHasRemote(connectionLog.logEntries().get(0), proxiedRemoteAddress, proxiedRemotePort); assertLogEntryHasRemote(connectionLog.logEntries().get(1), proxiedRemoteAddress, proxiedRemotePort); @@ -799,18 +804,18 @@ public class HttpServerTest { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - AccessLogMock accessLogMock = new AccessLogMock(); + RequestLogMock requestLogMock = new RequestLogMock(); InMemoryConnectionLog connectionLog = new InMemoryConnectionLog(); - TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, accessLogMock, /*mixedMode*/connectionLog, true); + TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, /*mixedMode*/connectionLog, true); String proxiedRemoteAddress = "192.168.0.100"; sendJettyClientRequest(driver, certificateFile, null); sendJettyClientRequest(driver, certificateFile, new V2.Tag(proxiedRemoteAddress, 12345)); assertTrue(driver.close()); - assertEquals(2, accessLogMock.logEntries.size()); - assertLogEntryHasRemote(accessLogMock.logEntries.get(0), "127.0.0.1", 0); - assertLogEntryHasRemote(accessLogMock.logEntries.get(1), proxiedRemoteAddress, 0); + assertEquals(2, requestLogMock.logEntries.size()); + assertLogEntryHasRemote(requestLogMock.logEntries.get(0), "127.0.0.1", 0); + assertLogEntryHasRemote(requestLogMock.logEntries.get(1), proxiedRemoteAddress, 0); Assertions.assertThat(connectionLog.logEntries()).hasSize(2); assertLogEntryHasRemote(connectionLog.logEntries().get(0), null, 0); assertLogEntryHasRemote(connectionLog.logEntries().get(1), proxiedRemoteAddress, 12345); @@ -821,9 +826,9 @@ public class HttpServerTest { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - AccessLogMock accessLogMock = new AccessLogMock(); + RequestLogMock requestLogMock = new RequestLogMock(); InMemoryConnectionLog connectionLog = new InMemoryConnectionLog(); - TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, accessLogMock, connectionLog, /*mixedMode*/false); + TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, connectionLog, /*mixedMode*/false); String proxiedRemoteAddress = "192.168.0.100"; int proxiedRemotePort = 12345; @@ -927,7 +932,7 @@ public class HttpServerTest { } private static TestDriver createSslWithProxyProtocolTestDriver( - Path certificateFile, Path privateKeyFile, AccessLogMock accessLogMock, + Path certificateFile, Path privateKeyFile, RequestLogMock requestLogMock, InMemoryConnectionLog connectionLog, boolean mixedMode) { ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder() .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder() @@ -943,7 +948,7 @@ public class HttpServerTest { new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)), connectorConfig, binder -> { - binder.bind(AccessLog.class).toInstance(accessLogMock); + binder.bind(RequestLog.class).toInstance(requestLogMock); binder.bind(ConnectionLog.class).toInstance(connectionLog); }); } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java index 4b8a9bed1f3..7d7530c32e0 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java @@ -5,6 +5,7 @@ import com.google.inject.AbstractModule; import com.google.inject.Module; import com.google.inject.util.Modules; import com.yahoo.container.logging.ConnectionLog; +import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; @@ -82,6 +83,7 @@ public class TestDrivers { bind(ConnectorConfig.class).toInstance(new ConnectorConfig(connectorConfigBuilder)); bind(FilterBindings.class).toInstance(new FilterBindings.Builder().build()); bind(ConnectionLog.class).toInstance(new VoidConnectionLog()); + bind(RequestLog.class).toInstance(new VoidRequestLog()); } }, new ConnectorFactoryRegistryModule(connectorConfigBuilder), diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java index 6637ee24b5f..ffd58902256 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java @@ -6,6 +6,7 @@ import com.google.inject.Module; import com.google.inject.util.Modules; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.AccessLogEntry; +import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.http.server.jetty.TestDriver; import com.yahoo.jdisc.http.server.jetty.TestDrivers; import org.junit.Test; @@ -21,6 +22,7 @@ import static org.mockito.Mockito.verify; /** * @author bakksjo + * @author bjorncs */ public class ServletAccessLoggingTest extends ServletTestBase { private static final long MAX_LOG_WAIT_TIME_MILLIS = TimeUnit.SECONDS.toMillis(60); @@ -41,20 +43,20 @@ public class ServletAccessLoggingTest extends ServletTestBase { verifyCallsLog(accessLog, timeout(MAX_LOG_WAIT_TIME_MILLIS).times(1)); } - private void verifyCallsLog(final AccessLog accessLog, final VerificationMode verificationMode) { - verify(accessLog, verificationMode).log(any(AccessLogEntry.class)); + private void verifyCallsLog(RequestLog requestLog, final VerificationMode verificationMode) { + verify(requestLog, verificationMode).log(any(AccessLogEntry.class)); } - private TestDriver newTestDriver(final AccessLog accessLog) throws IOException { - return TestDrivers.newInstance(dummyRequestHandler, bindings(accessLog)); + private TestDriver newTestDriver(RequestLog requestLog) throws IOException { + return TestDrivers.newInstance(dummyRequestHandler, bindings(requestLog)); } - private Module bindings(final AccessLog accessLog) { + private Module bindings(RequestLog requestLog) { return Modules.combine( new AbstractModule() { @Override protected void configure() { - bind(AccessLog.class).toInstance(accessLog); + bind(RequestLog.class).toInstance(requestLog); } }, guiceModule()); |