Issue GoogleCloudPlatform#231 - More post-review updates to logging u…

…pdates
jetty-project · Aug 29, 2016 · 204efa3 · 204efa3
1 parent 0c55fd7
commit 204efa3
Show file tree

Hide file tree

Showing 12 changed files with 140 additions and 124 deletions.
diff --git a/...ng/vmruntime/VmRuntimeFileLogHandler.java → ...ting/logging/CloudLoggingFileHandler.java b/...ng/vmruntime/VmRuntimeFileLogHandler.java → ...ting/logging/CloudLoggingFileHandler.java
@@ -14,42 +14,45 @@
  * limitations under the License.
  */
 
-package com.google.apphosting.vmruntime;
-
-import com.google.apphosting.logging.JsonFormatter;
-import com.google.apphosting.logging.SystemLogger;
+package com.google.apphosting.logging;
 
 import java.io.IOException;
-import java.util.logging.ConsoleHandler;
 import java.util.logging.FileHandler;
 import java.util.logging.Handler;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * {@code VmRuntimeFileLogHandler} is installed on the root logger. It converts all messages
- * to the json format understood by the cloud logging agent and logs to a file in a volume shared
- * with the cloud logging agent.
- *
+ * A root level {@link java.util.logging.FileHandler} for interfacing with the GCloud logging
+ * console. It converts all logging events to the json format understood by the cloud logging
+ * agent and logs to a file in a volume shared with the cloud logging agent.
  */
