Reduce noise from BGPSessionImpl 58/111258/2
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 4 Apr 2024 06:09:07 +0000 (08:09 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 4 Apr 2024 09:27:22 +0000 (11:27 +0200)
We are including Netty thread trace whenever we terminate because of a
BGPDocumentedException. Improve the logging/reporting smarts here to
exclude DecoderExceptions unless they are requested.

Change-Id: If6f036bae67b16d7796d8616e4fbdb2491171bba
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java

index 876265cb2d2dbad91494cd05fc8a14e8b06fa238..08dc78e484ce208878cce5938f307b0e6a28e761 100644 (file)
@@ -19,6 +19,7 @@ import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
+import io.netty.handler.codec.DecoderException;
 import io.netty.util.concurrent.ScheduledFuture;
 import java.io.IOException;
 import java.nio.channels.NonWritableChannelException;
@@ -586,10 +587,33 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification<?>>
     @Holding({"this.listener", "this"})
     @VisibleForTesting
     void handleException(final Throwable cause) {
-        LOG.warn("BGP session encountered error", cause);
-        final Throwable docCause = cause.getCause();
-        terminate(docCause instanceof BGPDocumentedException
-            ? (BGPDocumentedException) docCause : new BGPDocumentedException(BGPError.CEASE));
+        // We have two things to do here:
+        // - log a warning with appropriate context
+        final Throwable toLog;
+        // - terminate the session with the appropriate error
+        final BGPDocumentedException toReport;
+
+        if (cause instanceof BGPDocumentedException bde) {
+            // Easy case
+            toLog = toReport = bde;
+        } else if (cause.getCause() instanceof BGPDocumentedException bde) {
+            // we are going to report the cause, that's for sure
+            toReport = bde;
+
+            // if this is a DecoderException, assume it is coming from ByteToMessageDecoder.callDecode() and the context
+            // is just Netty thread call stack starting at a io.netty.util.internal.ThreadExecutorMap Runnable.
+            // Trim such context unless we have trace enabled.
+            toLog = cause instanceof DecoderException && !LOG.isTraceEnabled() ? bde : cause;
+        } else {
+            // if we can include the causal chain for terminate(), great, but only log cause, i.e. without us in
+            // in the picture.
+            toReport = cause instanceof Exception ex ? new BGPDocumentedException(null, BGPError.CEASE, ex)
+                : new BGPDocumentedException(BGPError.CEASE);
+            toLog = cause;
+        }
+
+        LOG.warn("BGP session encountered error", toLog);
+        terminate(toReport);
     }
 
     @Override