mimaison commented on code in PR #19527: URL: https://github.com/apache/kafka/pull/19527#discussion_r2060545988
########## clients/src/main/java/org/apache/kafka/common/security/authenticator/LoginManager.java: ########## @@ -53,20 +56,30 @@ public class LoginManager { // dynamic configs (broker or client) private static final Map<LoginMetadata<Password>, LoginManager> DYNAMIC_INSTANCES = new HashMap<>(); - private final Login login; + private final Plugin<Login> loginPlugin; private final LoginMetadata<?> loginMetadata; private final AuthenticateCallbackHandler loginCallbackHandler; private int refCount; - private LoginManager(JaasContext jaasContext, String saslMechanism, Map<String, ?> configs, - LoginMetadata<?> loginMetadata) throws LoginException { + private LoginManager( + JaasContext jaasContext, + String saslMechanism, + Map<String, ?> configs, + LoginMetadata<?> loginMetadata, + ConnectionMode connectionMode, + Metrics metrics + ) throws LoginException { this.loginMetadata = loginMetadata; - this.login = Utils.newInstance(loginMetadata.loginClass); + Login login = Utils.newInstance(loginMetadata.loginClass); + if (connectionMode == ConnectionMode.SERVER) + this.loginPlugin = Plugin.wrapInstance(login, metrics, SaslConfigs.SASL_LOGIN_CLASS, "mechanism", saslMechanism); Review Comment: I just had a very quick look and I'm not sure this is going to work. Each `SaslChannelBuilder` instance creates a `Login` instance. We have one `SaslChannelBuilder` instance per `Processor` and we have multiple processors so metrics from multiple `Login` plugins can collide I think. Even with a single `Processor` if you have multiple listeners using the same SASL mechanism they'll collide too. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org