Report close connection exceptions at INFO#81768
Conversation
In elastic#51612 we promoted certain log messages regarding unexpected network exceptions from `TRACE` to `DEBUG`. In fact it's often useful to see these exceptions by default, so with this commit we show the message (but not the stack trace) at `INFO` level. This commit also adds some commentary about what each of the exceptions means. Closes elastic#66473
|
Pinging @elastic/es-distributed (Team:Distributed) |
| logger.debug(() -> new ParameterizedMessage("send message failed [channel: {}]", channel), e); | ||
| final Level closeConnectionExceptionLevel = NetworkExceptionHelper.getCloseConnectionExceptionLevel(e); | ||
| if (closeConnectionExceptionLevel == Level.OFF) { | ||
| logger.warn(new ParameterizedMessage("send message failed [channel: {}]", channel), e); |
There was a problem hiding this comment.
Should the condition above be closeConnectionExceptionLevel == Level.WARN?
There was a problem hiding this comment.
No, the WARN case is handled in the final else branch. This branch is handling the case that the exception doesn't appear to be network-related, and preserves the earlier behaviour that such exceptions are logged as warnings.
There was a problem hiding this comment.
Oh, I was confused with Level.OFF actually representing a non network related.
I wonder if that could be changed to Level.WARN then, the first branch could be merged with the last one, unless I miss something.
There was a problem hiding this comment.
Kind of, but I think that risks future bugs. The intention is to distinguish "this exception is network-related and deserves a warning" from "this exception is not network-related". An earlier iteration actually returned WARN sometimes but then I moved them all to INFO because I found that all the Linux ones might be benign and I didn't want to emit warnings for benign problems. I didn't investigate the Windows behaviour and we might decide to move them back up to WARN in future.
I could have used an Optional<Level> or a null or similar but Level#OFF seemed like a simpler way to say the same thing.
|
Thanks Ievgen! |
In #51612 we promoted certain log messages regarding unexpected network
exceptions from
TRACEtoDEBUG. In fact it's often useful to seethese exceptions by default, so with this commit we show the message
(but not the stack trace) at
INFOlevel. This commit also adds somecommentary about what each of the exceptions means.
Closes #66473