chibenwa commented on code in PR #2386:
URL: https://github.com/apache/james-project/pull/2386#discussion_r1725661398
##########
protocols/imap/src/main/java/org/apache/james/imap/decode/main/DefaultImapDecoder.java:
##########
@@ -121,6 +119,7 @@ private ImapMessage
decodeCommandNamed(ImapRequestLineReader request, Tag tag, S
if (count == null || (int) count > 0) {
session.setAttribute(INVALID_COMMAND_COUNT, 0);
}
+ LOGGER.trace("Processing {} {} {}", tag.asString(), commandName,
message.toString());
Review Comment:
> .trace() do the check itself - should we check if the logger is enabled
still?
Without explicit calls to 'toString' of course we can aggressively call
`.trace()`
> I wonder - shouldn't this be handled by MDC (and would be more general)?
As explained on the mailing list, MDC needs to be explicitly carried other
accross threds in reactor wich could be expensive.
In 'classic' log statements not integrated with reactor syntax you can
assume the log would not be here.
One other alternative could be to rely explicitly on MDCStructuredLogger:
```
// Avoid cost of initializing MDC locally
if (LOGGER.isTraceEnabled()) {
new MDCStructuredLogger(LOGGER)
.field("username", username)
...
.log(logger -> logger.trace("My trace"));
}
```
A bit boiler plate but this may be better than seriously messing up reactor
syntax to cary other the MDC in this case, and cleaner than just adding the
username in the message......
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
