netty: quieter errors in NettyServerTransport (#3234)

Some errors such as connection resets should be logged at FINE, the
rest should be INFO rather than SEVERE.

fixes #1768

2 files changed, 78 insertions, 1 deletions

diff --git a/netty/src/main/java/io/grpc/netty/NettyServerTransport.java b/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
index e8046b041..5df54c71d 100644
--- a/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
+++ b/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
@@ -16,7 +16,9 @@
 
 package io.grpc.netty;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import io.grpc.ServerStreamTracer;
 import io.grpc.Status;
 import io.grpc.internal.LogId;
@@ -26,6 +28,7 @@ import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandler;
+import java.io.IOException;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -35,6 +38,10 @@ import java.util.logging.Logger;
  */
 class NettyServerTransport implements ServerTransport {
   private static final Logger log = Logger.getLogger(NettyServerTransport.class.getName());
+  // Some exceptions are not very useful and add too much noise to the log
+  private static final ImmutableList<String> QUIET_ERRORS = ImmutableList.of(
+      "Connection reset by peer",
+      "An existing connection was forcibly closed by the remote host");
 
   private final LogId logId = LogId.allocate(getClass().getName());
   private final Channel channel;
@@ -125,9 +132,25 @@ class NettyServerTransport implements ServerTransport {
     return channel;
   }
 
+  /**
+   * Accepts a throwable and returns the appropriate logging level. Uninteresting exceptions
+   * should not clutter the log.
+   */
+  @VisibleForTesting
+  static Level getLogLevel(Throwable t) {
+    if (t instanceof IOException && t.getMessage() != null) {
+      for (String msg : QUIET_ERRORS) {
+        if (t.getMessage().equals(msg)) {
+          return Level.FINE;
+        }
+      }
+    }
+    return Level.INFO;
+  }
+
   private void notifyTerminated(Throwable t) {
     if (t != null) {
-      log.log(Level.SEVERE, "Transport failed", t);
+      log.log(getLogLevel(t), "Transport failed", t);
     }
     if (!terminated) {
       terminated = true;
diff --git a/netty/src/test/java/io/grpc/netty/NettyServerTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyServerTransportTest.java
new file mode 100644
index 000000000..a3d9b43f2
--- /dev/null
+++ b/netty/src/test/java/io/grpc/netty/NettyServerTransportTest.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2017, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.netty;
+
+import static io.grpc.netty.NettyServerTransport.getLogLevel;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.io.IOException;
+import java.util.logging.Level;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class NettyServerTransportTest {
+  @Test
+  public void unknownException() {
+    assertEquals(Level.INFO, getLogLevel(new Exception()));
+  }
+
+  @Test
+  public void quiet() {
+    assertEquals(Level.FINE, getLogLevel(new IOException("Connection reset by peer")));
+    assertEquals(Level.FINE, getLogLevel(new IOException(
+        "An existing connection was forcibly closed by the remote host")));
+  }
+
+  @Test
+  public void nonquiet() {
+    assertEquals(Level.INFO, getLogLevel(new IOException("foo")));
+  }
+
+  @Test
+  public void nullMessage() {
+    IOException e = new IOException();
+    assertNull(e.getMessage());
+    assertEquals(Level.INFO, getLogLevel(e));
+  }
+}