Issue #113 - changes from review and bug fixes

%t now takes in locale and timezone in the format string argument
instead of getting it from the setters on the CustomRequestLog class

fixed issue with multiline format strings

requestLog writers are now managed objects

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2018-11-28 14:58:28 +01:00
parent 63aa9ce97d
commit 982895719c
5 changed files with 82 additions and 64 deletions

View File

@ -75,14 +75,14 @@ public class AsyncRequestLogWriter extends RequestLogWriter
AsyncRequestLogWriter.super.write(log);
}
}
catch (IOException e)
{
LOG.warn(e);
}
catch (InterruptedException e)
{
LOG.ignore(e);
}
catch (Throwable t)
{
LOG.warn(t);
}
}
}
}

View File

@ -25,6 +25,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import java.util.regex.Matcher;
@ -199,12 +200,22 @@ import static java.lang.invoke.MethodType.methodType;
* </tr>
*
* <tr>
* <td valign="top">%{format}t</td>
* <td valign="top">%{format|timeZone|locale}t</td>
* <td>
* The time, in the form given by an optional format, parameter (default format [18/Sep/2011:19:18:28 -0400] where
* the last number indicates the timezone offset from GMT.)
* <br>
* The format parameter should be in a format supported by {@link DateCache}
* The time that the request was received.
* Optional parameter in one of the following formats {format}, {format|timeZone} or {format|timeZone|locale}.<br><br>
*
* <pre>
* Format Parameter: (default format [18/Sep/2011:19:18:28 -0400] where the last number indicates the timezone offset from GMT.)
* Must be in a format supported by {@link DateCache}
*
* TimeZone Parameter:
* Default timeZone GMT
* Must be in a format supported by {@link TimeZone#getTimeZone(String)}
*
* Locale Parameter:
* Default locale {@link Locale#getDefault()}
* Must be in a format supported by {@link Locale#forLanguageTag(String)}</pre>
* </td>
* </tr>
*
@ -269,8 +280,6 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
private String[] _ignorePaths;
private transient PathMappings<String> _ignorePathMap;
private Locale _logLocale = Locale.getDefault();
private String _logTimeZone = "GMT";
private RequestLog.Writer _requestLogWriter;
private final MethodHandle _logHandle;
@ -369,7 +378,6 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
return _ignorePaths;
}
/**
* Retrieve the format string.
*
@ -401,48 +409,6 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
super.doStart();
}
/**
* Set the locale of the request log.
*
* @param logLocale locale object
*/
public void setLogLocale(Locale logLocale)
{
_logLocale = logLocale;
}
/**
* Retrieve the locale of the request log.
*
* @return locale object
*/
public Locale getLogLocale()
{
return _logLocale;
}
/**
* Set the timezone of the request log.
*
* @param tz timezone string
*/
public void setLogTimeZone(String tz)
{
_logTimeZone = tz;
}
/**
* Retrieve the timezone of the request log.
*
* @return timezone string
*/
@ManagedAttribute("the timezone")
public String getLogTimeZone()
{
return _logTimeZone;
}
private static void append(StringBuilder buf, String s)
{
if (s == null || s.length() == 0)
@ -487,7 +453,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
{PARAM} is an optional string parameter to the percent code.
CODE is a 1 to 2 character string corresponding to a format code.
*/
final Pattern PATTERN = Pattern.compile("^(?:%(?<MOD>!?[0-9,]+)?(?:\\{(?<ARG>[^}]+)})?(?<CODE>(?:(?:ti)|(?:to)|[a-zA-Z%]))|(?<LITERAL>[^%]+))(?<REMAINING>.*)");
final Pattern PATTERN = Pattern.compile("^(?:%(?<MOD>!?[0-9,]+)?(?:\\{(?<ARG>[^}]+)})?(?<CODE>(?:(?:ti)|(?:to)|[a-zA-Z%]))|(?<LITERAL>[^%]+))(?<REMAINING>.*)", Pattern.DOTALL|Pattern.MULTILINE);
List<Token> tokens = new ArrayList<>();
String remaining = formatString;
@ -835,11 +801,36 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "t":
{
DateCache logDateCache;
if (arg == null || arg.isEmpty())
logDateCache = new DateCache(DEFAULT_DATE_FORMAT, _logLocale, _logTimeZone);
else
logDateCache = new DateCache(arg, _logLocale, _logTimeZone);
String format = DEFAULT_DATE_FORMAT;
TimeZone timeZone = TimeZone.getTimeZone("GMT");
Locale locale = Locale.getDefault();
if (arg != null && !arg.isEmpty())
{
String[] args = arg.split("\\|");
switch (args.length)
{
case 1:
format = args[0];
break;
case 2:
format = args[0];
timeZone = TimeZone.getTimeZone(args[1]);
break;
case 3:
format = args[0];
timeZone = TimeZone.getTimeZone(args[1]);
locale = Locale.forLanguageTag(args[2]);
break;
default:
throw new IllegalArgumentException("Too many \"|\" characters in %t");
}
}
DateCache logDateCache = new DateCache(format, locale, timeZone);
String method = "logRequestTime";
MethodType logTypeDateCache = methodType(Void.TYPE, DateCache.class, StringBuilder.class, Request.class, Response.class);

View File

@ -25,6 +25,8 @@ import java.io.Writer;
import java.util.TimeZone;
import org.eclipse.jetty.util.RolloverFileOutputStream;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@ -32,6 +34,7 @@ import org.eclipse.jetty.util.log.Logger;
/**
* Writer which outputs pre-formatted request log strings to a file using {@link RolloverFileOutputStream}.
*/
@ManagedObject("Request Log writer which writes to file")
public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Writer
{
private static final Logger LOG = Log.getLogger(RequestLogWriter.class);
@ -83,6 +86,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
*
* @return file name of the request log
*/
@ManagedAttribute("filename")
public String getFileName()
{
return _filename;
@ -95,6 +99,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
*
* @return file name of the request log, or null if not applicable
*/
@ManagedAttribute("dated filename")
public String getDatedFilename()
{
if (_fileOut instanceof RolloverFileOutputStream)
@ -102,6 +107,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
return null;
}
@Deprecated
protected boolean isEnabled()
{
return (_fileOut != null);
@ -122,6 +128,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
*
* @return number of days to keep a log file
*/
@ManagedAttribute("number of days to keep a log file")
public int getRetainDays()
{
return _retainDays;
@ -143,6 +150,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
*
* @return value of the flag
*/
@ManagedAttribute("if request log file will be appended after restart")
public boolean isAppend()
{
return _append;
@ -164,6 +172,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
*
* @return the log File Date Format
*/
@ManagedAttribute("log file name date format")
public String getFilenameDateFormat()
{
return _filenameDateFormat;
@ -208,6 +217,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr
_timeZone = timeZone;
}
@ManagedAttribute("timezone of the log")
public String getTimeZone()
{
return _timeZone;

View File

@ -20,9 +20,15 @@ package org.eclipse.jetty.server;
import java.io.IOException;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.log.Slf4jLog;
/**
* Request log writer using a Slf4jLog Logger
*/
@ManagedObject("Slf4j RequestLog Writer")
public class Slf4jRequestLogWriter extends AbstractLifeCycle implements RequestLog.Writer
{
private Slf4jLog logger;
@ -39,6 +45,7 @@ public class Slf4jRequestLogWriter extends AbstractLifeCycle implements RequestL
this.loggerName = loggerName;
}
@ManagedAttribute("logger name")
public String getLoggerName()
{
return loggerName;

View File

@ -27,6 +27,7 @@ import java.net.Socket;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Enumeration;
import java.util.Locale;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.TimeUnit;
@ -373,20 +374,29 @@ public class CustomRequestLogTest
_connector.getResponse("GET / HTTP/1.0\n\n");
String log = _entries.poll(5,TimeUnit.SECONDS);
long requestTime = requestTimes.poll(5,TimeUnit.SECONDS);
DateCache dateCache = new DateCache(_log.DEFAULT_DATE_FORMAT, _log.getLogLocale(), _log.getLogTimeZone());
DateCache dateCache = new DateCache(_log.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT");
assertThat(log, is("RequestTime: ["+ dateCache.format(requestTime) +"]"));
}
@Test
public void testLogRequestTimeCustomFormats() throws Exception
{
testHandlerServerStart("RequestTime: %{EEE MMM dd HH:mm:ss zzz yyyy}t");
testHandlerServerStart("%{EEE MMM dd HH:mm:ss zzz yyyy}t\n" +
"%{EEE MMM dd HH:mm:ss zzz yyyy|EST}t\n" +
"%{EEE MMM dd HH:mm:ss zzz yyyy|EST|ja}t");
_connector.getResponse("GET / HTTP/1.0\n\n");
String log = _entries.poll(5,TimeUnit.SECONDS);
long requestTime = requestTimes.poll(5,TimeUnit.SECONDS);
DateCache dateCache = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", _log.getLogLocale(), _log.getLogTimeZone());
assertThat(log, is("RequestTime: ["+ dateCache.format(requestTime) +"]"));
DateCache dateCache1 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "GMT");
DateCache dateCache2 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "EST");
DateCache dateCache3 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.forLanguageTag("ja"), "EST");
String[] logs = log.split("\n");
assertThat(logs[0], is("["+ dateCache1.format(requestTime) +"]"));
assertThat(logs[1], is("["+ dateCache2.format(requestTime) +"]"));
assertThat(logs[2], is("["+ dateCache3.format(requestTime) +"]"));
}
@Test