You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Apoorv Mittal <ap...@gmail.com> on 2024/02/14 14:55:32 UTC
[Discuss] KIP-1019: Expose method to determine Metric Measurability
Hi,
I would like to start discussion of a small KIP which fills a gap in
determining Kafka Metric measurability.
KIP-1019: Expose method to determine Metric Measurability
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability>
Regards,
Apoorv Mittal
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by "Matthias J. Sax" <mj...@apache.org>.
Thanks for the KIP. Seems there is not much we need to discuss about it.
Feel free to start a VOTE.
-Matthias
On 2/15/24 7:24 AM, Manikumar wrote:
> LGTM, Thanks for the KIP.
>
> On Thu, Feb 15, 2024 at 8:50 PM Doğuşcan Namal <na...@gmail.com>
> wrote:
>
>> LGTM thanks for the KIP.
>>
>> +1(non-binding)
>>
>> On Wed, 14 Feb 2024 at 15:22, Andrew Schofield <
>> andrew_schofield_jira@outlook.com> wrote:
>>
>>> Hi Apoorv,
>>> Thanks for the KIP. Looks like a useful change to tidy up the metrics
>> code.
>>>
>>> Thanks,
>>> Andrew
>>>
>>>> On 14 Feb 2024, at 14:55, Apoorv Mittal <ap...@gmail.com>
>>> wrote:
>>>>
>>>> Hi,
>>>> I would like to start discussion of a small KIP which fills a gap in
>>>> determining Kafka Metric measurability.
>>>>
>>>> KIP-1019: Expose method to determine Metric Measurability
>>>> <
>>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
>>>>
>>>>
>>>> Regards,
>>>> Apoorv Mittal
>>>
>>>
>>
>
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Manikumar <ma...@gmail.com>.
LGTM, Thanks for the KIP.
On Thu, Feb 15, 2024 at 8:50 PM Doğuşcan Namal <na...@gmail.com>
wrote:
> LGTM thanks for the KIP.
>
> +1(non-binding)
>
> On Wed, 14 Feb 2024 at 15:22, Andrew Schofield <
> andrew_schofield_jira@outlook.com> wrote:
>
> > Hi Apoorv,
> > Thanks for the KIP. Looks like a useful change to tidy up the metrics
> code.
> >
> > Thanks,
> > Andrew
> >
> > > On 14 Feb 2024, at 14:55, Apoorv Mittal <ap...@gmail.com>
> > wrote:
> > >
> > > Hi,
> > > I would like to start discussion of a small KIP which fills a gap in
> > > determining Kafka Metric measurability.
> > >
> > > KIP-1019: Expose method to determine Metric Measurability
> > > <
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> > >
> > >
> > > Regards,
> > > Apoorv Mittal
> >
> >
>
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Doğuşcan Namal <na...@gmail.com>.
LGTM thanks for the KIP.
+1(non-binding)
On Wed, 14 Feb 2024 at 15:22, Andrew Schofield <
andrew_schofield_jira@outlook.com> wrote:
> Hi Apoorv,
> Thanks for the KIP. Looks like a useful change to tidy up the metrics code.
>
> Thanks,
> Andrew
>
> > On 14 Feb 2024, at 14:55, Apoorv Mittal <ap...@gmail.com>
> wrote:
> >
> > Hi,
> > I would like to start discussion of a small KIP which fills a gap in
> > determining Kafka Metric measurability.
> >
> > KIP-1019: Expose method to determine Metric Measurability
> > <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> >
> >
> > Regards,
> > Apoorv Mittal
>
>
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Andrew Schofield <an...@outlook.com>.
Hi Apoorv,
Thanks for the KIP. Looks like a useful change to tidy up the metrics code.
Thanks,
Andrew
> On 14 Feb 2024, at 14:55, Apoorv Mittal <ap...@gmail.com> wrote:
>
> Hi,
> I would like to start discussion of a small KIP which fills a gap in
> determining Kafka Metric measurability.
>
> KIP-1019: Expose method to determine Metric Measurability
> <https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability>
>
> Regards,
> Apoorv Mittal
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Jun Rao <ju...@confluent.io.INVALID>.
Hi, Apoorv,
Thanks for the reply. So it sounds like we could only take further actions
on Measurable. Then the new API makes sense.
Jun
On Wed, Feb 21, 2024 at 4:24 AM Apoorv Mittal <ap...@gmail.com>
wrote:
> Hi Jun,
> Thanks for reviewing.
>
> KafkaMetrics uses 2 types of MetricValueProvider as of now i.e. Measurable
> and Gauge. Gauge metrics are generally implemented as anonymous inner
> classes but Measurable has different implementations. KIP-714 (
> KafkaMetricsCollector
> <
> https://github.com/apache/kafka/blob/5cfcc52fb3fce4a43ca77df311382d7a02a40ed2/clients/src/main/java/org/apache/kafka/common/telemetry/internals/KafkaMetricsCollector.java#L218
> >)
> and other collectors (OpenTelemetry Kafka Client
> <
> https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/7a044f576fc06e041e07b4315a9afb0e1d081d4b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaMetricRegistry.java#L36
> >)
> needs to differentiate on type of Measurable to determine the metric types
> (i.e. Counter, Gauge, etc.) for the OpenTelemetry. Though that
> differentiation can happen with already exposed functionality by
> KafkaMetric but that approach is not straightforward and efficient hence
> this KIP adds another method.
>
> As the differentiation is only required for the Measurable value provider
> hence went ahead to expose the measurability check in KafkaMetric. I do not
> think that we should require isGauge check as there doesn't seem to be any
> need for determining Gauge types for external metrics data models. I did
> check the current implementations for Percentiles and Frequencies which
> also emits metrics over Measurable itself. Though I do understand that enum
> might be helpful to translate what metric type the value provider is but as
> we only have support
> <
> https://github.com/apache/kafka/blob/5cfcc52fb3fce4a43ca77df311382d7a02a40ed2/clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java#L36
> >
> for
> Measurable and Gauge hence went ahead only exposing what is currently
> needed.
>
> Regards,
> Apoorv Mittal
> +44 7721681581
>
>
> On Tue, Feb 20, 2024 at 10:58 PM Jun Rao <ju...@confluent.io.invalid> wrote:
>
> > Hi, Apoorv,
> >
> > Thanks for the KIP.
> >
> > Could we document how we plan to use the new isMeasurable() method? For
> > example, gauge is another type of metric. Do we plan to add an isGauge()
> > method too? If so, is it better to add a separate method for each metric
> > type or is it better to have a single method like metricType() that
> returns
> > an enum?
> >
> > Jun
> >
> >
> > On Wed, Feb 14, 2024 at 6:56 AM Apoorv Mittal <ap...@gmail.com>
> > wrote:
> >
> > > Hi,
> > > I would like to start discussion of a small KIP which fills a gap in
> > > determining Kafka Metric measurability.
> > >
> > > KIP-1019: Expose method to determine Metric Measurability
> > > <
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> > > >
> > >
> > > Regards,
> > > Apoorv Mittal
> > >
> >
>
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Apoorv Mittal <ap...@gmail.com>.
Hi Jun,
Thanks for reviewing.
KafkaMetrics uses 2 types of MetricValueProvider as of now i.e. Measurable
and Gauge. Gauge metrics are generally implemented as anonymous inner
classes but Measurable has different implementations. KIP-714 (
KafkaMetricsCollector
<https://github.com/apache/kafka/blob/5cfcc52fb3fce4a43ca77df311382d7a02a40ed2/clients/src/main/java/org/apache/kafka/common/telemetry/internals/KafkaMetricsCollector.java#L218>)
and other collectors (OpenTelemetry Kafka Client
<https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/7a044f576fc06e041e07b4315a9afb0e1d081d4b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaMetricRegistry.java#L36>)
needs to differentiate on type of Measurable to determine the metric types
(i.e. Counter, Gauge, etc.) for the OpenTelemetry. Though that
differentiation can happen with already exposed functionality by
KafkaMetric but that approach is not straightforward and efficient hence
this KIP adds another method.
As the differentiation is only required for the Measurable value provider
hence went ahead to expose the measurability check in KafkaMetric. I do not
think that we should require isGauge check as there doesn't seem to be any
need for determining Gauge types for external metrics data models. I did
check the current implementations for Percentiles and Frequencies which
also emits metrics over Measurable itself. Though I do understand that enum
might be helpful to translate what metric type the value provider is but as
we only have support
<https://github.com/apache/kafka/blob/5cfcc52fb3fce4a43ca77df311382d7a02a40ed2/clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java#L36>
for
Measurable and Gauge hence went ahead only exposing what is currently
needed.
Regards,
Apoorv Mittal
+44 7721681581
On Tue, Feb 20, 2024 at 10:58 PM Jun Rao <ju...@confluent.io.invalid> wrote:
> Hi, Apoorv,
>
> Thanks for the KIP.
>
> Could we document how we plan to use the new isMeasurable() method? For
> example, gauge is another type of metric. Do we plan to add an isGauge()
> method too? If so, is it better to add a separate method for each metric
> type or is it better to have a single method like metricType() that returns
> an enum?
>
> Jun
>
>
> On Wed, Feb 14, 2024 at 6:56 AM Apoorv Mittal <ap...@gmail.com>
> wrote:
>
> > Hi,
> > I would like to start discussion of a small KIP which fills a gap in
> > determining Kafka Metric measurability.
> >
> > KIP-1019: Expose method to determine Metric Measurability
> > <
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> > >
> >
> > Regards,
> > Apoorv Mittal
> >
>
Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability
Posted by Jun Rao <ju...@confluent.io.INVALID>.
Hi, Apoorv,
Thanks for the KIP.
Could we document how we plan to use the new isMeasurable() method? For
example, gauge is another type of metric. Do we plan to add an isGauge()
method too? If so, is it better to add a separate method for each metric
type or is it better to have a single method like metricType() that returns
an enum?
Jun
On Wed, Feb 14, 2024 at 6:56 AM Apoorv Mittal <ap...@gmail.com>
wrote:
> Hi,
> I would like to start discussion of a small KIP which fills a gap in
> determining Kafka Metric measurability.
>
> KIP-1019: Expose method to determine Metric Measurability
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> >
>
> Regards,
> Apoorv Mittal
>