diff options
author | Frode Lundgren <frodelu@yahoo-inc.com> | 2016-10-30 01:08:29 +0200 |
---|---|---|
committer | Frode Lundgren <frodelu@yahoo-inc.com> | 2016-10-30 01:08:29 +0200 |
commit | 9abe245c819f9405b162ca5bb7f38f04439ec042 (patch) | |
tree | 5a79bb273ba3b4a0b395842805e99c25e4b6d972 /container-accesslogging | |
parent | 1775306b3024afbf99aadfb525d70777d53701b4 (diff) |
Changes after code review [VESPA-5228]:
* Removed member variables; inlined assignments or create conversion methods
* Pure numeric fields now written as JSON integers rather than strings
Diffstat (limited to 'container-accesslogging')
3 files changed, 97 insertions, 182 deletions
diff --git a/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONAccessLog.java b/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONAccessLog.java index 467618fdc2e..549ca960268 100644 --- a/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONAccessLog.java +++ b/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONAccessLog.java @@ -20,7 +20,7 @@ public final class JSONAccessLog implements AccessLogInterface { } @Override - public void log(final AccessLogEntry logEntry) { + public void log(AccessLogEntry logEntry) { logHandler.access.log(Level.INFO, new JSONFormatter(logEntry).format() + '\n'); } diff --git a/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONFormatter.java b/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONFormatter.java index 4fcc6625a5d..8f2ef90318e 100644 --- a/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONFormatter.java +++ b/container-accesslogging/src/main/java/com/yahoo/container/logging/JSONFormatter.java @@ -25,29 +25,8 @@ public class JSONFormatter { private AccessLogEntry accessLogEntry; private final JsonFactory generatorFactory; - // Access log fields set from access log entry - private String ipV4AddressInDotDecimalNotation; - private String timestampWithMillis; - private String durationBetweenRequestResponseInMillis; - private String numBytesReturned; - private String statusCode; - private String uri; - private String httpVersion; - private String userAgent; - private String totalHits; - private String retrievedHits; - private String httpMethod; - private String hostString; - private String remoteAddress; - private String remotePort; - private String peerAddress; - private String peerPort; - - private Map<String,List<String>> keyValues; - private static Logger logger = Logger.getLogger(JSONFormatter.class.getName()); - public JSONFormatter(final AccessLogEntry entry) { accessLogEntry = entry; generatorFactory = new JsonFactory(); @@ -60,143 +39,94 @@ public class JSONFormatter { * @return The Vespa JSON access log string without trailing newline */ public String format() { - String logString; - - setIpV4Address(accessLogEntry.getIpV4Address()); - setTimeStampMillis(accessLogEntry.getTimeStampMillis()); - setDurationBetweenRequestResponseMillis(accessLogEntry.getDurationBetweenRequestResponseMillis()); - setReturnedContentSize(accessLogEntry.getReturnedContentSize()); - setStatusCode(accessLogEntry.getStatusCode()); - setHitCounts(accessLogEntry.getHitCounts()); - - setRemoteAddress(accessLogEntry.getRemoteAddress()); - setRemotePort(accessLogEntry.getRemotePort()); - - setHttpMethod(accessLogEntry.getHttpMethod()); - setURI(accessLogEntry.getURI()); - setHttpVersion(accessLogEntry.getHttpVersion()); - - setUserAgent(accessLogEntry.getUserAgent()); - - setHostString(accessLogEntry.getHostString()); - setPeerAddress(accessLogEntry.getPeerAddress()); - setPeerPort(accessLogEntry.getPeerPort()); - - keyValues = accessLogEntry.getKeyValues(); - - try { - logString = toJSONAccessEntry(); - } catch (IOException e) { - logString = ""; - } - - return logString; - } - - private String toJSONAccessEntry() throws IOException { ByteArrayOutputStream logLine = new ByteArrayOutputStream(); - JsonGenerator generator = generatorFactory.createGenerator(logLine, JsonEncoding.UTF8); - generator.writeStartObject(); - generator.writeStringField("ip", ipV4AddressInDotDecimalNotation); - generator.writeStringField("time", timestampWithMillis); - generator.writeStringField("duration", durationBetweenRequestResponseInMillis); - generator.writeStringField("size", numBytesReturned); - generator.writeStringField("code", statusCode); - generator.writeStringField("totalhits", totalHits); - generator.writeStringField("hits", retrievedHits); - generator.writeStringField("method", httpMethod); - generator.writeStringField("uri", uri); - generator.writeStringField("version", httpVersion); - generator.writeStringField("agent", userAgent); - generator.writeStringField("host", hostString); - - // Only add remote address/port fields if relevant - if (remoteAddressDiffers(ipV4AddressInDotDecimalNotation, remoteAddress)) { - generator.writeStringField("remoteaddr", remoteAddress); - if (remotePort != null) { - generator.writeStringField("remoteport", remotePort); + try { + JsonGenerator generator = generatorFactory.createGenerator(logLine, JsonEncoding.UTF8); + generator.writeStartObject(); + generator.writeStringField("ip", accessLogEntry.getIpV4Address()); + generator.writeStringField("time", toTimestampWithFraction(accessLogEntry.getTimeStampMillis())); + generator.writeNumberField("duration", + capDuration(accessLogEntry.getDurationBetweenRequestResponseMillis())); + generator.writeNumberField("size", accessLogEntry.getReturnedContentSize()); + generator.writeNumberField("code", accessLogEntry.getStatusCode()); + generator.writeNumberField("totalhits", getTotalHitCount(accessLogEntry.getHitCounts())); + generator.writeNumberField("hits", getRetrievedHitCount(accessLogEntry.getHitCounts())); + generator.writeStringField("method", accessLogEntry.getHttpMethod()); + generator.writeStringField("uri", getNormalizedURI(accessLogEntry.getURI())); + generator.writeStringField("version", accessLogEntry.getHttpVersion()); + generator.writeStringField("agent", accessLogEntry.getUserAgent()); + generator.writeStringField("host", accessLogEntry.getHostString()); + + // 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()); + } } - } - // Only add peer address/port fields if relevant - if (peerAddress != null) { - generator.writeStringField("peeraddr", peerAddress); - if (peerPort != null && !peerPort.equals(remotePort)) { - generator.writeStringField("peerport", peerPort); + // Only add peer address/port fields if relevant + if (accessLogEntry.getPeerAddress() != null) { + generator.writeStringField("peeraddr", accessLogEntry.getPeerAddress()); + + int peerPort = accessLogEntry.getPeerPort(); + if (peerPort > 0 && peerPort != accessLogEntry.getRemotePort()) { + generator.writeNumberField("peerport", peerPort); + } } - } - // 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 - if (keyValues != null && !keyValues.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)); - } else { - generator.writeFieldName(entry.getKey()); - generator.writeStartArray(); - for (String s : entry.getValue()) { - generator.writeString(s); + // 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()) { + 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)); + } else { + generator.writeFieldName(entry.getKey()); + generator.writeStartArray(); + for (String s : entry.getValue()) { + generator.writeString(s); + } + generator.writeEndArray(); } - generator.writeEndArray(); } + generator.writeEndObject(); } + generator.writeEndObject(); - } + generator.close(); - generator.writeEndObject(); - generator.close(); + } catch (IOException e) { + logger.log(Level.WARNING, "Unable to generate JSON access log entry: " + e.getMessage()); + } return logLine.toString(); } + private boolean remoteAddressDiffers(String ipV4Address, String remoteAddress) { return remoteAddress != null && !Objects.equals(ipV4Address, remoteAddress); } - private void setUserAgent(String userAgent) { this.userAgent = userAgent; } - - private void setHttpMethod(String method) { this.httpMethod = method; } - - private void setHttpVersion(String httpVersion) { this.httpVersion = httpVersion; } - - private void setHostString(String hostString) { this.hostString = hostString; } - - private void setRemoteAddress(String remoteAddress) { this.remoteAddress = remoteAddress; } - - private void setPeerAddress(final String peerAddress) { this.peerAddress = peerAddress; } - - private void setRemotePort(int remotePort) { - if (remotePort == 0) { - this.remotePort = null; - } else { - this.remotePort = String.valueOf(remotePort); + private long getTotalHitCount(HitCounts counts) { + if (counts == null) { + return 0; } - } - private void setPeerPort(int peerPort) { - if (peerPort == 0) { - this.peerPort = null; - } else { - this.peerPort = String.valueOf(peerPort); - } + return counts.getTotalHitCount(); } - private void setHitCounts(HitCounts counts) { + private int getRetrievedHitCount(HitCounts counts) { if (counts == null) { - return; + return 0; } - this.totalHits = String.valueOf(counts.getTotalHitCount()); - this.retrievedHits = String.valueOf(counts.getRetrievedHitCount()); - } - - private void setIpV4Address(String ipV4AddressInDotDecimalNotation) { - this.ipV4AddressInDotDecimalNotation = ipV4AddressInDotDecimalNotation; + return counts.getRetrievedHitCount(); } - private void setTimeStampMillis(long numMillisSince1Jan1970AtMidnightUTC) { + private String toTimestampWithFraction(long numMillisSince1Jan1970AtMidnightUTC) { int unixTime = (int)(numMillisSince1Jan1970AtMidnightUTC/1000); int milliSeconds = (int)(numMillisSince1Jan1970AtMidnightUTC % 1000); @@ -207,10 +137,10 @@ public class JSONFormatter { unixTime = (int)(numMillisSince1Jan1970AtMidnightUTC/1000 % 0x7fffffff); } - timestampWithMillis = unixTime + "." + milliSeconds; + return unixTime + "." + milliSeconds; } - private void setDurationBetweenRequestResponseMillis(long timeInMillis) { + private int capDuration(long timeInMillis) { int duration = (int)timeInMillis; if (timeInMillis > 0xffffffffL) { @@ -218,32 +148,17 @@ public class JSONFormatter { duration = 0xffffffff; } - durationBetweenRequestResponseInMillis = String.valueOf(duration); - } - - private void setReturnedContentSize(long byteCount) { - numBytesReturned = String.valueOf(byteCount); - } - - private void setURI(final URI uri) { - setNormalizedURI(uri.normalize()); - } - - private void setNormalizedURI(final URI normalizedUri) { - String uriString = normalizedUri.getPath(); - if (normalizedUri.getRawQuery() != null) { - uriString = uriString + "?" + normalizedUri.getRawQuery(); - } - - this.uri = uriString; + return duration; } - private void setStatusCode(int statusCode) { - if (statusCode == 0) { - return; + private String getNormalizedURI(URI uri) { + URI normalizedURI = uri.normalize(); + String uriString = normalizedURI.getPath(); + if (normalizedURI.getRawQuery() != null) { + uriString = uriString + "?" + normalizedURI.getRawQuery(); } - this.statusCode = String.valueOf(statusCode); + return uriString; } } diff --git a/container-accesslogging/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java b/container-accesslogging/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java index ff85dfb4835..f41e35243ad 100644 --- a/container-accesslogging/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java +++ b/container-accesslogging/src/test/java/com/yahoo/container/logging/JSONLogTestCase.java @@ -40,11 +40,11 @@ public class JSONLogTestCase extends junit.framework.TestCase { String expectedOutput = "{\"ip\":\"152.200.54.243\"," + "\"time\":\"920880005.123\"," + - "\"duration\":\"122\"," + - "\"size\":\"9875\"," + - "\"code\":\"200\"," + - "\"totalhits\":\"1234\"," + - "\"hits\":\"0\"," + + "\"duration\":122," + + "\"size\":9875," + + "\"code\":200," + + "\"totalhits\":1234," + + "\"hits\":0," + "\"method\":\"GET\"," + "\"uri\":\"?query=test\"," + "\"version\":\"HTTP/1.1\"," + @@ -64,11 +64,11 @@ public class JSONLogTestCase extends junit.framework.TestCase { String expectedOutput = "{\"ip\":\"152.200.54.243\"," + "\"time\":\"920880005.123\"," + - "\"duration\":\"122\"," + - "\"size\":\"9875\"," + - "\"code\":\"200\"," + - "\"totalhits\":\"1234\"," + - "\"hits\":\"0\"," + + "\"duration\":122," + + "\"size\":9875," + + "\"code\":200," + + "\"totalhits\":1234," + + "\"hits\":0," + "\"method\":\"GET\"," + "\"uri\":\"?query=test\"," + "\"version\":\"HTTP/1.1\"," + @@ -92,11 +92,11 @@ public class JSONLogTestCase extends junit.framework.TestCase { String expectedOutput = "{\"ip\":\"152.200.54.243\"," + "\"time\":\"920880005.123\"," + - "\"duration\":\"122\"," + - "\"size\":\"9875\"," + - "\"code\":\"200\"," + - "\"totalhits\":\"1234\"," + - "\"hits\":\"0\"," + + "\"duration\":122," + + "\"size\":9875," + + "\"code\":200," + + "\"totalhits\":1234," + + "\"hits\":0," + "\"method\":\"GET\"," + "\"uri\":\"?query=test\"," + "\"version\":\"HTTP/1.1\"," + @@ -113,18 +113,18 @@ public class JSONLogTestCase extends junit.framework.TestCase { expectedOutput = "{\"ip\":\"152.200.54.243\"," + "\"time\":\"920880005.123\"," + - "\"duration\":\"122\"," + - "\"size\":\"9875\"," + - "\"code\":\"200\"," + - "\"totalhits\":\"1234\"," + - "\"hits\":\"0\"," + + "\"duration\":122," + + "\"size\":9875," + + "\"code\":200," + + "\"totalhits\":1234," + + "\"hits\":0," + "\"method\":\"GET\"," + "\"uri\":\"?query=test\"," + "\"version\":\"HTTP/1.1\"," + "\"agent\":\"Mozilla/4.05 [en] (Win95; I)\"," + "\"host\":\"localhost\"," + "\"remoteaddr\":\"FE80:0000:0000:0000:0202:B3FF:FE1E:8329\"," + - "\"remoteport\":\"1234\"" + + "\"remoteport\":1234" + "}"; assertEquals(expectedOutput, new JSONFormatter(entry).format()); @@ -155,11 +155,11 @@ public class JSONLogTestCase extends junit.framework.TestCase { String expectedOutput = "{\"ip\":\"152.200.54.243\"," + "\"time\":\"920880005.123\"," + - "\"duration\":\"122\"," + - "\"size\":\"9875\"," + - "\"code\":\"200\"," + - "\"totalhits\":\"1234\"," + - "\"hits\":\"0\"," + + "\"duration\":122," + + "\"size\":9875," + + "\"code\":200," + + "\"totalhits\":1234," + + "\"hits\":0," + "\"method\":\"GET\"," + "\"uri\":\"?query=test\"," + "\"version\":\"HTTP/1.1\"," + |