Hi Bill,
Thanks for the KIP!
BC1
I find the name registerAdditionalMetrics() not specific enough. It is
not clear that the passed-in metrics are made available for
subscription. What about registerMetricsForSubscription()?
The Additional part is confusing in my opinion: additional to what?
BC2
Does the KIP need a transformation rule from MetricName to
OpenTelemetry-compatible names that are exposed at the broker for
subscription and retrieval?
Best,
Bruno
On 8/20/24 12:43 AM, Bill Bejeck wrote:
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
