Hi Mickael, On Mon, May 4, 2026, at 12:10 PM, Mickael Maison wrote: > Hi, > > Thanks for the KIP, I have a few questions: > > MM1: Can we have the definition for the new Exception classes? I'm > particularly interested in the exception for INVALID_CONFIG, how are > the failures returned to the client? From PushConfigResponse it seems > it's just a string? Should it be nullable collection with a specific > message for each issue?
I've added the definition for the ConfigTooLargeException which is now the only exception added. > MM2: The new broker class is presented as a policy but if I understand > correctly in case the process method throws and PushConfigResponse has > a non-NONE error code, the client still continues. This is different > from the other policy classes AlterConfigPolicy, CreateTopicPolicy > which prevent the action in case of a violation. Are we aiming for > validation, observability or both? We are focused on observability in this KIP, but as you point out, the policy moniker doesn't make sense in this context. > MM3: It seems you're treating Streams as a separate client instead of > an aggregation of Producers and Consumers (I see StreamsConfig in > Configuration Payload Size Enforcement). What about Connect? Clients such as Kafka Streams, Connect, etc. are tricky because it's important to provide context to the embedded client. For example, a consumer used in a Connect sink would benefit from having that context so that downstream filtering, aggregation, etc. can take that into account. LMK if that answers your question or not. > MM4: Regarding sensitive configurations, the current criteria would > treat custom configurations as non sensitive. For example if my > producer has a custom serializer it may have its own custom > configurations too. Is that behavior what you wanted? If so let's me > it clear. By precaution, I'd lean towards considering custom configs > as sensitive as the client has no idea what it is. The intention of the KIP is for any custom configuration (custom meaning not known by the client implementation) to be omitted by default. If the user overrides config.push.allowed.keys with a custom configuration, however, the KIP would allow that to be included in the configuration that is pushed to the broker. Does that seem sensible, or no? Thanks for the feedback! Kirk > > Thanks, > Mickael > > On Sun, May 3, 2026 at 5:24 PM Muralidhar Basani via dev > <[email protected]> wrote: > > > > Hi Kirk, > > > > Thanks for this thoughtful kip. > > > > I have a few additional points. > > > > mb-1 : Would there be any recommendation for reference implementation to > > store the client configs ? may be in a compacted topic etc. Otherwise, > > everyone will come up with their impl, and could be re-invented again and > > again? > > > > mb-2 : When a client reconnects, there might be duplicates of clientconfigs > > pushed. Should this be handled by plugin may be based on uuid/clientid? (I > > am not sure, if client restarts and there is a new uuid, would there be > > stale entries indefinitely) > > > > mb-3 : In your reply to Hector, you noted "it's difficult to ensure that > > client is marked as 'in violation' across all brokers in a cluster." Does > > the same challenge apply to the observability goal? If the PushConfig lands > > on a single randomly chosen broker, only that broker's ClientConfigPolicy > > sees it, so the plugin effectively must fan out to shared storage for the > > feature to be useful cluster-wide ? > > > > mb-4 : If AdminClient configs are not handled, how about any dynamic config > > changes like AdminClient.alterClientConfigs would also work like hot reload > > ? Or any explicit re-push mechanism be possible? > > > > mb-5 : Regarding configs.push.allowed.keys, would it help if there is a > > deny list which can override allow list? or to avoid any sensitive keys > > just in case. > > > > mb-6 : In general, if the rpc fails for one random node, will it retry to a > > different node? > > > > > > Thanks, > > Murali > > > > On Sun, May 3, 2026 at 1:37 PM Kirk True <[email protected]> wrote: > > > > > Hi Andrew, > > > > > > On Tue, Apr 28, 2026, at 6:28 PM, Andrew Schofield wrote: > > > > Hi Kirk, > > > > Thanks for the KIP. Inevitably, I have some comments. > > > > > > > > AS1: I see a few mysterious mentions of "profile". I suppose that these > > > are evidence of a concept which did not eventually see the light of day in > > > the KIP. > > > > > > That's a vestige from a previous design. I will remove them ASAP. > > > > > > > AS2: It seems that the default configurations pushed for a share > > > consumer are incorrect (because some of them are group configs not client > > > configs). I suggest: > > > > > > > > * client.id > > > > * group.id > > > > * share.acknowledgement.mode > > > > * share.acquire.mode > > > > * max.poll.records > > > > * max.poll.interval.ms > > > > * fetch.min.bytes > > > > * fetch.max.wait.ms > > > > > > Thanks for the feedback! > > > > > > > AS3: Is there any reason why you need to send the configuration type > > > information to the broker? I wonder if it would be simpler just to leave > > > everything as strings. I see that you've got a enum for ClientConfigType > > > which is awfully similar to Config.Type, and you're going to end up > > > mapping > > > between these enums. > > > > > > Most of the configuration we're collecting by default end up being > > > numeric, so defaulting to strings is less efficient. Also, the thought > > > process was that implementors of the ClientConfigPolicy plugin may benefit > > > from/need the type information to achieve their goal. > > > > > > Agreed that the mapping between ClientConfigType and Config.Type seems > > > unnecessary. ClientConfigType is a strict subset of Config.Type, > > > specifically no PASSWORD type, so I wanted to provide some compile time > > > support to prevent their usage. Maybe I was being too zealous in this > > > regard? > > > > > > > AS4: ClientConfigUnknownProfileException has a couple of problems. We > > > tend to use Unknown at the start of the error codes and exception class > > > names, not the middle. Profile is not a thing. I suggest it should be > > > UnknownClientConfigSomethingException, but I am not qualified to say what > > > the Something is. > > > > > > I will propose a change in the next revision of the KIP. Thanks for > > > catching that. > > > > > > > AS5: You should include a list of the exceptions and error codes you are > > > introducing to the protocol. I've seen a few new exceptions and they > > > generally have error codes which correspond 1:1. > > > > > > > > * ClientConfigUnknownProfileException - Should this just be > > > InvalidConfigurationException (exists) which maps to INVALID_CONFIG error > > > code? > > > > * ClientConfigTooLargeException - I would expect this to map to the > > > CLIENT_CONFIG_TOO_LARGE error code (new) > > > > * ClientConfigPolicyException - See AS4. Policy exceptions are usually > > > PolicyViolationException, but this policy doesn't validate, it just > > > processes. > > > > > > I wasn't certain where on the specificity vs. generality spectrum to land > > > on creating or reusing existing errors. In my next KIP revision I'll look > > > to reuse existing error codes where possible. > > > > > > > AS6: You've tended to use Config (singular) not Configs (plural). > > > However, in the configurations for the configurations, you've used plural, > > > such as client.configs.policy.class.name. I would err on the side of > > > consistency. > > > > > > Agreed. I'll make this more consistent. > > > > > > > AS7: We are going to have to resolve the relationship between this KIP > > > and KIP-1313. The latter introduces client instance ID on all RPCs, and > > > will simplify your KIP if it is accepted first. > > > > > > Yes, KIP-1313 are racing in this regard. I see KIP-1313 has entered the > > > voting phase, so I will likely end up removing most of the client ID > > > instance references from my KIP, leaving on those that are more pertinent > > > to the KIP's specific needs. > > > > > > > AS8: Please confirm whether there are any timing considerations between > > > PushConfigs and GetTelemetrySubscriptions/PushTelemetry RPCs. I suppose > > > that a client could overlap these RPCs, or even send them to different > > > randomly selected brokers, as part of its initial connection setup. > > > > > > The intention was for the PushConfig RPC to execute before the telemetry > > > RPCs. However, this will likely change based on other feedback, moving > > > toward more of an "overlap" sequencing. From the perspective of this KIP, > > > it shouldn't matter if they're sent to different brokers. I wonder if the > > > ClientConfigPolicy plugin implementors would see that differently? > > > > > > > AS9: Please add some broker metrics for this new feature. I suggest > > > looking at KIP-714 for inspiration. > > > > > > Will do. I intentionally omitted metrics on the broker, leaving them for > > > the specific implementations. But I will look at the KIP-714 metrics, as > > > suggested. > > > > > > > AS10: Why does the client block while the config handshake is being > > > performed? The handshake is not validating the configurations and the > > > client doesn't throw an exception even if the PushConfigs response > > > contains > > > an error code. Doesn't this unnecessarily slow down the initial > > > connection, > > > and that's arguably too long with Kafka clients already. > > > > > > The approach will likely move away from a blocking approach to a > > > background, best effort approach. I will update the KIP to reflect this. > > > > > > > AS11: Which are the default configurations sent for the Apache Kafka > > > Java admin client? > > > > > > In initial discussions, the admin client was deemed as too uninteresting > > > to warrant sending configuration since it doesn't effect correctness or > > > performance of the critical produce/consume path. Do you disagree? > > > > > > Thanks for the thorough feedback! > > > > > > Kirk > > > > > > > Thanks, > > > > Andrew > > > > > > > > On 2026/04/23 17:59:11 Kirk True wrote: > > > > > Hi all, > > > > > > > > > > I would like to start a discussion on KIP-1324: Support client > > > configuration observability: > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1324%3A+Support+client+configuration+observability > > > > > > > > > > Thanks, > > > > > Kirk > > > > > > > > > > > > >
