Skip to content

Commit

Permalink
netty: use FINE log for pure IOExceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
creamsoup committed Nov 5, 2019
1 parent 4dde588 commit 89eef59
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 24 deletions.
14 changes: 2 additions & 12 deletions netty/src/main/java/io/grpc/netty/NettyServerTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import io.grpc.InternalChannelz.SocketStats;
Expand Down Expand Up @@ -50,11 +49,6 @@ class NettyServerTransport implements ServerTransport {
// connectionLog is for connection related messages only
private static final Logger connectionLog = Logger.getLogger(
String.format("%s.connections", 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",
"An established connection was aborted by the software in your host machine");

private final InternalLogId logId;
private final Channel channel;
Expand Down Expand Up @@ -184,12 +178,8 @@ Channel channel() {
*/
@VisibleForTesting
static Level getLogLevel(Throwable t) {
if (t instanceof IOException && t.getMessage() != null) {
for (String msg : QUIET_ERRORS) {
if (t.getMessage().contains(msg)) {
return Level.FINE;
}
}
if (t.getClass().equals(IOException.class)) {
return Level.FINE;
}
return Level.INFO;
}
Expand Down
23 changes: 11 additions & 12 deletions netty/src/test/java/io/grpc/netty/NettyServerTransportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.grpc.netty;

import static com.google.common.truth.Truth.assertThat;
import static io.grpc.netty.NettyServerTransport.getLogLevel;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
Expand All @@ -34,27 +35,25 @@ public void unknownException() {
}

@Test
public void quiet() {
public void ioException() {
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 quiet_prefixed() {
assertEquals(Level.FINE, getLogLevel(new IOException(
"syscall:read(..) failed: Connection reset by peer")));
public void ioException_nullMessage() {
IOException e = new IOException();
assertNull(e.getMessage());
assertEquals(Level.FINE, getLogLevel(e));
}

@Test
public void nonquiet() {
assertEquals(Level.INFO, getLogLevel(new IOException("foo")));
}
public void extendedIoException() {
class ExtendedIoException extends IOException {}

@Test
public void nullMessage() {
IOException e = new IOException();
assertNull(e.getMessage());
assertEquals(Level.INFO, getLogLevel(e));
ExtendedIoException e = new ExtendedIoException();
assertThat(e.getMessage()).isNull();
assertThat(getLogLevel(e)).isEqualTo(Level.INFO);
}
}

0 comments on commit 89eef59

Please sign in to comment.