Merge changes I238c2692,I365109d3,I3a2bdba5

* changes:
  Respect enableReverseDnsLookup option in ErrorLogJsonLayout
  Use ISO 8601 timestamp format for error/httpd/sshd logs
  Do not use jsonevent-layout for error logs in JSON format

Documentation/licenses.txt

@@ -72,8 +72,6 @@ Apache2.0
 * jetty:server
 * jetty:servlet
 * jetty:util
-* log:json-smart
-* log:jsonevent-layout
 * log:log4j
 * lucene:lucene-analyzers-common
 * lucene:lucene-core-and-backward-codecs-merged

WORKSPACE

@@ -322,12 +322,6 @@ maven_jar(
     sha1 = "5af35056b4d257e4b64b9e8069c0746e8b08629f",
 )
 
-maven_jar(
-    name = "jsonevent-layout",
-    artifact = "net.logstash.log4j:jsonevent-layout:1.7",
-    sha1 = "507713504f0ddb75ba512f62763519c43cf46fde",
-)
-
 maven_jar(
     name = "json-smart",
     artifact = "net.minidev:json-smart:1.1.1",

java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java

@@ -26,16 +26,10 @@ import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER_AGENT;
 
 import com.google.gerrit.util.logging.JsonLayout;
 import com.google.gerrit.util.logging.JsonLogEntry;
-import java.time.format.DateTimeFormatter;
 import org.apache.log4j.spi.LoggingEvent;
 
 public class HttpLogJsonLayout extends JsonLayout {
 
-  @Override
-  public DateTimeFormatter createDateTimeFormatter() {
-    return DateTimeFormatter.ofPattern("dd/MMM/yyyy:HH:mm:ss,SSS Z");
-  }
-
   @Override
   public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
     return new HttpJsonLogEntry(event);
@@ -59,7 +53,7 @@ public class HttpLogJsonLayout extends JsonLayout {
       this.host = getMdcString(event, P_HOST);
       this.thread = event.getThreadName();
       this.user = getMdcString(event, P_USER);
-      this.timestamp = formatDate(event.getTimeStamp());
+      this.timestamp = timestampFormatter.format(event.getTimeStamp());
       this.method = getMdcString(event, P_METHOD);
       this.resource = getMdcString(event, P_RESOURCE);
       this.protocol = getMdcString(event, P_PROTOCOL);

java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java

@@ -14,24 +14,15 @@
 
 package com.google.gerrit.pgm.http.jetty;
 
-import java.text.SimpleDateFormat;
+import com.google.gerrit.util.logging.LogTimestampFormatter;
-import java.util.Date;
-import java.util.TimeZone;
 import org.apache.log4j.Layout;
 import org.apache.log4j.spi.LoggingEvent;
 
 public final class HttpLogLayout extends Layout {
-  private final SimpleDateFormat dateFormat;
+  private final LogTimestampFormatter timestampFormatter;
-  private long lastTimeMillis;
-  private String lastTimeString;
 
   public HttpLogLayout() {
-    final TimeZone tz = TimeZone.getDefault();
+    timestampFormatter = new LogTimestampFormatter();
-    dateFormat = new SimpleDateFormat("dd/MMM/yyyy:HH:mm:ss Z");
-    dateFormat.setTimeZone(tz);
-
-    lastTimeMillis = System.currentTimeMillis();
-    lastTimeString = dateFormat.format(new Date(lastTimeMillis));
   }
 
   @Override
@@ -53,7 +44,7 @@ public final class HttpLogLayout extends Layout {
 
     buf.append(' ');
     buf.append('[');
-    formatDate(event.getTimeStamp(), buf);
+    buf.append(timestampFormatter.format(event.getTimeStamp()));
     buf.append(']');
 
     buf.append(' ');
@@ -104,19 +95,6 @@ public final class HttpLogLayout extends Layout {
     }
   }
 
-  private void formatDate(long now, StringBuilder sbuf) {
-    final long rounded = now - (int) (now % 1000);
-    if (rounded != lastTimeMillis) {
-      synchronized (dateFormat) {
-        lastTimeMillis = rounded;
-        lastTimeString = dateFormat.format(new Date(lastTimeMillis));
-        sbuf.append(lastTimeString);
-      }
-    } else {
-      sbuf.append(lastTimeString);
-    }
-  }
-
   @Override
   public boolean ignoresThrowable() {
     return true;

java/com/google/gerrit/pgm/util/BUILD

@@ -17,12 +17,13 @@ java_library(
         "//java/com/google/gerrit/server/restapi",
         "//java/com/google/gerrit/server/schema",
         "//java/com/google/gerrit/util/cli",
+        "//java/com/google/gerrit/util/logging",
         "//lib:args4j",
+        "//lib:gson",
         "//lib:guava",
         "//lib:jgit",
         "//lib/flogger:api",
         "//lib/guice",
-        "//lib/log:jsonevent-layout",
         "//lib/log:log4j",
     ],
 )

java/com/google/gerrit/pgm/util/ErrorLogFile.java

@@ -18,9 +18,9 @@ import com.google.gerrit.common.FileUtil;
 import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.util.SystemLog;
+import com.google.gerrit.util.logging.LogTimestampFormatter;
 import java.io.IOException;
 import java.nio.file.Path;
-import net.logstash.log4j.JSONEventLayoutV1;
 import org.apache.log4j.ConsoleAppender;
 import org.apache.log4j.Level;
 import org.apache.log4j.LogManager;
@@ -78,13 +78,22 @@ public class ErrorLogFile {
     if (text) {
       root.addAppender(
           SystemLog.createAppender(
-              logdir, LOG_NAME, new PatternLayout("[%d] [%t] %-5p %c %x: %m%n"), rotate));
+              logdir,
+              LOG_NAME,
+              new PatternLayout(
+                  "[%d{" + LogTimestampFormatter.TIMESTAMP_FORMAT + "}] [%t] %-5p %c %x: %m%n"),
+              rotate));
     }
 
     if (json) {
+      Boolean enableReverseDnsLookup =
+          config.getBoolean("gerrit", null, "enableReverseDnsLookup", false);
       root.addAppender(
           SystemLog.createAppender(
-              logdir, LOG_NAME + JSON_SUFFIX, new JSONEventLayoutV1(), rotate));
+              logdir,
+              LOG_NAME + JSON_SUFFIX,
+              new ErrorLogJsonLayout(enableReverseDnsLookup),
+              rotate));
     }
   }
 }

