mimaison commented on code in PR #12045:
URL: https://github.com/apache/kafka/pull/12045#discussion_r862834421


##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java:
##########
@@ -110,25 +127,40 @@ public String toString() {
     protected void purgeObsoleteSamples(MetricConfig config, long now) {
         long expireAge = config.samples() * config.timeWindowMs();
         for (Sample sample : samples) {
-            if (now - sample.lastWindowMs >= expireAge)
+            if (now - sample.getLastWindowMs() >= expireAge)
                 sample.reset(now);
         }
     }
 
     protected static class Sample {
-        public double initialValue;
-        public long eventCount;
-        public long lastWindowMs;
-        public double value;
+        private double initialValue;

Review Comment:
   This is also part of the public API, so we shouldn't be changing these fields



##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java:
##########
@@ -138,6 +170,46 @@ public boolean isComplete(long timeMs, MetricConfig 
config) {
             return timeMs - lastWindowMs >= config.timeWindowMs() || 
eventCount >= config.eventWindow();
         }
 
+        public boolean isActive() {

Review Comment:
   Again because it's public API we can't add new public methods without having 
a KIP



##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java:
##########
@@ -52,10 +51,6 @@ public Rate(TimeUnit unit, SampledStat stat) {
         this.unit = unit;
     }
 
-    public String unitName() {

Review Comment:
   `Rate` is part of the public API 
(https://kafka.apache.org/31/javadoc/org/apache/kafka/common/metrics/stats/Rate.html)
 so we don't want to remove this method.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to