summaryrefslogtreecommitdiffstats
path: root/jdisc_http_service
diff options
context:
space:
mode:
authorBjørn Christian Seime <bjorncs@verizonmedia.com>2021-01-21 13:28:18 +0100
committerBjørn Christian Seime <bjorncs@verizonmedia.com>2021-01-21 14:22:25 +0100
commitb1bbb58449af091cf20f296ba19130b446d68022 (patch)
tree572838f05c7f0d081ed05d1b566d9d92377f4305 /jdisc_http_service
parent4c514714e3e739c66f0107629d44ef5e5877403a (diff)
Replace AccessLogEntry with non-blocking RequestLogEntry
Keep AccessLogEntry as interface for adding extra information in handlers, but use the new RequestLogEntry for access log serialization. Introduce new interface RequestLog that AccessLog class implements (to simplify unit testing). Rename AccessLogInterface to RequestLogHandler. Remove unused class AccessLogSampler.
Diffstat (limited to 'jdisc_http_service')
-rw-r--r--jdisc_http_service/pom.xml6
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLog.java15
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogSampler.java37
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONAccessLog.java6
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONFormatter.java94
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLog.java2
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLogHandler.java (renamed from jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogInterface.java)4
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/container/logging/VespaAccessLog.java28
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java119
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/VoidRequestLog.java2
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/container/logging/AccessLogSamplerTest.java62
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java140
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java78
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java57
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/InMemoryRequestLog.java20
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java4
16 files changed, 299 insertions, 375 deletions
diff --git a/jdisc_http_service/pom.xml b/jdisc_http_service/pom.xml
index 1563a1a3d17..68a0f0636d3 100644
--- a/jdisc_http_service/pom.xml
+++ b/jdisc_http_service/pom.xml
@@ -145,6 +145,12 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.yahoo.vespa</groupId>
+ <artifactId>testutil</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
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 f60c52f7f38..cdb4febb775 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
@@ -13,10 +13,10 @@ import com.yahoo.component.provider.ComponentRegistry;
*/
public class AccessLog implements RequestLog {
- private ComponentRegistry<AccessLogInterface> implementers;
+ private final ComponentRegistry<RequestLogHandler> implementers;
@Inject
- public AccessLog(ComponentRegistry<AccessLogInterface> implementers) {
+ public AccessLog(ComponentRegistry<RequestLogHandler> implementers) {
this.implementers = implementers;
}
@@ -25,15 +25,10 @@ public class AccessLog implements RequestLog {
}
@Override
- public void log(AccessLogEntry accessLogEntry) {
- for (AccessLogInterface log: implementers.allComponents()) {
- log.log(accessLogEntry);
- }
- }
-
- @Override
public void log(RequestLogEntry entry) {
- // TODO Implement
+ for (RequestLogHandler handler: implementers.allComponents()) {
+ handler.log(entry);
+ }
}
}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogSampler.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogSampler.java
deleted file mode 100644
index 12d29c2f333..00000000000
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogSampler.java
+++ /dev/null
@@ -1,37 +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.container.logging;
-
-import java.util.concurrent.atomic.AtomicLong;
-
-/**
- * Samples entries from access log. It first samples every query until it have some data, and then sub-samples
- * much less frequently to reduce CPU usage and latency impact. It only samples successful requests and requests
- * that starts with /search.
- *
- * @author dybis
- */
-public class AccessLogSampler implements AccessLogInterface {
-
- private final AtomicLong accessLineCounter = new AtomicLong(0);
- private final CircularArrayAccessLogKeeper circularArrayAccessLogKeeper;
-
- public AccessLogSampler(CircularArrayAccessLogKeeper circularArrayAccessLogKeeper) {
- this.circularArrayAccessLogKeeper = circularArrayAccessLogKeeper;
- }
-
- @Override
- public void log(AccessLogEntry accessLogEntry) {
- if (accessLogEntry.getStatusCode() != 200) {
- return;
- }
- String uriString = accessLogEntry.getRawPath();
- if (! uriString.startsWith("/search")) {
- return;
- }
- final long count = accessLineCounter.incrementAndGet();
- if (count >= CircularArrayAccessLogKeeper.SIZE && count % CircularArrayAccessLogKeeper.SIZE != 0) {
- return;
- }
- circularArrayAccessLogKeeper.addUri(uriString);
- }
-}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONAccessLog.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONAccessLog.java
index 5a188d68e89..aa2df959a4c 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONAccessLog.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONAccessLog.java
@@ -11,7 +11,7 @@ import java.util.logging.Level;
* @author frodelu
* @author Tony Vaagenes
*/
-public final class JSONAccessLog implements AccessLogInterface {
+public final class JSONAccessLog implements RequestLogHandler {
private final AccessLogHandler logHandler;
private final JSONFormatter formatter;
@@ -22,8 +22,8 @@ public final class JSONAccessLog implements AccessLogInterface {
}
@Override
- public void log(AccessLogEntry logEntry) {
- logHandler.access.log(Level.INFO, formatter.format(logEntry) + '\n');
+ public void log(RequestLogEntry entry) {
+ logHandler.access.log(Level.INFO, formatter.format(entry) + '\n');
}
// TODO: This is never called. We should have a DI provider and call this method from its deconstruct.
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONFormatter.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONFormatter.java
index b6ceaa08007..dc0acc4fbac 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONFormatter.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/JSONFormatter.java
@@ -12,8 +12,7 @@ import java.io.IOException;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.security.Principal;
-import java.util.List;
-import java.util.Map;
+import java.util.Collection;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -42,76 +41,76 @@ public class JSONFormatter {
}
/**
- * The main method for formatting the associated {@link AccessLogEntry} as a Vespa JSON access log string
+ * The main method for formatting the associated {@link RequestLogEntry} as a Vespa JSON access log string
*
* @return The Vespa JSON access log string without trailing newline
*/
- public String format(AccessLogEntry accessLogEntry) {
+ public String format(RequestLogEntry entry) {
ByteArrayOutputStream logLine = new ByteArrayOutputStream();
try {
JsonGenerator generator = generatorFactory.createGenerator(logLine, JsonEncoding.UTF8);
generator.writeStartObject();
- generator.writeStringField("ip", accessLogEntry.getIpV4Address());
- generator.writeNumberField("time", toTimestampInSeconds(accessLogEntry.getTimeStampMillis()));
- generator.writeNumberField("duration", durationAsSeconds(accessLogEntry.getDurationBetweenRequestResponseMillis()));
- generator.writeNumberField("responsesize", accessLogEntry.getReturnedContentSize());
- generator.writeNumberField("code", accessLogEntry.getStatusCode());
- generator.writeStringField("method", accessLogEntry.getHttpMethod());
- generator.writeStringField("uri", getNormalizedURI(accessLogEntry.getRawPath(), accessLogEntry.getRawQuery().orElse(null)));
- generator.writeStringField("version", accessLogEntry.getHttpVersion());
- generator.writeStringField("agent", accessLogEntry.getUserAgent());
- generator.writeStringField("host", accessLogEntry.getHostString());
- generator.writeStringField("scheme", accessLogEntry.getScheme());
- generator.writeNumberField("localport", accessLogEntry.getLocalPort());
-
- String connectionId = accessLogEntry.getConnectionId().orElse(null);
+ String peerAddress = entry.peerAddress().get();
+ generator.writeStringField("ip", peerAddress);
+ long time = entry.timestamp().get().toEpochMilli();
+ generator.writeNumberField("time", toTimestampInSeconds(time));
+ generator.writeNumberField("duration", durationAsSeconds(entry.duration().get().toMillis()));
+ generator.writeNumberField("responsesize", entry.contentSize().orElse(0));
+ generator.writeNumberField("code", entry.statusCode().orElse(0));
+ generator.writeStringField("method", entry.httpMethod().orElse(""));
+ generator.writeStringField("uri", getNormalizedURI(entry.rawPath().get(), entry.rawQuery().orElse(null)));
+ generator.writeStringField("version", entry.httpVersion().orElse(""));
+ generator.writeStringField("agent", entry.userAgent().orElse(""));
+ generator.writeStringField("host", entry.hostString().orElse(""));
+ generator.writeStringField("scheme", entry.scheme().orElse(null));
+ generator.writeNumberField("localport", entry.localPort().getAsInt());
+
+ String connectionId = entry.connectionId().orElse(null);
if (connectionId != null) {
generator.writeStringField("connection", connectionId);
}
- Principal principal = accessLogEntry.getUserPrincipal();
+ Principal principal = entry.userPrincipal().orElse(null);
if (principal != null) {
generator.writeStringField("user-principal", principal.getName());
}
- Principal sslPrincipal = accessLogEntry.getSslPrincipal();
- if (sslPrincipal != null) {
- generator.writeStringField("ssl-principal", sslPrincipal.getName());
- }
-
+ String remoteAddress = entry.remoteAddress().orElse(null);
+ int remotePort = entry.remotePort().orElse(0);
// Only add remote address/port fields if relevant
- if (remoteAddressDiffers(accessLogEntry.getIpV4Address(), accessLogEntry.getRemoteAddress())) {
- generator.writeStringField("remoteaddr", accessLogEntry.getRemoteAddress());
- if (accessLogEntry.getRemotePort() > 0) {
- generator.writeNumberField("remoteport", accessLogEntry.getRemotePort());
+ if (remoteAddressDiffers(peerAddress, remoteAddress)) {
+ generator.writeStringField("remoteaddr", remoteAddress);
+ if (remotePort > 0) {
+ generator.writeNumberField("remoteport", remotePort);
}
}
// Only add peer address/port fields if relevant
- if (accessLogEntry.getPeerAddress() != null) {
- generator.writeStringField("peeraddr", accessLogEntry.getPeerAddress());
+ if (peerAddress != null) {
+ generator.writeStringField("peeraddr", peerAddress);
- int peerPort = accessLogEntry.getPeerPort();
- if (peerPort > 0 && peerPort != accessLogEntry.getRemotePort()) {
+ int peerPort = entry.peerPort().getAsInt();
+ if (peerPort > 0 && peerPort != remotePort) {
generator.writeNumberField("peerport", peerPort);
}
}
- TraceNode trace = accessLogEntry.getTrace();
+ TraceNode trace = entry.traceNode().orElse(null);
if (trace != null) {
long timestamp = trace.timestamp();
if (timestamp == 0L) {
- timestamp = accessLogEntry.getTimeStampMillis();
+ timestamp = time;
}
trace.accept(new TraceRenderer(generator, timestamp));
}
// Only add search sub block of this is a search request
- if (isSearchRequest(accessLogEntry)) {
+ if (isSearchRequest(entry)) {
+ HitCounts hitCounts = entry.hitCounts().get();
generator.writeObjectFieldStart("search");
- generator.writeNumberField("totalhits", getTotalHitCount(accessLogEntry.getHitCounts()));
- generator.writeNumberField("hits", getRetrievedHitCount(accessLogEntry.getHitCounts()));
- Coverage c = accessLogEntry.getHitCounts().getCoverage();
+ generator.writeNumberField("totalhits", getTotalHitCount(hitCounts));
+ generator.writeNumberField("hits", getRetrievedHitCount(hitCounts));
+ Coverage c = hitCounts.getCoverage();
if (c != null) {
generator.writeObjectFieldStart(COVERAGE);
generator.writeNumberField(COVERAGE_COVERAGE, c.getResultPercentage());
@@ -135,16 +134,17 @@ public class JSONFormatter {
// Add key/value access log entries. Keys with single values are written as single
// string value fields while keys with multiple values are written as string arrays
- Map<String,List<String>> keyValues = accessLogEntry.getKeyValues();
- if (keyValues != null && !keyValues.isEmpty()) {
+ Collection<String> keys = entry.extraAttributeKeys();
+ if (!keys.isEmpty()) {
generator.writeObjectFieldStart("attributes");
- for (Map.Entry<String,List<String>> entry : keyValues.entrySet()) {
- if (entry.getValue().size() == 1) {
- generator.writeStringField(entry.getKey(), entry.getValue().get(0));
+ for (String key : keys) {
+ Collection<String> values = entry.extraAttributeValues(key);
+ if (values.size() == 1) {
+ generator.writeStringField(key, values.iterator().next());
} else {
- generator.writeFieldName(entry.getKey());
+ generator.writeFieldName(key);
generator.writeStartArray();
- for (String s : entry.getValue()) {
+ for (String s : values) {
generator.writeString(s);
}
generator.writeEndArray();
@@ -168,8 +168,8 @@ public class JSONFormatter {
return remoteAddress != null && !Objects.equals(ipV4Address, remoteAddress);
}
- private boolean isSearchRequest(AccessLogEntry logEntry) {
- return logEntry != null && (logEntry.getHitCounts() != null);
+ private boolean isSearchRequest(RequestLogEntry entry) {
+ return entry != null && entry.hitCounts().isPresent();
}
private long getTotalHitCount(HitCounts counts) {
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
index f1036dcb880..2090ba1b9f1 100644
--- 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
@@ -8,8 +8,6 @@ package com.yahoo.container.logging;
*/
public interface RequestLog {
- void log(AccessLogEntry entry);
-
void log(RequestLogEntry entry);
}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogInterface.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLogHandler.java
index 2523174abef..85df08e4abb 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/AccessLogInterface.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/RequestLogHandler.java
@@ -4,6 +4,6 @@ package com.yahoo.container.logging;
/**
* @author Tony Vaagenes
*/
-public interface AccessLogInterface {
- void log(AccessLogEntry accessLogEntry);
+public interface RequestLogHandler {
+ void log(RequestLogEntry entry);
}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/VespaAccessLog.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/VespaAccessLog.java
index 054fc0fcbf7..1040846dda3 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/VespaAccessLog.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/VespaAccessLog.java
@@ -12,7 +12,7 @@ import java.util.logging.Level;
* @author Bjorn Borud
* @author Oyvind Bakksjo
*/
-public final class VespaAccessLog implements AccessLogInterface {
+public final class VespaAccessLog implements RequestLogHandler {
private static final ThreadLocal<SimpleDateFormat> dateFormat = ThreadLocal.withInitial(VespaAccessLog::createDateFormat);
@@ -92,20 +92,20 @@ public final class VespaAccessLog implements AccessLogInterface {
}
@Override
- public void log(final AccessLogEntry accessLogEntry) {
+ public void log(RequestLogEntry entry) {
writeLog(
- accessLogEntry.getIpV4Address(),
- accessLogEntry.getUser(),
+ entry.peerAddress().get(),
+ null,
getRequest(
- accessLogEntry.getHttpMethod(),
- accessLogEntry.getRawPath(),
- accessLogEntry.getRawQuery().orElse(null),
- accessLogEntry.getHttpVersion()),
- accessLogEntry.getReferer(),
- accessLogEntry.getUserAgent(),
- accessLogEntry.getDurationBetweenRequestResponseMillis(),
- accessLogEntry.getReturnedContentSize(),
- accessLogEntry.getHitCounts(),
- accessLogEntry.getStatusCode());
+ entry.httpMethod().orElse(null),
+ entry.rawPath().orElse(null),
+ entry.rawQuery().orElse(null),
+ entry.httpVersion().orElse(null)),
+ entry.referer().orElse(null),
+ entry.userAgent().orElse(null),
+ entry.duration().get().toMillis(),
+ entry.contentSize().orElse(0L),
+ entry.hitCounts().orElse(null),
+ entry.statusCode().orElse(0));
}
}
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 fb4db5a6307..e82373fdaae 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
@@ -5,6 +5,7 @@ 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.container.logging.RequestLogEntry;
import com.yahoo.jdisc.http.ServerConfig;
import com.yahoo.jdisc.http.servlet.ServletRequest;
import org.eclipse.jetty.server.Request;
@@ -13,11 +14,12 @@ import org.eclipse.jetty.util.component.AbstractLifeCycle;
import javax.servlet.http.HttpServletRequest;
import java.security.Principal;
-import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.Instant;
import java.util.List;
-import java.util.Optional;
import java.util.OptionalInt;
import java.util.UUID;
+import java.util.function.BiConsumer;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -50,83 +52,67 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty
@Override
public void log(Request request, Response response) {
try {
- AccessLogEntry accessLogEntry = Optional.ofNullable(request.getAttribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY))
- .map(AccessLogEntry.class::cast)
- .orElseGet(AccessLogEntry::new);
-
- accessLogEntry.setRawPath(request.getRequestURI());
- String queryString = request.getQueryString();
- if (queryString != null) {
- accessLogEntry.setRawQuery(queryString);
- }
-
- accessLogEntry.setUserAgent(request.getHeader("User-Agent"));
- accessLogEntry.setHttpMethod(request.getMethod());
- accessLogEntry.setHostString(request.getHeader("Host"));
- accessLogEntry.setReferer(request.getHeader("Referer"));
+ RequestLogEntry.Builder builder = new RequestLogEntry.Builder();
String peerAddress = request.getRemoteAddr();
- accessLogEntry.setIpV4Address(peerAddress);
- accessLogEntry.setPeerAddress(peerAddress);
- String remoteAddress = getRemoteAddress(request);
- if (!Objects.equal(remoteAddress, peerAddress)) {
- accessLogEntry.setRemoteAddress(remoteAddress);
- }
-
int peerPort = request.getRemotePort();
- accessLogEntry.setPeerPort(peerPort);
- int remotePort = getRemotePort(request);
- if (remotePort != peerPort) {
- accessLogEntry.setRemotePort(remotePort);
- }
- accessLogEntry.setHttpVersion(request.getProtocol());
- accessLogEntry.setScheme(request.getScheme());
- accessLogEntry.setLocalPort(getConnectorLocalPort(request));
+ long startTime = request.getTimeStamp();
+ long endTime = System.currentTimeMillis();
+ builder.peerAddress(peerAddress)
+ .peerPort(peerPort)
+ .localPort(getConnectorLocalPort(request))
+ .timestamp(Instant.ofEpochMilli(startTime))
+ .duration(Duration.ofMillis(endTime - startTime))
+ .contentSize(response.getHttpChannel().getBytesWritten())
+ .statusCode(response.getCommittedMetaData().getStatus());
+
+ addNonNullValue(builder, request.getMethod(), RequestLogEntry.Builder::httpMethod);
+ addNonNullValue(builder, request.getRequestURI(), RequestLogEntry.Builder::rawPath);
+ addNonNullValue(builder, request.getProtocol(), RequestLogEntry.Builder::httpVersion);
+ addNonNullValue(builder, request.getScheme(), RequestLogEntry.Builder::scheme);
+ addNonNullValue(builder, request.getHeader("User-Agent"), RequestLogEntry.Builder::userAgent);
+ addNonNullValue(builder, request.getHeader("Host"), RequestLogEntry.Builder::hostString);
+ addNonNullValue(builder, request.getHeader("Referer"), RequestLogEntry.Builder::referer);
+ addNonNullValue(builder, request.getQueryString(), RequestLogEntry.Builder::rawQuery);
+
Principal principal = (Principal) request.getAttribute(ServletRequest.JDISC_REQUEST_PRINCIPAL);
- if (principal != null) {
- accessLogEntry.setUserPrincipal(principal);
- }
- X509Certificate[] clientCert = (X509Certificate[]) request.getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT);
- if (clientCert != null && clientCert.length > 0) {
- accessLogEntry.setSslPrincipal(clientCert[0].getSubjectX500Principal());
- }
- String sslSessionId = (String) request.getAttribute(ServletRequest.SERVLET_REQUEST_SSL_SESSION_ID);
- if (sslSessionId != null) {
- accessLogEntry.addKeyValue("ssl-session-id", sslSessionId);
- }
- String cipherSuite = (String) request.getAttribute(ServletRequest.SERVLET_REQUEST_CIPHER_SUITE);
- if (cipherSuite != null) {
- accessLogEntry.addKeyValue("cipher-suite", cipherSuite);
- }
+ addNonNullValue(builder, principal, RequestLogEntry.Builder::userPrincipal);
+
String requestFilterId = (String) request.getAttribute(ServletRequest.JDISC_REQUEST_CHAIN);
- if (requestFilterId != null) {
- accessLogEntry.addKeyValue("request-chain", requestFilterId);
- }
+ addNonNullValue(builder, requestFilterId, (b, chain) -> b.addExtraAttribute("request-chain", chain));
+
String responseFilterId = (String) request.getAttribute(ServletRequest.JDISC_RESPONSE_CHAIN);
- if (responseFilterId != null) {
- accessLogEntry.addKeyValue("response-chain", responseFilterId);
- }
+ addNonNullValue(builder, responseFilterId, (b, chain) -> b.addExtraAttribute("response-chain", chain));
- long startTime = request.getTimeStamp();
- long endTime = System.currentTimeMillis();
- accessLogEntry.setTimeStamp(startTime);
- accessLogEntry.setDurationBetweenRequestResponse(endTime - startTime);
- accessLogEntry.setReturnedContentSize(response.getHttpChannel().getBytesWritten());
- accessLogEntry.setStatusCode(response.getCommittedMetaData().getStatus());
+ UUID connectionId = (UUID) request.getAttribute(JettyConnectionLogger.CONNECTION_ID_REQUEST_ATTRIBUTE);
+ addNonNullValue(builder, connectionId, (b, uuid) -> b.connectionId(uuid.toString()));
+ String remoteAddress = getRemoteAddress(request);
+ if (!Objects.equal(remoteAddress, peerAddress)) {
+ builder.remoteAddress(remoteAddress);
+ }
+ int remotePort = getRemotePort(request);
+ if (remotePort != peerPort) {
+ builder.remotePort(remotePort);
+ }
LOGGED_REQUEST_HEADERS.forEach(header -> {
String value = request.getHeader(header);
if (value != null) {
- accessLogEntry.addKeyValue(header, value);
+ builder.addExtraAttribute(header, value);
}
});
- UUID connectionId = (UUID) request.getAttribute(JettyConnectionLogger.CONNECTION_ID_REQUEST_ATTRIBUTE);
- if (connectionId != null) {
- accessLogEntry.setConnectionId(connectionId.toString());
+ AccessLogEntry accessLogEntry = (AccessLogEntry) request.getAttribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY);
+ if (accessLogEntry != null) {
+ var extraAttributes = accessLogEntry.getKeyValues();
+ if (extraAttributes != null) {
+ extraAttributes.forEach(builder::addExtraAttributes);
+ }
+ addNonNullValue(builder, accessLogEntry.getHitCounts(), RequestLogEntry.Builder::hitCounts);
+ addNonNullValue(builder, accessLogEntry.getTrace(), RequestLogEntry.Builder::traceNode);
}
- requestLog.log(accessLogEntry);
+ requestLog.log(builder.build());
} 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);
@@ -160,4 +146,11 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty
}
}
+ private static <T> void addNonNullValue(
+ RequestLogEntry.Builder builder, T value, BiConsumer<RequestLogEntry.Builder, T> setter) {
+ if (value != null) {
+ setter.accept(builder, value);
+ }
+ }
+
}
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
index f0ebf562fae..9db5ba99115 100644
--- 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
@@ -1,7 +1,6 @@
// 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;
@@ -10,7 +9,6 @@ import com.yahoo.container.logging.RequestLogEntry;
*/
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/container/logging/AccessLogSamplerTest.java b/jdisc_http_service/src/test/java/com/yahoo/container/logging/AccessLogSamplerTest.java
deleted file mode 100644
index 32a55b4bb72..00000000000
--- a/jdisc_http_service/src/test/java/com/yahoo/container/logging/AccessLogSamplerTest.java
+++ /dev/null
@@ -1,62 +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.container.logging;
-
-import org.junit.Test;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import static org.hamcrest.CoreMatchers.hasItem;
-import static org.hamcrest.CoreMatchers.hasItems;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.Matchers.contains;
-import static org.junit.Assert.assertThat;
-
-public class AccessLogSamplerTest {
-
- private final List<String> uris = new ArrayList<>();
- private CircularArrayAccessLogKeeper circularArrayAccessLogKeeperMock = new CircularArrayAccessLogKeeper() {
- @Override
- public void addUri(String uri) {
- uris.add(uri);
- }
- };
- private AccessLogSampler accessLogSampler = new AccessLogSampler(circularArrayAccessLogKeeperMock);
-
- @Test
- public void testAFewLines() {
- accessLogSampler.log(createLogEntry(200, "/search/foo"));
- accessLogSampler.log(createLogEntry(500, "/search/bar"));
- accessLogSampler.log(createLogEntry(500, "bar"));
- accessLogSampler.log(createLogEntry(200, "/search/what"));
- assertThat(uris, contains("/search/foo", "/search/what"));
- }
-
- @Test
- public void testSubSampling() {
- for (int i = 0; i < CircularArrayAccessLogKeeper.SIZE; i++) {
- accessLogSampler.log(createLogEntry(200, "/search/" + String.valueOf(i)));
- }
- assertThat(uris.size(), is(CircularArrayAccessLogKeeper.SIZE));
- assertThat(uris, hasItems("/search/0", "/search/1", "/search/2",
- "/search/" + String.valueOf(CircularArrayAccessLogKeeper.SIZE - 1)));
- uris.clear();
- for (int i = 0; i < CircularArrayAccessLogKeeper.SIZE; i++) {
- accessLogSampler.log(createLogEntry(200, "/search/fuzz"));
- }
- assertThat(uris, hasItem("/search/fuzz"));
- assertThat(uris.size(), is(1));
- for (int i = 0; i < CircularArrayAccessLogKeeper.SIZE; i++) {
- accessLogSampler.log(createLogEntry(200, "/search/ball"));
- }
- assertThat(uris, hasItem("/search/ball"));
- assertThat(uris.size(), is(2));
- }
-
- private AccessLogEntry createLogEntry(int statusCode, String uri) {
- AccessLogEntry accessLogEntry = new AccessLogEntry();
- accessLogEntry.setStatusCode(statusCode);
- accessLogEntry.setRawPath(uri);
- return accessLogEntry;
- }
-}
diff --git a/jdisc_http_service/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java
index 703dcb95d95..f6a0d7418a0 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java
@@ -6,8 +6,10 @@ import org.junit.Test;
import java.io.IOException;
import java.net.URI;
+import java.time.Duration;
+import java.time.Instant;
-import static org.junit.Assert.assertEquals;
+import static com.yahoo.test.json.JsonTestHelper.assertJsonEquals;
/**
@@ -19,25 +21,25 @@ public class JSONLogTestCase {
private static final String EMPTY_REFERRER = "";
private static final String EMPTY_USERAGENT = "";
- private AccessLogEntry newAccessLogEntry(final String query) {
- return newAccessLogEntry(query, new Coverage(100,100,100,0));
+ private RequestLogEntry.Builder newRequestLogEntry(final String query) {
+ return newRequestLogEntry(query, new Coverage(100,100,100,0));
}
- private AccessLogEntry newAccessLogEntry(final String query, Coverage coverage) {
- final AccessLogEntry entry = new AccessLogEntry();
- entry.setRawQuery("query="+query);
- entry.setRawPath("");
- entry.setIpV4Address(ipAddress);
- entry.setHttpMethod("GET");
- entry.setHttpVersion("HTTP/1.1");
- entry.setUserAgent("Mozilla/4.05 [en] (Win95; I)");
- entry.setHitCounts(new HitCounts(0, 10, 1234, 0, 10, coverage));
- entry.setHostString("localhost");
- entry.setStatusCode(200);
- entry.setTimeStamp(920880005023L);
- entry.setDurationBetweenRequestResponse(122);
- entry.setReturnedContentSize(9875);
-
- return entry;
+ private RequestLogEntry.Builder newRequestLogEntry(final String query, Coverage coverage) {
+ return new RequestLogEntry.Builder()
+ .rawQuery("query=" + query)
+ .rawPath("")
+ .peerAddress(ipAddress)
+ .httpMethod("GET")
+ .httpVersion("HTTP/1.1")
+ .userAgent("Mozilla/4.05 [en] (Win95; I)")
+ .hitCounts(new HitCounts(0, 10, 1234, 0, 10, coverage))
+ .hostString("localhost")
+ .statusCode(200)
+ .timestamp(Instant.ofEpochMilli(920880005023L))
+ .duration(Duration.ofMillis(122))
+ .contentSize(9875)
+ .localPort(0)
+ .peerPort(0);
}
private static URI newQueryUri(final String query) {
@@ -46,10 +48,11 @@ public class JSONLogTestCase {
@Test
public void test_json_log_entry() throws Exception {
- AccessLogEntry entry = newAccessLogEntry("test");
+ RequestLogEntry entry = newRequestLogEntry("test").build();
String expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -68,17 +71,19 @@ public class JSONLogTestCase {
"}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
}
@Test
public void test_json_of_trace() throws IOException {
TraceNode root = new TraceNode("root", 7);
- AccessLogEntry entry = newAccessLogEntry("test");
- entry.setTrace(root);
+ RequestLogEntry entry = newRequestLogEntry("test")
+ .traceNode(root)
+ .build();
String expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -98,18 +103,20 @@ public class JSONLogTestCase {
"}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
}
@Test
public void test_with_keyvalues() {
- AccessLogEntry entry = newAccessLogEntry("test");
- entry.addKeyValue("singlevalue", "value1");
- entry.addKeyValue("multivalue", "value2");
- entry.addKeyValue("multivalue", "value3");
+ RequestLogEntry entry = newRequestLogEntry("test")
+ .addExtraAttribute("singlevalue", "value1")
+ .addExtraAttribute("multivalue", "value2")
+ .addExtraAttribute("multivalue", "value3")
+ .build();
String expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -131,19 +138,19 @@ public class JSONLogTestCase {
"\"multivalue\":[\"value2\",\"value3\"]}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
}
@Test
public void test_with_remoteaddrport() throws Exception {
- AccessLogEntry entry = newAccessLogEntry("test");
-
- // First test with only remote address and not port set
- entry.setRemoteAddress("FE80:0000:0000:0000:0202:B3FF:FE1E:8329");
+ RequestLogEntry entry = newRequestLogEntry("test")
+ .remoteAddress("FE80:0000:0000:0000:0202:B3FF:FE1E:8329")
+ .build();
String expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -163,13 +170,17 @@ public class JSONLogTestCase {
"}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
// Add remote port and verify
- entry.setRemotePort(1234);
+ entry = newRequestLogEntry("test")
+ .remoteAddress("FE80:0000:0000:0000:0202:B3FF:FE1E:8329")
+ .remotePort(1234)
+ .build();
expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -190,36 +201,41 @@ public class JSONLogTestCase {
"}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
}
@Test
public void test_remote_address_same_as_ip_address() throws Exception {
- AccessLogEntry entry = newAccessLogEntry("test");
- AccessLogEntry entrywithremote = newAccessLogEntry("test");
- entrywithremote.setRemoteAddress(entry.getIpV4Address());
+ RequestLogEntry entry = newRequestLogEntry("test").build();
+ RequestLogEntry entrywithremote = newRequestLogEntry("test")
+ .remoteAddress(entry.peerAddress().get())
+ .build();
JSONFormatter formatter = new JSONFormatter();
- assertEquals(formatter.format(entry), formatter.format(entrywithremote));
+ assertJsonEquals(formatter.format(entry), formatter.format(entrywithremote));
}
@Test
public void test_useragent_with_quotes() {
- final AccessLogEntry entry = new AccessLogEntry();
- entry.setRawQuery("query=test");
- entry.setRawPath("");
- entry.setIpV4Address(ipAddress);
- entry.setHttpMethod("GET");
- entry.setHttpVersion("HTTP/1.1");
- entry.setUserAgent("Mozilla/4.05 [en] (Win95; I; \"Best Browser Ever\")");
- entry.setHitCounts(new HitCounts(0, 10, 1234, 0, 10, new Coverage(100,200,200,0)));
- entry.setHostString("localhost");
- entry.setStatusCode(200);
- entry.setTimeStamp(920880005023L);
- entry.setDurationBetweenRequestResponse(122);
- entry.setReturnedContentSize(9875);
+ RequestLogEntry entry = new RequestLogEntry.Builder()
+ .rawQuery("query=test")
+ .rawPath("")
+ .peerAddress(ipAddress)
+ .httpMethod("GET")
+ .httpVersion("HTTP/1.1")
+ .userAgent("Mozilla/4.05 [en] (Win95; I; \"Best Browser Ever\")")
+ .hitCounts(new HitCounts(0, 10, 1234, 0, 10, new Coverage(100, 200, 200, 0)))
+ .hostString("localhost")
+ .statusCode(200)
+ .timestamp(Instant.ofEpochMilli(920880005023L))
+ .duration(Duration.ofMillis(122))
+ .contentSize(9875)
+ .localPort(0)
+ .peerPort(0)
+ .build();
String expectedOutput =
"{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -238,11 +254,13 @@ public class JSONLogTestCase {
"}" +
"}";
- assertEquals(expectedOutput, new JSONFormatter().format(entry));
+ assertJsonEquals(new JSONFormatter().format(entry), expectedOutput);
}
- private void verifyCoverage(String coverage, AccessLogEntry entry) {
- assertEquals("{\"ip\":\"152.200.54.243\"," +
+ private void verifyCoverage(String coverage, RequestLogEntry entry) {
+ assertJsonEquals(new JSONFormatter().format(entry),
+ "{\"ip\":\"152.200.54.243\"," +
+ "\"peeraddr\":\"152.200.54.243\"," +
"\"time\":920880005.023," +
"\"duration\":0.122," +
"\"responsesize\":9875," +
@@ -259,18 +277,18 @@ public class JSONLogTestCase {
"\"hits\":0," +
coverage +
"}" +
- "}", new JSONFormatter().format(entry));
+ "}");
}
@Test
public void test_with_coverage_degradation() {
verifyCoverage("\"coverage\":{\"coverage\":50,\"documents\":100,\"degraded\":{\"non-ideal-state\":true}}",
- newAccessLogEntry("test", new Coverage(100,200,200,0)));
+ newRequestLogEntry("test", new Coverage(100,200,200,0)).build());
verifyCoverage("\"coverage\":{\"coverage\":50,\"documents\":100,\"degraded\":{\"match-phase\":true}}",
- newAccessLogEntry("test", new Coverage(100,200,200,1)));
+ newRequestLogEntry("test", new Coverage(100,200,200,1)).build());
verifyCoverage("\"coverage\":{\"coverage\":50,\"documents\":100,\"degraded\":{\"timeout\":true}}",
- newAccessLogEntry("test", new Coverage(100,200,200,2)));
+ newRequestLogEntry("test", new Coverage(100,200,200,2)).build());
verifyCoverage("\"coverage\":{\"coverage\":50,\"documents\":100,\"degraded\":{\"adaptive-timeout\":true}}",
- newAccessLogEntry("test", new Coverage(100,200,200,4)));
+ newRequestLogEntry("test", new Coverage(100,200,200,4)).build());
}
}
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 68ab41e756f..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
@@ -3,6 +3,7 @@ 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;
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(RequestLog.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;
}
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 2e460288cdc..96464a48938 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,7 +3,6 @@ package com.yahoo.jdisc.http.server.jetty;
import com.google.inject.AbstractModule;
import com.google.inject.Module;
-import com.yahoo.container.logging.AccessLogEntry;
import com.yahoo.container.logging.ConnectionLog;
import com.yahoo.container.logging.ConnectionLogEntry;
import com.yahoo.container.logging.RequestLog;
@@ -72,7 +71,6 @@ import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.UUID;
-import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
@@ -177,7 +175,7 @@ public class HttpServerTest {
@Test
public void requireThatAccessLogIsCalledForRequestRejectedByJetty() throws Exception {
- RequestLogMock requestLogMock = new RequestLogMock();
+ InMemoryRequestLog requestLogMock = new InMemoryRequestLog();
final TestDriver driver = TestDrivers.newConfiguredInstance(
mockRequestHandler(),
new ServerConfig.Builder(),
@@ -186,26 +184,9 @@ public class HttpServerTest {
driver.client().get("/status.html")
.expectStatusCode(is(REQUEST_URI_TOO_LONG));
assertThat(driver.close(), is(true));
-
- assertThat(requestLogMock.logEntries.size(), equalTo(1));
- AccessLogEntry accessLogEntry = requestLogMock.logEntries.get(0);
- assertEquals(414, accessLogEntry.getStatusCode());
- }
-
- private static class RequestLogMock implements RequestLog {
-
- final List<AccessLogEntry> logEntries = new CopyOnWriteArrayList<>();
-
- @Override
- public void log(AccessLogEntry accessLogEntry) {
- logEntries.add(accessLogEntry);
- }
-
- @Override
- public void log(RequestLogEntry entry) {
- // TODO Implement
- }
-
+ assertThat(requestLogMock.entries().size(), equalTo(1));
+ RequestLogEntry entry = requestLogMock.entries().get(0);
+ assertEquals(414, entry.statusCode().getAsInt());
}
@Test
@@ -781,7 +762,7 @@ public class HttpServerTest {
Path privateKeyFile = tmpFolder.newFile().toPath();
Path certificateFile = tmpFolder.newFile().toPath();
generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
- RequestLogMock requestLogMock = new RequestLogMock();
+ InMemoryRequestLog requestLogMock = new InMemoryRequestLog();
InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, /*mixedMode*/connectionLog, false);
@@ -791,9 +772,9 @@ public class HttpServerTest {
sendJettyClientRequest(driver, certificateFile, new V2.Tag(proxiedRemoteAddress, proxiedRemotePort));
assertTrue(driver.close());
- assertEquals(2, requestLogMock.logEntries.size());
- assertLogEntryHasRemote(requestLogMock.logEntries.get(0), proxiedRemoteAddress, proxiedRemotePort);
- assertLogEntryHasRemote(requestLogMock.logEntries.get(1), proxiedRemoteAddress, proxiedRemotePort);
+ assertEquals(2, requestLogMock.entries().size());
+ assertLogEntryHasRemote(requestLogMock.entries().get(0), proxiedRemoteAddress, proxiedRemotePort);
+ assertLogEntryHasRemote(requestLogMock.entries().get(1), proxiedRemoteAddress, proxiedRemotePort);
Assertions.assertThat(connectionLog.logEntries()).hasSize(2);
assertLogEntryHasRemote(connectionLog.logEntries().get(0), proxiedRemoteAddress, proxiedRemotePort);
assertLogEntryHasRemote(connectionLog.logEntries().get(1), proxiedRemoteAddress, proxiedRemotePort);
@@ -804,7 +785,7 @@ public class HttpServerTest {
Path privateKeyFile = tmpFolder.newFile().toPath();
Path certificateFile = tmpFolder.newFile().toPath();
generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
- RequestLogMock requestLogMock = new RequestLogMock();
+ InMemoryRequestLog requestLogMock = new InMemoryRequestLog();
InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, /*mixedMode*/connectionLog, true);
@@ -813,9 +794,9 @@ public class HttpServerTest {
sendJettyClientRequest(driver, certificateFile, new V2.Tag(proxiedRemoteAddress, 12345));
assertTrue(driver.close());
- assertEquals(2, requestLogMock.logEntries.size());
- assertLogEntryHasRemote(requestLogMock.logEntries.get(0), "127.0.0.1", 0);
- assertLogEntryHasRemote(requestLogMock.logEntries.get(1), proxiedRemoteAddress, 0);
+ assertEquals(2, requestLogMock.entries().size());
+ assertLogEntryHasRemote(requestLogMock.entries().get(0), "127.0.0.1", 0);
+ assertLogEntryHasRemote(requestLogMock.entries().get(1), proxiedRemoteAddress, 0);
Assertions.assertThat(connectionLog.logEntries()).hasSize(2);
assertLogEntryHasRemote(connectionLog.logEntries().get(0), null, 0);
assertLogEntryHasRemote(connectionLog.logEntries().get(1), proxiedRemoteAddress, 12345);
@@ -826,7 +807,7 @@ public class HttpServerTest {
Path privateKeyFile = tmpFolder.newFile().toPath();
Path certificateFile = tmpFolder.newFile().toPath();
generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
- RequestLogMock requestLogMock = new RequestLogMock();
+ InMemoryRequestLog requestLogMock = new InMemoryRequestLog();
InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
TestDriver driver = createSslWithProxyProtocolTestDriver(certificateFile, privateKeyFile, requestLogMock, connectionLog, /*mixedMode*/false);
@@ -911,10 +892,10 @@ public class HttpServerTest {
return client;
}
- private static void assertLogEntryHasRemote(AccessLogEntry entry, String expectedAddress, int expectedPort) {
- assertEquals(expectedAddress, entry.getPeerAddress());
+ private static void assertLogEntryHasRemote(RequestLogEntry entry, String expectedAddress, int expectedPort) {
+ assertEquals(expectedAddress, entry.peerAddress().get());
if (expectedPort > 0) {
- assertEquals(expectedPort, entry.getPeerPort());
+ assertEquals(expectedPort, entry.peerPort().getAsInt());
}
}
@@ -932,8 +913,8 @@ public class HttpServerTest {
}
private static TestDriver createSslWithProxyProtocolTestDriver(
- Path certificateFile, Path privateKeyFile, RequestLogMock requestLogMock,
- InMemoryConnectionLog connectionLog, boolean mixedMode) {
+ Path certificateFile, Path privateKeyFile, RequestLog requestLog,
+ ConnectionLog connectionLog, boolean mixedMode) {
ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder()
.proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder()
.enabled(true)
@@ -948,7 +929,7 @@ public class HttpServerTest {
new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)),
connectorConfig,
binder -> {
- binder.bind(RequestLog.class).toInstance(requestLogMock);
+ binder.bind(RequestLog.class).toInstance(requestLog);
binder.bind(ConnectionLog.class).toInstance(connectionLog);
});
}
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/InMemoryRequestLog.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/InMemoryRequestLog.java
new file mode 100644
index 00000000000..b87ec5e8b8b
--- /dev/null
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/InMemoryRequestLog.java
@@ -0,0 +1,20 @@
+// 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.RequestLog;
+import com.yahoo.container.logging.RequestLogEntry;
+
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+/**
+ * @author bjorncs
+ */
+public class InMemoryRequestLog implements RequestLog {
+
+ private final List<RequestLogEntry> entries = new CopyOnWriteArrayList<>();
+
+ @Override public void log(RequestLogEntry entry) { entries.add(entry); }
+
+ List<RequestLogEntry> entries() { return entries; }
+}
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 ffd58902256..a533a447f6a 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
@@ -5,8 +5,8 @@ import com.google.inject.AbstractModule;
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.container.logging.RequestLogEntry;
import com.yahoo.jdisc.http.server.jetty.TestDriver;
import com.yahoo.jdisc.http.server.jetty.TestDrivers;
import org.junit.Test;
@@ -44,7 +44,7 @@ public class ServletAccessLoggingTest extends ServletTestBase {
}
private void verifyCallsLog(RequestLog requestLog, final VerificationMode verificationMode) {
- verify(requestLog, verificationMode).log(any(AccessLogEntry.class));
+ verify(requestLog, verificationMode).log(any(RequestLogEntry.class));
}
private TestDriver newTestDriver(RequestLog requestLog) throws IOException {