adikou commented on PR #22129: URL: https://github.com/apache/kafka/pull/22129#issuecomment-4550001336
> Thanks for the PR @adikou. Have you considered using metric.metricName() for logging, I can see the usage of same in Kafka code base for logging? I also do not currently see where we log KafkaMetric hence don't see the need of this change. However the change is minimal, I am happy to review further if you put this in a PR where you actually have a valid usage of same or can find incorrect logs printed through Kafka currently. Please let me know your thoughts. Hi @apoorvmittal10 thanks for the feedback. The main use case I'm looking at as a user is the `MetricsReporter#metricChange(KafkaMetric)` method. Other than update an internal data structure sometimes logging is common practice to trace steps after the fact. I do see that in several places in the codebase, with a `KafkaMetric` object, we log the metricName(). In this case, I see an issue with naming and conventions that logging just the metricName() might be insufficient. 1. For instance, in producer-topic-metrics `compression-rate` is _an average of compression ratio_ : it is neither a rate, nor is it the avg according to the description. It has a right counterpart `compression-rate-avg` in producer-metrics. I'm planning on filing a follow-up KIP that fixes these two. 2. I see another mismatch in producer, and streams: `byte-total` is a total (knowing CumulativeSum or WindowedSum here would be useful to know) and `cache-size-bytes-total` is a `Value`, so not a total. I think relying on convention and metric naming alone does not give a full picture. So, the underlying stat when available might be useful in logs. The description gets logged, but tbh I feel the MetricValueProvider classname is simpler than going through the description string. One thing to note, I can probably skip lambdas in the calculation because they might look awkward in the logs and are not really meaningful. LMK what you think. Best, Aditya -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
