Improve logger error reporting

gregw · gregw · commit b661c6265112 · 2017-02-09T11:56:24.000+11:00
Handler initialization may now call user supplied code in enhancers during
initialization and logging.  This commit catches any exceptions thrown and
uses the existing error reporting mechanism to log the exception the stderr.

The GaeFlexEnhancer now separately logs any exception when obtaining the zone,
but does not throw it as having the zone is only optional and should be non
fatal to the logger.
diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java
@@ -17,6 +17,7 @@
 package com.google.cloud.logging;
 
 import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.HttpURLConnection;
 import java.net.URL;
@@ -91,8 +92,9 @@ public void enhanceMonitoredResource(Builder builder) {
           builder.addLabel("zone", zone);
         }
       }
-    } catch (Exception e) {
-      // unable to fetch zone;
+    } catch (IOException ex) {
+      // report the error, but do not throw as the inclusion of zone is optional
+      ex.printStackTrace();
     }
   }
 
diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
@@ -163,19 +163,25 @@ public LoggingHandler(String log, LoggingOptions options, MonitoredResource moni
    * or {@link LogEntry} instances built by this handler.
    */
   public LoggingHandler(String log, LoggingOptions options, MonitoredResource monitoredResource, List<Enhancer> enhancers) {
-    LogConfigHelper helper = new LogConfigHelper();
-    String className = getClass().getName();
-    this.options = options != null ? options : LoggingOptions.getDefaultInstance();
-    this.flushLevel = helper.getLevelProperty(className + ".flushLevel", LoggingLevel.ERROR);
-    this.flushSize = helper.getLongProperty(className + ".flushSize", 1L);
-    setLevel(helper.getLevelProperty(className + ".level", Level.INFO));
-    setFilter(helper.getFilterProperty(className + ".filter", null));
-    setFormatter(helper.getFormatterProperty(className + ".formatter", new SimpleFormatter()));
-    String logName = firstNonNull(log, helper.getProperty(className + ".log", "java.log"));
-    this.enhancers = enhancers != null ? enhancers : helper.getEnhancerProperty(className + ".enhancers");
-    String resourceType = helper.getProperty(className + ".resourceType", "global");
-    MonitoredResource resource = monitoredResource != null ? monitoredResource : getDefaultResource(resourceType);
-    writeOptions = new WriteOption[]{WriteOption.logName(logName), WriteOption.resource(resource)};
+    try {
+      LogConfigHelper helper = new LogConfigHelper();
+      String className = getClass().getName();
+      this.options = options != null ? options : LoggingOptions.getDefaultInstance();
+      this.flushLevel = helper.getLevelProperty(className + ".flushLevel", LoggingLevel.ERROR);
+      this.flushSize = helper.getLongProperty(className + ".flushSize", 1L);
+      setLevel(helper.getLevelProperty(className + ".level", Level.INFO));
+      setFilter(helper.getFilterProperty(className + ".filter", null));
+      setFormatter(helper.getFormatterProperty(className + ".formatter", new SimpleFormatter()));
+      String logName = firstNonNull(log, helper.getProperty(className + ".log", "java.log"));
+      this.enhancers = enhancers != null ? enhancers : helper.getEnhancerProperty(className + ".enhancers");
+      String resourceType = helper.getProperty(className + ".resourceType", "global");
+      MonitoredResource resource = monitoredResource != null ? monitoredResource : getDefaultResource(resourceType);
+      writeOptions = new WriteOption[]{WriteOption.logName(logName), WriteOption.resource(resource)};
+    }
+    catch (Exception ex) {
+      reportError(null, ex, ErrorManager.OPEN_FAILURE);
+      throw ex;
+    }
   }
 
   private static List<LoggingHandler> getLoggingHandlers(Logger logger) {
@@ -354,23 +360,24 @@ private LogEntry entryFor(LogRecord record) {
     String payload;
     try {
       payload = getFormatter().format(record);
+      Level level = record.getLevel();
+      LogEntry.Builder builder = LogEntry.newBuilder(Payload.StringPayload.of(payload))
+          .addLabel("levelName", level.getName())
+          .addLabel("levelValue", String.valueOf(level.intValue()))
+          .setTimestamp(record.getMillis())
+          .setSeverity(severityFor(level));
+
+      for (Enhancer enhancer : enhancers) {
+        enhancer.enhanceLogEntry(builder, record);
+      }
+      enhanceLogEntry(builder, record);
+      return builder.build();
     } catch (Exception ex) {
-      // Formatting can fail but we should not throw an exception, we report the error instead
+      // Formatting or enhancing can fail but we should not throw an exception, 
+      // we report the error instead
       reportError(null, ex, ErrorManager.FORMAT_FAILURE);
       return null;
     }
-    Level level = record.getLevel();
-    LogEntry.Builder builder = LogEntry.newBuilder(Payload.StringPayload.of(payload))
-        .addLabel("levelName", level.getName())
-        .addLabel("levelValue", String.valueOf(level.intValue()))
-        .setTimestamp(record.getMillis())
-        .setSeverity(severityFor(level));
-    
-    for (Enhancer enhancer : enhancers) {
-      enhancer.enhanceLogEntry(builder, record);
-    }
-    enhanceLogEntry(builder, record);
-    return builder.build();
   }
   
   @Deprecated
@@ -493,7 +500,7 @@ public static void addHandler(Logger logger, LoggingHandler handler) {
    * A Log Enhancer.
    * May be used to enhance the {@link MonitoredResource} and/or the {@link LogEntry}
    */
-  public interface Enhancer {
+  interface Enhancer {
     void enhanceMonitoredResource(MonitoredResource.Builder builder);
     void enhanceLogEntry(LogEntry.Builder builder, LogRecord record);
   }

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@`
`17`	`17`	`package com.google.cloud.logging;`
`18`	`18`
`19`	`19`	`import java.io.BufferedReader;`
	`20`	`+import java.io.IOException;`
`20`	`21`	`import java.io.InputStreamReader;`
`21`	`22`	`import java.net.HttpURLConnection;`
`22`	`23`	`import java.net.URL;`
`@@ -91,8 +92,9 @@ public void enhanceMonitoredResource(Builder builder) {`
`91`	`92`	`builder.addLabel("zone", zone);`
`92`	`93`	`}`
`93`	`94`	`}`
`94`		`- } catch (Exception e) {`
`95`		`- // unable to fetch zone;`
	`95`	`+ } catch (IOException ex) {`
	`96`	`+ // report the error, but do not throw as the inclusion of zone is optional`
	`97`	`+ ex.printStackTrace();`
`96`	`98`	`}`
`97`	`99`	`}`
`98`	`100`