fix: Need a way to disable flushing (#1206)

* fix: Need a way to disable flushing

* fix test name

* add test for flush level warning

* add a comment explaining Severity.UNRECOGNIZED usage for flush level

* fix comment

* Remove redundant definition

* Adjust to latest PR review

* Address PR comments

* Add test to validate that Severity.NONE cannot be used

* add null treatment for flushSeverity

* Remove redundand null check

Author: losalex

google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java

@@ -715,6 +715,9 @@ public String toStructuredJsonString() {
     if (payload.getType() == Payload.Type.PROTO) {
       throw new UnsupportedOperationException("LogEntry with protobuf payload cannot be converted");
     }
+    if (severity == Severity.NONE) {
+      throw new IllegalArgumentException("Severity.NONE cannot be used for LogEntry");
+    }
 
     final StringBuilder builder = new StringBuilder("{");
     final StructuredLogFormatter formatter = new StructuredLogFormatter(builder);

google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java

@@ -303,8 +303,8 @@ public static TailOption project(String project) {
 
   /**
    * Sets flush severity for asynchronous logging writes. It is disabled by default, enabled when
-   * this method is called with not null value. Logs will be immediately written out for entries at
-   * or higher than flush severity.
+   * this method is called with any {@link Severity} value other than {@link Severity.NONE}. Logs
+   * will be immediately written out for entries at or higher than flush severity.
    *
    * <p>Enabling this can cause the leaking and hanging threads, see BUG(2796) BUG(3880). However
    * you can explicitly call {@link #flush}.

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

@@ -498,29 +498,21 @@ private static Severity severityFor(Level level) {
     if (level instanceof LoggingLevel) {
       return ((LoggingLevel) level).getSeverity();
     }
-
-    switch (level.intValue()) {
-        // FINEST
-        // FINER
-        // FINE
-      case 300:
-      case 400:
-      case 500:
-        return Severity.DEBUG;
-        // CONFIG
-        // INFO
-      case 700:
-      case 800:
-        return Severity.INFO;
-        // WARNING
-      case 900:
-        return Severity.WARNING;
-        // SEVERE
-      case 1000:
-        return Severity.ERROR;
-      default:
-        return Severity.DEFAULT;
+    // Choose the severity based on Level range.
+    // The assumption is that Level values below maintain same numeric value
+    int value = level.intValue();
+    if (value <= Level.FINE.intValue()) {
+      return Severity.DEBUG;
+    } else if (value <= Level.INFO.intValue()) {
+      return Severity.INFO;
+    } else if (value <= Level.WARNING.intValue()) {
+      return Severity.WARNING;
+    } else if (value <= Level.SEVERE.intValue()) {
+      return Severity.ERROR;
+    } else if (value == Level.OFF.intValue()) {
+      return Severity.NONE;
     }
+    return Severity.DEFAULT;
   }
 
   private static boolean isTrueOrNull(Boolean b) {

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java

@@ -107,7 +107,7 @@ class LoggingImpl extends BaseService<LoggingOptions> implements Logging {
   private final Map<Object, ApiFuture<Void>> pendingWrites = new ConcurrentHashMap<>();
 
   private volatile Synchronicity writeSynchronicity = Synchronicity.ASYNC;
-  private volatile Severity flushSeverity = null;
+  private volatile Severity flushSeverity = Severity.NONE;
   private boolean closed;
 
   private static Boolean emptyToBooleanFunction(Empty input) {
@@ -138,7 +138,12 @@ public void setWriteSynchronicity(Synchronicity writeSynchronicity) {
 
   @Override
   public void setFlushSeverity(Severity flushSeverity) {
-    this.flushSeverity = flushSeverity;
+    // For backward compatibility we treat null as Severity.NONE
+    if (flushSeverity == null) {
+      this.flushSeverity = Severity.NONE;
+    } else {
+      this.flushSeverity = flushSeverity;
+    }
   }
 
   @Override
@@ -882,7 +887,7 @@ public void write(Iterable<LogEntry> logEntries, WriteOption... options) {
         options = Instrumentation.addPartialSuccessOption(options);
       }
       writeLogEntries(logEntries, options);
-      if (flushSeverity != null) {
+      if (flushSeverity != Severity.NONE) {
         for (LogEntry logEntry : logEntries) {
           // flush pending writes if log severity at or above flush severity
           if (logEntry.getSeverity().compareTo(flushSeverity) >= 0) {

google-cloud-logging/src/main/java/com/google/cloud/logging/Severity.java

@@ -26,6 +26,9 @@
  */
 public enum Severity {
 
+  /** The None severity level. Should not be used with log entries. */
+  NONE(LogSeverity.UNRECOGNIZED),
+
   /** The log entry has no assigned severity level. */
   DEFAULT(LogSeverity.DEFAULT),
 

google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java

@@ -394,4 +394,14 @@ public void testStructureLogPresentations() {
   public void testStructureLogPresentationWithProtobufPayload() {
     assertThrows(UnsupportedOperationException.class, () -> PROTO_ENTRY.toStructuredJsonString());
   }
+
+  @Test
+  public void testStructureLogInvalidSeverity() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> PROTO_ENTRY.toBuilder().setSeverity(Severity.NONE).build().toPb(PROJECT));
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> STRING_ENTRY.toBuilder().setSeverity(Severity.NONE).build().toStructuredJsonString());
+  }
 }

google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingHandlerTest.java

@@ -231,6 +231,12 @@ protected void after() {
     }
   }
 
