Hi Mickael, Thanks for the KIP.
IIUC, the pushIntervalMs value passed via the context to exportMetrics() is the _current_ push interval, right? OTOH, the payload parameter represents the data collected in the _previous_ push interval, right? I'm assuming this won't matter in practice, but I wanted to highlight it. Thanks, Kirk On Mon, Sep 29, 2025, at 10:14 AM, Mickael Maison wrote: > 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 > > > > > > > > >
