apoorvmittal10 commented on PR #22129: URL: https://github.com/apache/kafka/pull/22129#issuecomment-4561749364
> 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. > TBH I am not sure if really logging the metricValueProvider is anyways helpful to determine the issues you pointed, those issues are much more fundamental. Do you have an example of how these log lines will be helpful? Also as I said earlier the change is very minimal so will merge but I ll still wait to hear how these logs are going to help. -- 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]
