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
googleapis · Dec 1, 2022 · aa0c176 · aa0c176
1 parent 2d684c2
commit aa0c176
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 28 deletions.
diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java b/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);

diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java b/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}.

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
@@ -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) {

diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java 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) {

diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/Severity.java b/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),
 

diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java b/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());
+ }
 }
diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingHandlerTest.java b/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);