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