+  static class CustomLevel extends Level {
+    CustomLevel() {
+      super("CUSTOM", 510);
+    }
+  }
+
   @Rule public final OutputStreamPatcher outputStreamPatcher = new OutputStreamPatcher();
 
   @Before
@@ -241,7 +247,7 @@ public void setUp() {
     expect(options.getProjectId()).andStubReturn(PROJECT);
     expect(options.getService()).andStubReturn(logging);
     expect(options.getAutoPopulateMetadata()).andStubReturn(Boolean.FALSE);
-    logging.setFlushSeverity(EasyMock.anyObject(Severity.class));
+    logging.setFlushSeverity(Severity.ERROR);
     expectLastCall().anyTimes();
     logging.setWriteSynchronicity(EasyMock.anyObject(Synchronicity.class));
     expectLastCall().anyTimes();
@@ -537,6 +543,37 @@ public void testFlushLevel() {
     handler.publish(newLogRecord(Level.WARNING, MESSAGE));
   }
 
+  @Test
+  public void testFlushLevelOff() {
+    logging.setFlushSeverity(Severity.NONE);
+    expectLastCall().once();
+    replay(options, logging);
+    LoggingHandler handler = new LoggingHandler(LOG_NAME, options, DEFAULT_RESOURCE);
+    handler.setFlushLevel(Level.OFF);
+    assertEquals(Level.OFF, handler.getFlushLevel());
+  }
+
+  @Test
+  public void testFlushLevelOn() {
+    logging.setFlushSeverity(Severity.WARNING);
+    expectLastCall().once();
+    replay(options, logging);
+    LoggingHandler handler = new LoggingHandler(LOG_NAME, options, DEFAULT_RESOURCE);
+    handler.setFlushLevel(Level.WARNING);
+    assertEquals(Level.WARNING, handler.getFlushLevel());
+  }
+
+  @Test
+  public void testCustomFlushLevelOn() {
+    CustomLevel level = new CustomLevel();
+    logging.setFlushSeverity(Severity.INFO);
+    expectLastCall().once();
+    replay(options, logging);
+    LoggingHandler handler = new LoggingHandler(LOG_NAME, options, DEFAULT_RESOURCE);
+    handler.setFlushLevel(level);
+    assertEquals(level, handler.getFlushLevel());
+  }
+
   @Test
   public void testSyncWrite() {
     reset(logging);