m1a2st commented on code in PR #19527:
URL: https://github.com/apache/kafka/pull/19527#discussion_r2376035589
##########
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 meant using that for the new "component" tag.
Yes, I think it's a good idea.
> For KafkaPrincipalBuilder, I don't think these tags are enough, see
https://github.com/apache/kafka/pull/18935#issuecomment-2776351491. We should
check if this is the case for the other plugins too.
I’ve looked more closely at the plugins: `Login`, `SslEngineFactory`,
`AuthenticateCallbackHandler`, and `KafkaPrincipalBuilder`. I think
`KafkaPrincipalBuilder` is different from the others.
The other plugins are bound to the selector, where each selector maps to
exactly one instance of `Login`, `SslEngineFactory`, or
`AuthenticateCallbackHandler`. In contrast, a single selector can map to
multiple channels, and `KafkaPrincipalBuilder` is tied to the channel
lifecycle, which is shorter than that of the selector.
Therefore, I agree that adding metrics for `KafkaPrincipalBuilder` is
meaningless.
--
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]
