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 <j...@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 <apoorvmitta...@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 > > >
