Hi Chia-Ping, chia_00: While it initially looks like a simpler option, it turns out the exposed API isn't nice. This was my original approach, hence why Andrew mentions an earlier version, but after implementing it I really disliked the user experience. With this alternative if you want to implement the new method you would still need to implement the older method as it does not have a default in the ClientTelemetryReceiver interface. Not only this is counter intuitive but it also means that both older and newer implementations would need to change if we ever remove the older method. With the proposed API, new implementations can directly implement the new interface and won't need changing in the future. Obviously existing implementations wanting to migrate will require code changes and thus recompiling.
Thanks, Mickael On Mon, Sep 29, 2025 at 6:13 PM Chia-Ping Tsai <[email protected]> wrote: > > hi Mickael > > Apologies for the delayed review. I have a quick question. > > chia_00: The rejected alternative is a simpler approach, and the need for > recompilation is an acceptable drawback. The method is slated for removal in > a major release, which requires recompilation anyway I think. > > Best, > Chia-Ping > > On 2025/09/24 15:56:03 Mickael Maison wrote: > > Hi, > > > > Thanks for the feedback. > > Since it's a relatively small KIP, if I don't see any more feedback in > > the next few days, I'll start a vote. > > > > Thanks, > > Mickael > > > > On Wed, Sep 17, 2025 at 5:44 AM Luke Chen <[email protected]> wrote: > > > > > > Hi Mickael, > > > > > > From the KIP-714, we annotated the ClientTelemetry and > > > ClientTelemetryReceiver > > > as `@InterfaceStability.Evolving`. > > > I was thinking that we are safe to break the compatibility in the next > > > minor release, but it is removed earlier in this PR: > > > https://github.com/apache/kafka/pull/19036... > > > > > > So, I'm +1 on this. > > > > > > Thanks. > > > Luke > > > > > > On Wed, Sep 10, 2025 at 11:44 PM Mickael Maison <[email protected]> > > > wrote: > > > > > > > Hi Andrew, > > > > > > > > Thanks for taking a look! > > > > Yeah after experimenting with the approach now listed in the rejected > > > > alternatives, I thought it was better to just bring whole new > > > > interfaces. > > > > > > > > AS1: I was following the naming from ClientTelemetry, but I agree it's > > > > probably simpler to name that method clientTelemetryExporter(). > > > > I've updated the KIP. > > > > > > > > Thanks, > > > > Mickael > > > > > > > > On Wed, Sep 10, 2025 at 4:26 PM Andrew Schofield > > > > <[email protected]> wrote: > > > > > > > > > > Hi Mickael, > > > > > Thanks for the KIP. It looks like a nice improvement. > > > > > > > > > > I previously read an earlier version and had some comments, but > > > > > you've already addressed them and added to the rejected alternatives > > > > > :) > > > > > Just one additional comment. > > > > > > > > > > AS1: I suggest the method in the ClientTelemetryExporterProvider > > > > > interface would be better named clientTelemetryExporter. > > > > > > > > > > Thanks, > > > > > Andrew > > > > > > > > > > ________________________________________ > > > > > From: Mickael Maison <[email protected]> > > > > > Sent: 09 September 2025 16:56 > > > > > To: dev <[email protected]> > > > > > Subject: [DISCUSS] KIP-1217: Include push interval in > > > > ClientTelemetryReceiver context > > > > > > > > > > Apologies, the KIP number is 1217: > > > > > KIP-1217: Include push interval in ClientTelemetryReceiver context: > > > > > https://cwiki.apache.org/confluence/x/6QnxFg > > > > > > > > > > On Tue, Sep 9, 2025 at 5:52 PM Mickael Maison > > > > > <[email protected]> > > > > wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > I'd like to start a discussion on KIP-1127: > > > > > > https://cwiki.apache.org/confluence/x/6QnxFg > > > > > > > > > > > > This small KIP proposes providing the push interval to > > > > > > ClientTelemetryReceiver implementations to improve the management of > > > > > > client telemetry metrics. > > > > > > > > > > > > Thanks, > > > > > > Mickael > > > > > >
