services: let BinaryLog factory return null (grpc#3868)

Author: zpencer

services/src/main/java/io/grpc/services/BinaryLog.java

@@ -34,8 +34,6 @@
  */
 final class BinaryLog {
   private static final Logger logger = Logger.getLogger(BinaryLog.class.getName());
-  private static final BinaryLog NOOP_LOG =
-      new BinaryLog(/*maxHeaderBytes=*/ 0, /*maxMessageBytes=*/ 0);
   private final int maxHeaderBytes;
   private final int maxMessageBytes;
 
@@ -68,23 +66,18 @@ public String toString() {
   }
 
   private static final Factory DEFAULT_FACTORY;
-  private static final Factory NOOP_FACTORY = new Factory() {
-    @Override
-    public BinaryLog getLog(String fullMethodName) {
-      return NOOP_LOG;
-    }
-  };
+  private static final Factory NULL_FACTORY = new NullFactory();
 
   static {
-    Factory defaultFactory = NOOP_FACTORY;
+    Factory defaultFactory = NULL_FACTORY;
     try {
       String configStr = System.getenv("GRPC_BINARY_LOG_CONFIG");
       if (configStr != null && configStr.length() > 0) {
         defaultFactory = new FactoryImpl(configStr);
       }
     } catch (Throwable t) {
       logger.log(Level.SEVERE, "Failed to initialize binary log. Disabling binary log.", t);
-      defaultFactory = NOOP_FACTORY;
+      defaultFactory = NULL_FACTORY;
     }
     DEFAULT_FACTORY = defaultFactory;
   }
@@ -98,6 +91,7 @@ static BinaryLog getLog(String fullMethodName) {
   }
 
   interface Factory {
+    @Nullable
     BinaryLog getLog(String fullMethodName);
   }
 
@@ -167,7 +161,7 @@ static final class FactoryImpl implements Factory {
           logger.info(String.format("Method binlog: method=%s log=%s", methodOrSvc, binLog));
         }
       }
-      this.globalLog = globalLog == null ? NOOP_LOG : globalLog;
+      this.globalLog = globalLog;
       this.perServiceLogs = Collections.unmodifiableMap(perServiceLogs);
       this.perMethodLogs = Collections.unmodifiableMap(perMethodLogs);
     }
@@ -243,4 +237,11 @@ static boolean isServiceGlob(String input) {
       return input.endsWith("/*");
     }
   }
+
+  private static final class NullFactory implements Factory {
+    @Override
+    public BinaryLog getLog(String fullMethodName) {
+      return null;
+    }
+  }
 }

services/src/test/java/io/grpc/services/BinaryLogTest.java

@@ -27,7 +27,6 @@
 /** Tests for {@link BinaryLog}. */
 @RunWith(JUnit4.class)
 public final class BinaryLogTest {
-  private static final BinaryLog NONE = new Builder().build();
   private static final BinaryLog HEADER_FULL = new Builder().header(Integer.MAX_VALUE).build();
   private static final BinaryLog HEADER_256 = new Builder().header(256).build();
   private static final BinaryLog MSG_FULL = new Builder().msg(Integer.MAX_VALUE).build();
@@ -73,7 +72,7 @@ public void configBinLog_method() throws Exception {
 
   @Test
   public void configBinLog_method_absent() throws Exception {
-    assertEquals(NONE, new FactoryImpl("p.s/m").getLog("p.s/absent"));
+    assertNull(new FactoryImpl("p.s/m").getLog("p.s/absent"));
   }
 
   @Test
@@ -95,7 +94,7 @@ public void configBinLog_service() throws Exception {
 
   @Test
   public void configBinLog_service_absent() throws Exception {
-    assertEquals(NONE, new FactoryImpl("p.s/*").getLog("p.other/m"));
+    assertNull(new FactoryImpl("p.s/*").getLog("p.other/m"));
   }
 
   @Test
@@ -157,7 +156,7 @@ public void configBinLog_multiConfig_noGlobal() throws Exception {
         "package.both256/*{h:256;m:256},"
         + "package.service1/both128{h:128;m:128},"
         + "package.service2/method_messageOnly{m}");
-    assertEquals(NONE, factory.getLog("otherpackage.service/method"));
+    assertNull(factory.getLog("otherpackage.service/method"));
 
     assertEquals(BOTH_256, factory.getLog("package.both256/method1"));
     assertEquals(BOTH_256, factory.getLog("package.both256/method2"));
@@ -166,11 +165,11 @@ public void configBinLog_multiConfig_noGlobal() throws Exception {
     assertEquals(
         new Builder().header(128).msg(128).build(), factory.getLog("package.service1/both128"));
     // no global config in effect
-    assertEquals(NONE, factory.getLog("package.service1/absent"));
+    assertNull(factory.getLog("package.service1/absent"));
 
     assertEquals(MSG_FULL, factory.getLog("package.service2/method_messageOnly"));
     // no global config in effect
-    assertEquals(NONE, factory.getLog("package.service2/absent"));
+    assertNull(factory.getLog("package.service2/absent"));
   }
 
   @Test