java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java

@@ -0,0 +1,138 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.pgm.util;
+
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
+import com.google.gson.annotations.SerializedName;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.log4j.spi.LoggingEvent;
+import org.apache.log4j.spi.ThrowableInformation;
+
+/** Layout for formatting error log events in the JSON format. */
+public class ErrorLogJsonLayout extends JsonLayout {
+  private final Boolean enableReverseDnsLookup;
+
+  public ErrorLogJsonLayout(Boolean enableDnsReverseLookup) {
+    super();
+    this.enableReverseDnsLookup = enableDnsReverseLookup;
+  }
+
+  @Override
+  public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
+    return new ErrorJsonLogEntry(event);
+  }
+
+  @SuppressWarnings("unused")
+  private class ErrorJsonLogEntry extends JsonLogEntry {
+    /** Timestamp of when the log entry was created. */
+    @SerializedName("@timestamp")
+    public final String timestamp;
+
+    /** Hostname of the machine running Gerrit. */
+    public final String sourceHost;
+    /** Logged message. */
+    public final String message;
+    /** File containing the code creating the log entry. */
+    public final String file;
+    /** Line number of code creating the log entry. */
+    public final String lineNumber;
+
+    /** Class from which the log entry was created. */
+    @SerializedName("class")
+    public final String clazz;
+
+    /** Method from which the log entry was created. */
+    public final String method;
+    /** Name of the logger creating the log entry. */
+    public final String loggerName;
+
+    /** Mapped diagnostic context. */
+    @SuppressWarnings("rawtypes")
+    public final Map mdc;
+
+    /** Nested diagnostic context. */
+    public final String ndc;
+    /** Logging level/severity. */
+    public final String level;
+    /** Thread executing the code creating the log entry. */
+    public final String threadName;
+
+    /** Version of log format. */
+    @SerializedName("@version")
+    public final int version = 2;
+
+    /**
+     * Map containing information of a logged exception. It contains the following key-value pairs:
+     * exception_class: Which class threw the exception exception_method: Which method threw the
+     * exception stacktrace: The exception stacktrace
+     */
+    public Map<String, String> exception;
+
+    public ErrorJsonLogEntry(LoggingEvent event) {
+      this.timestamp = timestampFormatter.format(event.getTimeStamp());
+      this.sourceHost = getSourceHost(enableReverseDnsLookup);
+      this.message = event.getRenderedMessage();
+      this.file = event.getLocationInformation().getFileName();
+      this.lineNumber = event.getLocationInformation().getLineNumber();
+      this.clazz = event.getLocationInformation().getClassName();
+      this.method = event.getLocationInformation().getMethodName();
+      this.loggerName = event.getLoggerName();
+      this.mdc = event.getProperties();
+      this.ndc = event.getNDC();
+      this.level = event.getLevel().toString();
+      this.threadName = event.getThreadName();
+      if (event.getThrowableInformation() != null) {
+        this.exception = getException(event.getThrowableInformation());
+      }
+    }
+
+    private String getSourceHost(Boolean enableReverseDnsLookup) {
+      InetAddress in;
+      try {
+        in = InetAddress.getLocalHost();
+        if (Boolean.TRUE.equals(enableReverseDnsLookup)) {
+          return in.getCanonicalHostName();
+        }
+        return in.getHostAddress();
+      } catch (UnknownHostException e) {
+        return "unknown-host";
+      }
+    }
+
+    private Map<String, String> getException(ThrowableInformation throwable) {
+      HashMap<String, String> exceptionInformation = new HashMap<>();
+
+      String throwableName = throwable.getThrowable().getClass().getCanonicalName();
+      if (throwableName != null) {
+        exceptionInformation.put("exception_class", throwableName);
+      }
+
+      String throwableMessage = throwable.getThrowable().getMessage();
+      if (throwableMessage != null) {
+        exceptionInformation.put("exception_message", throwableMessage);
+      }
+
+      String[] stackTrace = throwable.getThrowableStrRep();
+      if (stackTrace != null) {
+        exceptionInformation.put("stacktrace", String.join("\n", stackTrace));
+      }
+      return exceptionInformation;
+    }
+  }
+}

