This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new a8575ef Change access log time argument to use nanosecond a8575ef is described below commit a8575efbc1de64fe58c8fd99e53a0011b59a8e86 Author: remm <r...@apache.org> AuthorDate: Fri Feb 28 12:28:26 2020 +0100 Change access log time argument to use nanosecond Clear up the %T discrepancies with the usual access log format. --- java/org/apache/catalina/AccessLog.java | 2 +- .../apache/catalina/connector/CoyoteAdapter.java | 5 +-- .../catalina/valves/AbstractAccessLogValve.java | 45 +++++++++++++--------- .../catalina/valves/ExtendedAccessLogValve.java | 2 +- webapps/docs/changelog.xml | 5 +++ webapps/docs/config/valve.xml | 9 +---- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/java/org/apache/catalina/AccessLog.java b/java/org/apache/catalina/AccessLog.java index 138f9f4..6674309 100644 --- a/java/org/apache/catalina/AccessLog.java +++ b/java/org/apache/catalina/AccessLog.java @@ -76,7 +76,7 @@ public interface AccessLog { * @param request Request (associated with the response) to log * @param response Response (associated with the request) to log * @param time Time taken to process the request/response in - * milliseconds (use 0 if not known) + * nanoseconds (use 0 if not known) */ public void log(Request request, Response response, long time); diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 18b8025..f448f8f 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.EnumSet; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import jakarta.servlet.ReadListener; @@ -275,7 +274,7 @@ public class CoyoteAdapter implements Adapter { if (!success || !request.isAsync()) { long time = 0; if (req.getStartTimeNanos() != -1) { - time = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - req.getStartTimeNanos()); + time = System.nanoTime() - req.getStartTimeNanos(); } Context context = request.getContext(); if (context != null) { @@ -403,7 +402,7 @@ public class CoyoteAdapter implements Adapter { // The other possibility is that an error occurred early in // processing and the request could not be mapped to a Context. // Log via the host or engine in that case. - long time = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - req.getStartTimeNanos()); + long time = System.nanoTime() - req.getStartTimeNanos(); if (context != null) { context.logAccess(request, response, time, false); } else if (response.isError()) { diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java b/java/org/apache/catalina/valves/AbstractAccessLogValve.java index 5cc1f03..1f00595 100644 --- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java +++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java @@ -700,9 +700,8 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access return; } - long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - request.getCoyoteRequest().getStartTimeNanos()); // Date for access log should be the beginning of the request - Date date = getDate(System.currentTimeMillis() - elapsed); + Date date = getDate(request.getCoyoteRequest().getStartTime()); CharArrayWriter result = charArrayWriters.pop(); if (result == null) { @@ -1109,8 +1108,8 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access Response response, long time) { long timestamp = date.getTime(); long frac; - if (usesBegin) { - timestamp -= time; + if (!usesBegin) { + timestamp += TimeUnit.NANOSECONDS.toMillis(time); } /* Implementation note: This is deliberately not implemented using * switch. If a switch is used the compiler (at least the Oracle @@ -1330,30 +1329,29 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access * write time taken to process the request - %D, %T */ protected static class ElapsedTimeElement implements AccessLogElement { + private final boolean micros; private final boolean millis; /** - * @param millis <code>true</code>, write time in millis - %D, - * if <code>false</code>, write time in seconds - %T + * @param micros <code>true</code>, write time in micros - %D + * @param millis <code>true</code>, write time in millis, + * if both arguments are <code>false</code>, write time in seconds - %T */ - public ElapsedTimeElement(boolean millis) { + public ElapsedTimeElement(boolean micros, boolean millis) { + this.micros = micros; this.millis = millis; } @Override public void addElement(CharArrayWriter buf, Date date, Request request, Response response, long time) { - if (millis) { - buf.append(Long.toString(time)); + if (micros) { + buf.append(Long.toString(TimeUnit.NANOSECONDS.toMicros(time))); + } else if (millis) { + buf.append(Long.toString(TimeUnit.NANOSECONDS.toMillis(time))); } else { // second - buf.append(Long.toString(time / 1000)); - buf.append('.'); - int remains = (int) (time % 1000); - buf.append(Long.toString(remains / 100)); - remains = remains % 100; - buf.append(Long.toString(remains / 10)); - buf.append(Long.toString(remains % 10)); + buf.append(Long.toString(TimeUnit.NANOSECONDS.toSeconds(time))); } } } @@ -1364,7 +1362,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access protected static class FirstByteTimeElement implements AccessLogElement { @Override public void addElement(CharArrayWriter buf, Date date, Request request, Response response, long time) { - long commitTime = response.getCoyoteResponse().getCommitTime(); + long commitTime = response.getCoyoteResponse().getCommitTimeNanos(); if (commitTime == -1) { buf.append('-'); } else { @@ -1745,6 +1743,15 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access return new SessionAttributeElement(name); case 't': return new DateAndTimeElement(name); + case 'T': + // ms for milliseconds, us for microseconds, and s for seconds + if ("ms".equals(name)) { + return new ElapsedTimeElement(false, true); + } else if ("us".equals(name)) { + return new ElapsedTimeElement(true, false); + } else { + return new ElapsedTimeElement(false, false); + } default: return new StringElement("???"); } @@ -1766,7 +1773,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access case 'B': return new ByteSentElement(false); case 'D': - return new ElapsedTimeElement(true); + return new ElapsedTimeElement(true, false); case 'F': return new FirstByteTimeElement(); case 'h': @@ -1790,7 +1797,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access case 't': return new DateAndTimeElement(); case 'T': - return new ElapsedTimeElement(false); + return new ElapsedTimeElement(false, false); case 'u': return new UserElement(); case 'U': diff --git a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java index f453760..10893d0 100644 --- a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java +++ b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java @@ -584,7 +584,7 @@ public class ExtendedAccessLogValve extends AccessLogValve { if (tokenizer.hasSubToken()) { String nextToken = tokenizer.getToken(); if ("taken".equals(nextToken)) { - return new ElapsedTimeElement(false); + return new ElapsedTimeElement(false, false); } } else { return new TimeElement(); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 513420c..32386d3 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -73,6 +73,11 @@ <code>org.apache.tomcat.util.digester.EnvironmentPropertySource</code>. Patch provided by Bernd Bohmann. (markt) </scode> + <fix> + <bug>63286</bug>: Resolve inconsistencies with access log valve. This + changes the element API to use a nanosecond resolution elapsed time + argument. (remm) + </fix> </changelog> </subsection> <subsection name="Coyote"> diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml index 613a0a6..daaa77d 100644 --- a/webapps/docs/config/valve.xml +++ b/webapps/docs/config/valve.xml @@ -304,13 +304,8 @@ <li><b>%u</b> - Remote user that was authenticated (if any), else '-'</li> <li><b>%U</b> - Requested URL path</li> <li><b>%v</b> - Local server name</li> - <li><b>%D</b> - Time taken to process the request in millis. Note: In - httpd %D is microseconds. Behaviour will be aligned to httpd - in Tomcat 10 onwards.</li> - <li><b>%T</b> - Time taken to process the request, in seconds. Note: This - value has millisecond resolution whereas in httpd it has - second resolution. Behaviour will be align to httpd - in Tomcat 10 onwards.</li> + <li><b>%D</b> - Time taken to process the request in millis.</li> + <li><b>%T</b> - Time taken to process the request, in seconds.</li> <li><b>%F</b> - Time taken to commit the response, in millis</li> <li><b>%I</b> - Current request thread name (can compare later with stacktraces)</li> <li><b>%X</b> - Connection status when response is completed: --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org