Hi Lucas, thanks for the comments. > LB1 Yes, I'll update the KIP signature to `? extends Metric`
>LB2 If `enable.metrics.push` is disabled, the metrics passed in are ignored. Thinking about this now, I'm considering updating the return type of `registerAdditionalMetrics` to a `boolean` from `void`, true if the metrics have been successfully applied and false otherwise. WDYT? Cheers, Bill On Mon, Aug 12, 2024 at 11:33 AM Lucas Brutschy <lbruts...@confluent.io.invalid> wrote: > Hi Bill! > > Thanks for the KIP! I like this solution, it's general enough for many > kinds of application. > > LB1: Could we accept an instance of the Metric interface instead of > KafkaMetric? No strong reason to do it, it would just make the > interface slightly more generic. > > LB2: What is the behavior is `enable.metrics.push` is disabled? > > Cheers, > Lucas > > On Fri, Aug 2, 2024 at 9:30 PM Bill Bejeck <bbej...@apache.org> wrote: > > > > Hi all, > > > > I would like to start a discussion thread on KIP-1076 to enable metrics > > collection for applications that embed a Kafka client. > > > > The KIP can be found here: > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1076%3A++Metrics+for+client+applications+KIP-714+extension > > > > I look forward to the discussion and your feedback. > > > > Thanks, > > Bill >