From 29dba1e89f20ab96a0a9dbb0fb7c61e03e273e52 Mon Sep 17 00:00:00 2001
From: zpencer <spencerfang@google.com>
Date: Mon, 4 Jun 2018 14:11:55 -0700
Subject: [PATCH] services: allow binlog to blacklist methods (#4523)

The spec says users can specify a blacklist a method from binlogs by
saying "-package.service/method".
---
 .../main/java/io/grpc/services/BinlogHelper.java | 16 ++++++++++++++++
 .../java/io/grpc/services/BinlogHelperTest.java  | 12 ++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/services/src/main/java/io/grpc/services/BinlogHelper.java b/services/src/main/java/io/grpc/services/BinlogHelper.java
index ec9c36f242..907c488a24 100644
--- a/services/src/main/java/io/grpc/services/BinlogHelper.java
+++ b/services/src/main/java/io/grpc/services/BinlogHelper.java
@@ -59,7 +59,9 @@ import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -364,6 +366,7 @@ final class BinlogHelper {
     private final BinlogHelper globalLog;
     private final Map<String, BinlogHelper> perServiceLogs;
     private final Map<String, BinlogHelper> perMethodLogs;
+    private final Set<String> blacklistedMethods;
 
     /**
      * Accepts a string in the format specified by the binary log spec.
@@ -374,6 +377,7 @@ final class BinlogHelper {
       BinlogHelper globalLog = null;
       Map<String, BinlogHelper> perServiceLogs = new HashMap<String, BinlogHelper>();
       Map<String, BinlogHelper> perMethodLogs = new HashMap<String, BinlogHelper>();
+      Set<String> blacklistedMethods = new HashSet<String>();
       if (configurationString != null && configurationString.length() > 0) {
         for (String configuration : Splitter.on(',').split(configurationString)) {
           Matcher configMatcher = configRe.matcher(configuration);
@@ -404,6 +408,14 @@ final class BinlogHelper {
                 Level.INFO,
                 "Service binlog: service={0} config={1}",
                 new Object[] {service, binlogOptionStr});
+          } else if (methodOrSvc.startsWith("-")) {
+            String blacklistedMethod = methodOrSvc.substring(1);
+            if (blacklistedMethod.length() == 0) {
+              continue;
+            }
+            if (!blacklistedMethods.add(blacklistedMethod)) {
+              logger.log(Level.SEVERE, "Ignoring duplicate entry: {0}", configuration);
+            }
           } else {
             // assume fully qualified method name
             if (perMethodLogs.containsKey(methodOrSvc)) {
@@ -421,6 +433,7 @@ final class BinlogHelper {
       this.globalLog = globalLog;
       this.perServiceLogs = Collections.unmodifiableMap(perServiceLogs);
       this.perMethodLogs = Collections.unmodifiableMap(perMethodLogs);
+      this.blacklistedMethods = Collections.unmodifiableSet(blacklistedMethods);
     }
 
     /**
@@ -428,6 +441,9 @@ final class BinlogHelper {
      */
     @Override
     public BinlogHelper getLog(String fullMethodName) {
+      if (blacklistedMethods.contains(fullMethodName)) {
+        return null;
+      }
       BinlogHelper methodLog = perMethodLogs.get(fullMethodName);
       if (methodLog != null) {
         return methodLog;
diff --git a/services/src/test/java/io/grpc/services/BinlogHelperTest.java b/services/src/test/java/io/grpc/services/BinlogHelperTest.java
index 2b400a2496..9c7417b70c 100644
--- a/services/src/test/java/io/grpc/services/BinlogHelperTest.java
+++ b/services/src/test/java/io/grpc/services/BinlogHelperTest.java
@@ -20,6 +20,7 @@ import static io.grpc.services.BinaryLogProvider.BYTEARRAY_MARSHALLER;
 import static io.grpc.services.BinlogHelper.DUMMY_SOCKET;
 import static io.grpc.services.BinlogHelper.getPeerSocket;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.mockito.Matchers.eq;
@@ -264,6 +265,17 @@ public final class BinlogHelperTest {
     assertNull(makeLog(configStr, "package.service2/absent"));
   }
 
+  @Test
+  public void configBinLog_blacklist() {
+    assertNull(makeLog("*,-p.s/blacklisted", "p.s/blacklisted"));
+    assertNull(makeLog("-p.s/blacklisted,*", "p.s/blacklisted"));
+    assertNotNull(makeLog("-p.s/method,*", "p.s/allowed"));
+
+    assertNull(makeLog("p.s/*,-p.s/blacklisted", "p.s/blacklisted"));
+    assertNull(makeLog("-p.s/blacklisted,p.s/*", "p.s/blacklisted"));
+    assertNotNull(makeLog("-p.s/blacklisted,p.s/*", "p.s/allowed"));
+  }
+
   @Test
   public void configBinLog_ignoreDuplicates_global() throws Exception {
     String configStr = "*{h},p.s/m,*{h:256}";