KAFKA-7509: Avoid passing most non-applicable properties to producer, consumer, and admin client

[rhauch]

The producer, consumer, and admin client log properties that are supplied but unused by the producer. Previously, Connect would pass many of its worker configuration properties into the producer, consumer, and admin client used for internal topics, resulting in lots of log warnings about unused config properties.

With this change, Connect attempts to filter out the worker’s configuration properties that are not applicable to the producer, consumer, or admin client used for internal topics. (Connect is already including only producer and consumer properties when creating those clients for connectors, since those properties are prefixed in the worker config.)

For the most part, this is relatively straightforward, since there are some top-level worker-specific properties that can be removed, and most extension-specific properties have Connect-specific properties. Unfortunately, the REST extension is the only type of connect extension that uses unprefixed properties from the worker config, so any it is not possible to remove those from the properties passed to the producer, consumer, and admin clients. Hopefully, REST extensions are prevalant yet, and this will mean most users may not see any warnings about unused properties in the producer, consumer, and admin client.

Removing the Connect worker configs is one step. The other is to remove the any properties for the producer that are specific to the consumer and admin client. Likewise, for the consumer we have to remove any properties that are specific to the producer and admin client, and for the admin client remove any properties that are specific to the producer and consumer. Note that any property that is unknown (e.g., properties for REST extension, interceptors, metric reporters, serdes, partitioners, etc.) must be passed to the producer, consumer, and admin client. All of these — except for the REST extension properties — should be used by both the producer and consumer. But, since the admin client only supports metric reporters, any properties for interceptors, serdes, partitioners and REST extension will also be logged as unused. Connect configures the serdes for the producers, so we're left with any custom properties for interceptors and partitioners still getting passed to the AdminClient, but this is about the best we can do at this point.

All of this filtering logic was added to the ConnectUtils class, allowing the logic to be easily unit tested and minimize changes to other existing code. All changes are limited to Kafka Connect, and will work with all client and Connect extensions (passing them to the clients if they are unknown). Also, note that when configuring the producers and consumers for connectors, Connect only uses the properties that begin with the producer. and consumer. prefixes, respectively.

This supersedes #5867 and #5802.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[rhauch]

retest this please

[dennis-reiter]

Has this fix been abandoned?

[rhauch]

KAFKA-6793 and #8043 appears to be a super simple fix that will eliminate these warnings by making them debug statements. Since that change obviates the need for this far more complex change, I'm closing this PR without merging and marking KAFKA-7509 as WONTFIX.

[szalapski]

I'd like to ask for this to be reopened and addressed, since it seems KAFKA-6793 and #8043 won't happen.

[rhauch]

I'll reopen, and I hope to update this soon to remove the merge conflicts.

[szalapski]

I still hope this can be fixed soon.

[ronatartifact]

As a new user, I am a bit concerned about warnings that appear in logs.
Warnings usually indicate that the software found something questionable but not fatal.
In most applications, these are things that the user/system admin should investigate to be sure that the issue flagged is not a "real" problem or is not going to cause unexpected behavior.
Spitting out a whole lot of messages as WARN that have no value and are expected normal behavior is not correct.
When things are not working, they are a definite PITA as one tries to find out what the real problem may be.

[tahseen447]

just checking in, has this been resolved?

[EduardoFLima]

Should KAFKA-10090 also fix this ?

[vashu1]

Setting those configuration options to null solves the problem for me, works with SSL:

kafkaEndpointBuilder.kerberosRenewWindowFactor((String) null); kafkaEndpointBuilder.kerberosRenewJitter((String) null); kafkaEndpointBuilder.kerberosInitCmd(null); kafkaEndpointBuilder.kerberosBeforeReloginMinTime((String) null); kafkaEndpointBuilder.sslTruststoreType(null); kafkaEndpointBuilder.sslTrustmanagerAlgorithm(null); kafkaEndpointBuilder.sslKeystoreType(null); kafkaEndpointBuilder.sslEndpointAlgorithm(null); kafkaEndpointBuilder.sslProtocol(null); kafkaEndpointBuilder.sslEnabledProtocols(null); kafkaEndpointBuilder.sslKeymanagerAlgorithm(null);

[C0urante]

I've taken an alternative approach in #11986. If anyone wants to take a look and either review the changes or take them for a spin locally, I'd appreciate any feedback. Thanks!

[szalapski]

Would love to get this fixed. How can I help?

[C0urante]

@szalapski Would you be willing to do a round of review on #11986? It's grown out of date and I have to do a rebase on it, but the high-level idea should remain the same. IMO the biggest hurdle to getting it merged is deciding whether or not the changes proposed there require a KIP; would be good to get thoughts on that in addition to the standard code review.

[szalapski]

The changes seem good to me, however I am not familiar enough with this repo's code to approve it. Maybe @vashu1 or @rhauch can help or suggest who can?

[szalapski]

Sad that this didn't get in.

[szalapski]

@rhauch Any idea how to move this along?

[szalapski]

@rhauch Can we either close or merge this?

@C0urante, any suggestions on moving this along?

[C0urante]

@szalapski I think we've deprioritized work on this front since #13225 appears to be a reasonable compromise. We still emit messages about unused properties, which of course isn't great, but since they now take place at the INFO level instead of WARN, the overall issue is addressed: users no longer have to worry about the WARN level being polluted with unactionable and unavoidable warnings.

If you've upgraded to a version of Kafka that contains the changes from #13225 and aren't satisfied, can you help us understand (either here or with a comment on KAFKA-7509) how this is still impacting you?

[szalapski]

That sounds great. I'll try to upgrade and look into it soon. Feel free to close.

[szalapski]

@rhauch or @C0urante please close this as I suspect you'd like to.

[rhauch]

Yeah, I'm going to close this. As @C0urante mentioned above:

We still emit messages about unused properties, which of course isn't great, but since they now take place at the INFO level instead of WARN, the overall issue is addressed: users no longer have to worry about the WARN level being polluted with unactionable and unavoidable warnings.