-public class VmRuntimeFileLogHandler extends FileHandler implements SystemLogger {
+public class CloudLoggingFileHandler extends FileHandler implements SystemLogger {
   // This exists for testing purposes only.  If set, the cloud logger may lose logs.
   public static final String LOG_DIRECTORY_PROPERTY = "com.google.apphosting.logs";
   public static final String LOG_PATTERN_CONFIG_PROPERTY =
-      "com.google.apphosting.vmruntime.VmRuntimeFileLogHandler.pattern";
+      CloudLoggingFileHandler.class.getName() + ".pattern";
   // Log files to /var/log/app_engine/app.[0-2].log.json
   private static final String DEFAULT_LOG_DIRECTORY = "/var/log/app_engine";
   private static final String DEFAULT_LOG_PATTERN = "app.%u.%g.log.json";
   private static final String APP_ENGINE_LOG_CONFIG_PATTERN_ENV = "APP_ENGINE_LOG_CONFIG_PATTERN";
-  private static final int LOG_PER_FILE_SIZE = 100 * 1024 * 1024;
-  private static final int LOG_MAX_FILES = 3;
+  private static final int LOG_PER_FILE_SIZE = getConfigInt("limit", 100 * 1024 * 1024);
+  private static final int LOG_MAX_FILES = getConfigInt("count", 3);
 
-  public VmRuntimeFileLogHandler() throws IOException {
+  public CloudLoggingFileHandler() throws IOException {
     super(fileLogPattern(), LOG_PER_FILE_SIZE, LOG_MAX_FILES, true);
     setLevel(Level.FINEST);
     setFormatter(new JsonFormatter());
   }
 
+  private static int getConfigInt(String suffix, int defValue) {
+    String val = System.getProperty(CloudLoggingFileHandler.class.getName() + '.' + suffix);
+    if (val == null) {
+      return defValue;
+    }
+    return Integer.parseInt(val);
+  }
+
   private static String fileLogPattern() {
     String pattern = System.getenv(APP_ENGINE_LOG_CONFIG_PATTERN_ENV);
     // For Cloud SDK usage only for local Jetty processes.
@@ -76,12 +79,12 @@ public void configure() throws IOException {
 
     // Look for the expected Handlers
     for (Handler handler : rootLogger.getHandlers()) {
-      if (handler instanceof VmRuntimeFileLogHandler) {
+      if (handler instanceof CloudLoggingFileHandler) {
         // Nothing else to do, return
         return;
       }
     }
 
-    rootLogger.addHandler(new VmRuntimeFileLogHandler());
+    rootLogger.addHandler(new CloudLoggingFileHandler());
   }
 }
diff --git a/appengine-java-logging/src/main/java/com/google/apphosting/logging/LogContext.java b/appengine-java-logging/src/main/java/com/google/apphosting/logging/LogContext.java
@@ -23,8 +23,9 @@
 /**
  * The context for logging information associated with the current Thread.
  * <p>
- * <p>This is an implementation of a Mapped Diagnostic Context for use with the java.util.logging
+ * This is an implementation of a Mapped Diagnostic Context for use with the java.util.logging
  * framework.
+ * </p>
  */
 public class LogContext extends ConcurrentHashMap<String, Object> {
 

diff --git a/...ngine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java b/...ngine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java
@@ -22,16 +22,16 @@
 import java.util.concurrent.ThreadLocalRandom;
 
 class CommonsLoggingExample implements Runnable {
-  private static final Log LOG = LogFactory.getLog(CommonsLoggingExample.class);
+  private static final Log logger = LogFactory.getLog(CommonsLoggingExample.class);
 
   @Override
   public void run() {
     ThreadLocalRandom rand = ThreadLocalRandom.current();
-    LOG.trace(String.format("A CommonsLogging Trace Event: %d", rand.nextInt()));
-    LOG.debug(String.format("A CommonsLogging Debug Event: %d", rand.nextInt()));
-    LOG.info(String.format("A CommonsLogging Info Event: %d", rand.nextInt()));
-    LOG.warn(String.format("A CommonsLogging Warn Event: %d", rand.nextInt()));
-    LOG.error(String.format("A CommonsLogging Error Event: %d", rand.nextInt()),
+    logger.trace(String.format("A CommonsLogging Trace Event: %d", rand.nextInt()));
+    logger.debug(String.format("A CommonsLogging Debug Event: %d", rand.nextInt()));
+    logger.info(String.format("A CommonsLogging Info Event: %d", rand.nextInt()));
+    logger.warn(String.format("A CommonsLogging Warn Event: %d", rand.nextInt()));
+    logger.error(String.format("A CommonsLogging Error Event: %d", rand.nextInt()),
         new RuntimeException("Generic Error"));
   }
 }
diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java
@@ -21,14 +21,9 @@
 import java.util.logging.Handler;
 import java.util.logging.LogRecord;
 
-public class JsonCaptureHandler extends Handler {
-  private final JsonFormatter formatter;
-  private List<String> events;
-
-  public JsonCaptureHandler() {
-    formatter = new JsonFormatter();
-    events = new ArrayList<>();
-  }
+public final class JsonCaptureHandler extends Handler {
+  private final JsonFormatter formatter = new JsonFormatter();
+  private List<String> events = new ArrayList<>();
 
   @Override
   public void publish(LogRecord record) {
@@ -40,7 +35,7 @@ public void flush() {
   }
 
   @Override
-  public void close() throws SecurityException {
+  public void close() {
   }
 
   public List<String> getEvents() {

diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java
@@ -110,8 +110,8 @@ public void messageIncludesStackTrace() throws Exception {
 
   @Test
   public void testCommonsLogging() {
-    final Instant now = Instant.now();
-    final String expectedThreadName = Thread.currentThread().getName();
+    Instant now = Instant.now();
+    String expectedThreadName = Thread.currentThread().getName();
 
     // Generate Events from Commons Logging
     new CommonsLoggingExample().run();
@@ -143,8 +143,8 @@ public void testCommonsLogging() {
 
   @Test
   public void testJavaUtilLogging() {
-    final Instant now = Instant.now();
-    final String expectedThreadName = Thread.currentThread().getName();
+    Instant now = Instant.now();
+    String expectedThreadName = Thread.currentThread().getName();
 
     // Generate Events from java.util.logging
     new JulExample().run();
@@ -178,8 +178,8 @@ public void testJavaUtilLogging() {
 
   @Test
   public void testLog4jLogging() {
-    final Instant now = Instant.now();
-    final String expectedThreadName = Thread.currentThread().getName();
+    Instant now = Instant.now();
+    String expectedThreadName = Thread.currentThread().getName();
 
     // Generate Events from Apache Log4j Logging
     new Log4jExample().run();
@@ -212,8 +212,8 @@ public void testLog4jLogging() {
 
   @Test
   public void testSlf4jLogging() {
-    final Instant now = Instant.now();
-    final String expectedThreadName = Thread.currentThread().getName();
+    Instant now = Instant.now();
+    String expectedThreadName = Thread.currentThread().getName();
 
     // Generate Events from Slf4j Logging
     new Slf4jExample().run();
@@ -252,7 +252,6 @@ public static class LogTimestamp {
       public int nanos;
     }
 
-
     public LogTimestamp timestamp;
     public String severity;
     public String thread;

diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java
@@ -21,18 +21,18 @@
 import java.util.logging.Logger;
 
 class JulExample implements Runnable {
-  public static final Logger LOG = Logger.getLogger(JulExample.class.getName());
+  public static final Logger logger = Logger.getLogger(JulExample.class.getName());
 
   @Override
   public void run() {
     ThreadLocalRandom rand = ThreadLocalRandom.current();
-    LOG.log(Level.FINEST, "A JUL Finest Event: {0}", rand.nextInt());
-    LOG.log(Level.FINER, "A JUL Finer Event: {0}", rand.nextInt());
-    LOG.log(Level.FINE, "A JUL Fine Event: {0}", rand.nextInt());
-    LOG.log(Level.CONFIG, "A JUL Config Event: {0}", rand.nextInt());
-    LOG.log(Level.INFO, "A JUL Info Event: {0}", rand.nextInt());
-    LOG.log(Level.WARNING, "A JUL Warning Event: {0}", rand.nextInt());
-    LOG.log(Level.SEVERE, String.format("A JUL Severe Event: %d", rand.nextInt()),
+    logger.log(Level.FINEST, "A JUL Finest Event: {0}", rand.nextInt());
+    logger.log(Level.FINER, "A JUL Finer Event: {0}", rand.nextInt());
+    logger.log(Level.FINE, "A JUL Fine Event: {0}", rand.nextInt());
+    logger.log(Level.CONFIG, "A JUL Config Event: {0}", rand.nextInt());
+    logger.log(Level.INFO, "A JUL Info Event: {0}", rand.nextInt());
+    logger.log(Level.WARNING, "A JUL Warning Event: {0}", rand.nextInt());
+    logger.log(Level.SEVERE, String.format("A JUL Severe Event: %d", rand.nextInt()),
         new RuntimeException("Generic Error"));
   }
 }
diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java
@@ -21,16 +21,16 @@
 import java.util.concurrent.ThreadLocalRandom;
 
 class Log4jExample implements Runnable {
-  private static final Logger LOG = Logger.getLogger(Log4jExample.class);
+  private static final Logger logger = Logger.getLogger(Log4jExample.class);
 
   @Override
   public void run() {
     ThreadLocalRandom rand = ThreadLocalRandom.current();
-    LOG.trace(String.format("A Log4j Trace Event: %d", rand.nextInt()));
-    LOG.debug(String.format("A Log4j Debug Event: %d", rand.nextInt()));
-    LOG.info(String.format("A Log4j Info Event: %d", rand.nextInt()));
-    LOG.warn(String.format("A Log4j Warn Event: %d", rand.nextInt()));
-    LOG.error(String.format("A Log4j Error Event: %d", rand.nextInt()),
+    logger.trace(String.format("A Log4j Trace Event: %d", rand.nextInt()));
+    logger.debug(String.format("A Log4j Debug Event: %d", rand.nextInt()));
+    logger.info(String.format("A Log4j Info Event: %d", rand.nextInt()));
+    logger.warn(String.format("A Log4j Warn Event: %d", rand.nextInt()));
+    logger.error(String.format("A Log4j Error Event: %d", rand.nextInt()),
         new RuntimeException("Generic Error"));
   }
 }
diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java
@@ -22,16 +22,16 @@
 import java.util.concurrent.ThreadLocalRandom;
 
 class Slf4jExample implements Runnable {
-  private static final Logger LOG = LoggerFactory.getLogger(Slf4jExample.class);
+  private static final Logger logger = LoggerFactory.getLogger(Slf4jExample.class);
 
   @Override
   public void run() {
     ThreadLocalRandom rand = ThreadLocalRandom.current();
-    LOG.trace("A Slf4j Trace Event: {}", rand.nextInt());
-    LOG.debug("A Slf4j Debug Event: {}", rand.nextInt());
-    LOG.info("A Slf4j Info Event: {}", rand.nextInt());
-    LOG.warn("A Slf4j Warn Event: {}", rand.nextInt());
-    LOG.error(String.format("A Slf4j Error Event: %d", rand.nextInt()),
+    logger.trace("A Slf4j Trace Event: {}", rand.nextInt());
+    logger.debug("A Slf4j Debug Event: {}", rand.nextInt());
+    logger.info("A Slf4j Info Event: {}", rand.nextInt());
+    logger.warn("A Slf4j Warn Event: {}", rand.nextInt());
+    logger.error(String.format("A Slf4j Error Event: %d", rand.nextInt()),
         new RuntimeException("Generic Error"));
   }
 }
diff --git a/...d-runtime/src/main/resources/META-INF/services/com.google.apphosting.logging.SystemLogger b/...d-runtime/src/main/resources/META-INF/services/com.google.apphosting.logging.SystemLogger
@@ -1 +1 @@
-com.google.apphosting.vmruntime.VmRuntimeFileLogHandler
+com.google.apphosting.logging.CloudLoggingFileHandler
diff --git a/...ompat-base/src/main/java/com/google/apphosting/vmruntime/jetty9/RequestLoggerHandler.java b/...ompat-base/src/main/java/com/google/apphosting/vmruntime/jetty9/RequestLoggerHandler.java
@@ -10,6 +10,12 @@
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 
+/**
+ * The {@link java.util.logging.Handler} responsible for capturing the
+ * access logging events from {@link org.eclipse.jetty.server.handler.RequestLogHandler}
+ * and {@link org.eclipse.jetty.server.Slf4jRequestLog} to a rolling file
+ * on disk.
+ */
 public class RequestLoggerHandler extends FileHandler implements SystemLogger {
   public static class RequestLogFormatter extends Formatter {
     @Override
@@ -19,22 +25,37 @@ public String format(LogRecord record) {
     }
   }
 
-
   public static final String LOG_DIRECTORY_PROPERTY = "com.google.apphosting.logs";
   public static final String LOG_PATTERN_CONFIG_PROPERTY =
       RequestLoggerHandler.class.getName() + ".pattern";
   private static final String DEFAULT_LOG_DIRECTORY = "/var/log/app_engine";
   private static final String DEFAULT_LOG_PATTERN = "access.%g.log";
 
-  private static final int LOG_PER_FILE_SIZE = 10 * 1024 * 1024;
-  private static final int LOG_MAX_FILES = 3;
-  private static final boolean LOG_APPEND = true;
+  private static final int LOG_PER_FILE_SIZE = getConfigInt("limit", 10 * 1024 * 1024);
+  private static final int LOG_MAX_FILES = getConfigInt("count", 3);
+  private static final boolean LOG_APPEND = getConfigBool("append", true);
 
   public RequestLoggerHandler() throws IOException, SecurityException {
     super(getFilePattern(), LOG_PER_FILE_SIZE, LOG_MAX_FILES, LOG_APPEND);
     setFormatter(new RequestLogFormatter());
   }
 
+  private static boolean getConfigBool(String suffix, boolean defValue) {
+    String val = System.getProperty(RequestLoggerHandler.class.getName() + '.' + suffix);
+    if (val == null) {
+      return defValue;
+    }
+    return Boolean.parseBoolean(val);
+  }
+
+  private static int getConfigInt(String suffix, int defValue) {
+    String val = System.getProperty(RequestLoggerHandler.class.getName() + '.' + suffix);
+    if (val == null) {
+      return defValue;
+    }
+    return Integer.parseInt(val);
+  }
+
   private static String getFilePattern() {
     String directory = System.getProperty(LOG_DIRECTORY_PROPERTY, DEFAULT_LOG_DIRECTORY);
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		com.google.apphosting.vmruntime.VmRuntimeFileLogHandler
		com.google.apphosting.logging.CloudLoggingFileHandler