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
>

Reply via email to