From 3dfca397e3e4f397675f57714eef9cb19da761c2 Mon Sep 17 00:00:00 2001 From: gfuller1 Date: Thu, 1 Oct 2020 17:48:26 -0700 Subject: [PATCH 1/2] add check for timeout exception --- .../java/com/newrelic/ResponseObserver.java | 13 ++++++++++++- .../com/newrelic/ResponseObserverTest.java | 18 +++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java b/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java index a475773de8..fc61624de0 100644 --- a/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java +++ b/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java @@ -43,7 +43,10 @@ public void onError(Throwable t) { return; } - logger.log(Level.WARNING, t, "Encountered gRPC exception"); + if (!isConnectionTimeoutException(t)) { + logger.log(Level.WARNING, t, "Encountered gRPC exception"); + } + metricAggregator.incrementCounter("Supportability/InfiniteTracing/Response/Error"); Status status = null; @@ -70,6 +73,14 @@ private boolean isChannelClosing(Throwable t) { return t instanceof StatusRuntimeException && t.getCause() instanceof ChannelClosingException; } + /** + * Detects if the error received was a connection timeout exception. This can happen if the agent hasn't sent any spans for more than 15 seconds. + */ + protected boolean isConnectionTimeoutException(Throwable t) { + return t instanceof StatusRuntimeException + && t.getMessage().startsWith("INTERNAL: No error: A GRPC status of OK should have been sent"); + } + /** * Attempts to detect if the error received indicates that ALPN support is not provided by this JVM. */ diff --git a/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java b/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java index c71cc24536..96f5afe4fb 100644 --- a/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java +++ b/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java @@ -15,6 +15,9 @@ import static org.mockito.Mockito.verifyNoInteractions; class ResponseObserverTest { + + AtomicBoolean shouldRecreateCall = new AtomicBoolean(); + @Test public void shouldIncrementCounterOnNext() { MetricAggregator metricAggregator = mock(MetricAggregator.class); @@ -115,5 +118,18 @@ public void shouldTerminateOnALPNError() { verify(disconnectionHandler).terminate(); } - AtomicBoolean shouldRecreateCall = new AtomicBoolean(); + @Test + public void testIsConnectionTimeoutException() { + DisconnectionHandler disconnectionHandler = mock(DisconnectionHandler.class); + MetricAggregator metricAggregator = mock(MetricAggregator.class); + + ResponseObserver target = new ResponseObserver( + metricAggregator, + mock(Logger.class), + disconnectionHandler, shouldRecreateCall); + + Throwable exception = new StatusRuntimeException(Status.fromCode(Status.Code.INTERNAL).withDescription("No error: A GRPC status of OK should have been sent\nRst Stream")); + assertTrue(target.isConnectionTimeoutException(exception)); + } + } \ No newline at end of file From c53a9aca232a27be40434be1785aba6f6f9cb06f Mon Sep 17 00:00:00 2001 From: gfuller1 Date: Fri, 2 Oct 2020 10:16:20 -0700 Subject: [PATCH 2/2] add more tests and use mockito never() --- .../java/com/newrelic/ResponseObserver.java | 2 +- .../com/newrelic/ResponseObserverTest.java | 46 +++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java b/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java index fc61624de0..ca72f74971 100644 --- a/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java +++ b/infinite-tracing/src/main/java/com/newrelic/ResponseObserver.java @@ -76,7 +76,7 @@ private boolean isChannelClosing(Throwable t) { /** * Detects if the error received was a connection timeout exception. This can happen if the agent hasn't sent any spans for more than 15 seconds. */ - protected boolean isConnectionTimeoutException(Throwable t) { + private boolean isConnectionTimeoutException(Throwable t) { return t instanceof StatusRuntimeException && t.getMessage().startsWith("INTERNAL: No error: A GRPC status of OK should have been sent"); } diff --git a/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java b/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java index 96f5afe4fb..a107c2a9b8 100644 --- a/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java +++ b/infinite-tracing/src/test/java/com/newrelic/ResponseObserverTest.java @@ -8,9 +8,11 @@ import org.junit.jupiter.api.Test; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -122,14 +124,52 @@ public void shouldTerminateOnALPNError() { public void testIsConnectionTimeoutException() { DisconnectionHandler disconnectionHandler = mock(DisconnectionHandler.class); MetricAggregator metricAggregator = mock(MetricAggregator.class); + Logger logger = mock(Logger.class); ResponseObserver target = new ResponseObserver( metricAggregator, - mock(Logger.class), + logger, + disconnectionHandler, shouldRecreateCall); + + Throwable exception = new StatusRuntimeException( + Status.fromCode(Status.Code.INTERNAL).withDescription("No error: A GRPC status of OK should have been sent\nRst Stream")); + target.onError(exception); + + verify(logger, never()).log(Level.WARNING, exception, "Encountered gRPC exception"); + } + + @Test + public void testConnectionTimeoutExceptionWrongType() { + DisconnectionHandler disconnectionHandler = mock(DisconnectionHandler.class); + MetricAggregator metricAggregator = mock(MetricAggregator.class); + Logger logger = mock(Logger.class); + + ResponseObserver target = new ResponseObserver( + metricAggregator, + logger, disconnectionHandler, shouldRecreateCall); - Throwable exception = new StatusRuntimeException(Status.fromCode(Status.Code.INTERNAL).withDescription("No error: A GRPC status of OK should have been sent\nRst Stream")); - assertTrue(target.isConnectionTimeoutException(exception)); + Throwable exception = new RuntimeException("No error: A GRPC status of OK should have been sent\nRst Stream"); + target.onError(exception); + + verify(logger).log(Level.WARNING, exception, "Encountered gRPC exception"); + } + + @Test + public void testConnectionTimeoutExceptionWrongMessage() { + DisconnectionHandler disconnectionHandler = mock(DisconnectionHandler.class); + MetricAggregator metricAggregator = mock(MetricAggregator.class); + Logger logger = mock(Logger.class); + + ResponseObserver target = new ResponseObserver( + metricAggregator, + logger, + disconnectionHandler, shouldRecreateCall); + + Throwable exception = new StatusRuntimeException(Status.fromCode(Status.Code.INTERNAL).withDescription("A REALLY BAD ERROR: PRINT ME")); + target.onError(exception); + + verify(logger).log(Level.WARNING, exception, "Encountered gRPC exception"); } } \ No newline at end of file