gemmellr commented on code in PR #4237:
URL: https://github.com/apache/activemq-artemis/pull/4237#discussion_r983732721
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMServerSASLFactory.java:
##########
@@ -44,14 +44,16 @@
import org.apache.activemq.artemis.spi.core.security.scram.UserData;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.lang.invoke.MethodHandles;
/**
* abstract class that implements the SASL-SCRAM authentication scheme,
concrete implementations
* must supply the {@link SCRAM} type to use and be register via SPI
*/
public abstract class SCRAMServerSASLFactory implements ServerSASLFactory {
- private final Logger logger = LoggerFactory.getLogger(getClass());
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
Review Comment:
Its also a mistake to change it in this way.
Not all logging is debug level, the one here is a warning during exceptional
failure, and if it ever occurs it wont be possible to distinguish what
mechanism it was operating for when it failed after this change, as it will
always log for the abstract parent. This is effectively breaking the
original/useful aimed use of the logging. Either the logging statement also
needs changed to indicate what its operating for, or it should pass in the
mechanism-related logger to give the same effect it was doing originally.
--
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]