java/com/google/gerrit/sshd/SshLogJsonLayout.java

@@ -25,16 +25,10 @@ import static com.google.gerrit.sshd.SshLog.P_WAIT;
 
 import com.google.gerrit.util.logging.JsonLayout;
 import com.google.gerrit.util.logging.JsonLogEntry;
-import java.time.format.DateTimeFormatter;
 import org.apache.log4j.spi.LoggingEvent;
 
 public class SshLogJsonLayout extends JsonLayout {
 
-  @Override
-  public DateTimeFormatter createDateTimeFormatter() {
-    return DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss,SSS Z");
-  }
-
   @Override
   public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
     return new SshJsonLogEntry(event);
@@ -65,7 +59,7 @@ public class SshLogJsonLayout extends JsonLayout {
     public String bytesTotal;
 
     public SshJsonLogEntry(LoggingEvent event) {
-      this.timestamp = formatDate(event.getTimeStamp());
+      this.timestamp = timestampFormatter.format(event.getTimeStamp());
       this.session = getMdcString(event, P_SESSION);
       this.thread = event.getThreadName();
       this.user = getMdcString(event, P_USER_NAME);

java/com/google/gerrit/sshd/SshLogLayout.java

@@ -23,27 +23,16 @@ import static com.google.gerrit.sshd.SshLog.P_STATUS;
 import static com.google.gerrit.sshd.SshLog.P_USER_NAME;
 import static com.google.gerrit.sshd.SshLog.P_WAIT;
 
-import java.text.SimpleDateFormat;
+import com.google.gerrit.util.logging.LogTimestampFormatter;
-import java.util.Calendar;
-import java.util.TimeZone;
 import org.apache.log4j.Layout;
 import org.apache.log4j.spi.LoggingEvent;
 import org.eclipse.jgit.util.QuotedString;
 
 public final class SshLogLayout extends Layout {
-
+  protected final LogTimestampFormatter timestampFormatter;
-  private final Calendar calendar;
-  private long lastTimeMillis;
-  private final char[] lastTimeString = new char[20];
-  private final SimpleDateFormat tzFormat;
-  private char[] timeZone;
 
   public SshLogLayout() {
-    final TimeZone tz = TimeZone.getDefault();
+    timestampFormatter = new LogTimestampFormatter();
-    calendar = Calendar.getInstance(tz);
-
-    tzFormat = new SimpleDateFormat("Z");
-    tzFormat.setTimeZone(tz);
   }
 
   @Override
@@ -51,7 +40,7 @@ public final class SshLogLayout extends Layout {
     final StringBuffer buf = new StringBuffer(128);
 
     buf.append('[');
-    formatDate(event.getTimeStamp(), buf);
+    buf.append(timestampFormatter.format(event.getTimeStamp()));
     buf.append(']');
 
     req(P_SESSION, buf, event);
@@ -77,41 +66,6 @@ public final class SshLogLayout extends Layout {
     return buf.toString();
   }
 
-  private void formatDate(long now, StringBuffer sbuf) {
-    final int millis = (int) (now % 1000);
-    final long rounded = now - millis;
-    if (rounded != lastTimeMillis) {
-      synchronized (calendar) {
-        final int start = sbuf.length();
-        calendar.setTimeInMillis(rounded);
-        sbuf.append(calendar.get(Calendar.YEAR));
-        sbuf.append('-');
-        sbuf.append(toTwoDigits(calendar.get(Calendar.MONTH) + 1));
-        sbuf.append('-');
-        sbuf.append(toTwoDigits(calendar.get(Calendar.DAY_OF_MONTH)));
-        sbuf.append(' ');
-        sbuf.append(toTwoDigits(calendar.get(Calendar.HOUR_OF_DAY)));
-        sbuf.append(':');
-        sbuf.append(toTwoDigits(calendar.get(Calendar.MINUTE)));
-        sbuf.append(':');
-        sbuf.append(toTwoDigits(calendar.get(Calendar.SECOND)));
-        sbuf.append(',');
-        sbuf.getChars(start, sbuf.length(), lastTimeString, 0);
-        lastTimeMillis = rounded;
-        timeZone = tzFormat.format(calendar.getTime()).toCharArray();
-      }
-    } else {
-      sbuf.append(lastTimeString);
-    }
-    sbuf.append(String.format("%03d", millis));
-    sbuf.append(' ');
-    sbuf.append(timeZone);
-  }
-
-  private String toTwoDigits(int input) {
-    return String.format("%02d", input);
-  }
-
   private void req(String key, StringBuffer buf, LoggingEvent event) {
     Object val = event.getMDC(key);
     buf.append(' ');

java/com/google/gerrit/util/logging/JsonLayout.java

@@ -17,30 +17,18 @@ package com.google.gerrit.util.logging;
 import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
-import java.time.Instant;
-import java.time.LocalDateTime;
-import java.time.OffsetDateTime;
-import java.time.ZoneId;
-import java.time.ZoneOffset;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
 import org.apache.log4j.Layout;
 import org.apache.log4j.spi.LoggingEvent;
 
 public abstract class JsonLayout extends Layout {
-  private final DateTimeFormatter dateFormatter;
   private final Gson gson;
-  private final ZoneOffset timeOffset;
+  protected final LogTimestampFormatter timestampFormatter;
 
   public JsonLayout() {
-    dateFormatter = createDateTimeFormatter();
+    timestampFormatter = new LogTimestampFormatter();
-    timeOffset = OffsetDateTime.now(ZoneId.systemDefault()).getOffset();
-
     gson = newGson();
   }
 
-  public abstract DateTimeFormatter createDateTimeFormatter();
-
   public abstract JsonLogEntry toJsonLogEntry(LoggingEvent event);
 
   @Override
@@ -56,12 +44,6 @@ public abstract class JsonLayout extends Layout {
     return gb.create();
   }
 
-  public String formatDate(long now) {
-    return ZonedDateTime.of(
-            LocalDateTime.ofInstant(Instant.ofEpochMilli(now), timeOffset), ZoneId.systemDefault())
-        .format(dateFormatter);
-  }
-
   @Override
   public void activateOptions() {}
 

java/com/google/gerrit/util/logging/LogTimestampFormatter.java

@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.util.logging;
+
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+
+/** Formatter for timestamps used in log entries. */
+public class LogTimestampFormatter {
+  public static final String TIMESTAMP_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSSZ";
+
+  private final DateTimeFormatter dateFormatter;
+  private final ZoneOffset timeOffset;
+
+  public LogTimestampFormatter() {
+    dateFormatter = DateTimeFormatter.ofPattern(TIMESTAMP_FORMAT);
+    timeOffset = OffsetDateTime.now(ZoneId.systemDefault()).getOffset();
+  }
+
+  /**
+   * Formats time given in milliseconds since UNIX epoch to ISO 8601 format.
+   *
+   * @param epochMilli milliseconds since UNIX epoch
+   * @return ISO 8601-formatted timestamp
+   */
+  public String format(long epochMilli) {
+    return ZonedDateTime.of(
+            LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMilli), timeOffset),
+            ZoneId.systemDefault())
+        .format(dateFormatter);
+  }
+}

lib/log/BUILD

@@ -40,17 +40,6 @@ java_library(
     exports = ["@log4j//jar"],
 )
 
-java_library(
-    name = "jsonevent-layout",
-    data = ["//lib:LICENSE-Apache2.0"],
-    visibility = ["//visibility:public"],
-    exports = ["@jsonevent-layout//jar"],
-    runtime_deps = [
-        ":json-smart",
-        "//lib/commons:lang",
-    ],
-)
-
 java_library(
     name = "json-smart",
     data = ["//lib:LICENSE-Apache2.0"],