kevin-wu24 commented on code in PR #18304:
URL: https://github.com/apache/kafka/pull/18304#discussion_r1935864197


##########
raft/src/main/java/org/apache/kafka/raft/internals/KafkaRaftMetrics.java:
##########
@@ -137,6 +99,14 @@ public KafkaRaftMetrics(Metrics metrics, String 
metricGrpPrefix, QuorumState sta
                 "Number of unknown voters whose connection information is not 
cached; would never be larger than quorum-size.");
         metrics.addMetric(this.numUnknownVoterConnectionsMetricName, (mConfig, 
currentTimeMs) -> numUnknownVoterConnections);
 
+        this.numVotersMetricName = metrics.metricName("number-of-voters", 
metricGroupName, "Number of voters for a KRaft topic partition.");
+        metrics.addMetric(this.numVotersMetricName, (Gauge<Integer>) (mConfig, 
currentTimestamp) -> numVoters);

Review Comment:
   I believe we are exposing doubles unintentionally. When we take the metric 
and return the value in `KafkaMetric.metricValue()`:
   ```
   @Override
       public Object metricValue() {
           long now = time.milliseconds();
           synchronized (this.lock) {
               if (isMeasurable())
                   return ((Measurable) metricValueProvider).measure(config, 
now);
               else if (this.metricValueProvider instanceof Gauge)
                   return ((Gauge<?>) metricValueProvider).value(config, now);
               else
                   throw new IllegalStateException("Not a valid metric: " + 
this.metricValueProvider.getClass());
           }
       }
   ```
   We would be calling `Measurable.measure()` and returning a double.
   
   I don't think I've ever seen this kind of case before where we overload a 
method signature in this fashion (the first signature takes in `Measurable`, an 
interface that extends MetricValueProvider<Double>, while the second signature 
takes a generic of the `MetricValueProvider<T>` parent interface). 
   
   Unless I perform an explicit cast/declaration of a 
Gauge<Integer/Boolean/etc.> object to pass in as the second parameter of 
`addMetric`, the compiler always assigns the method to call as 
`addMetric(MetricName, Measurable)`. I think this is because the second method 
signature is taking in the super-interface `MetricValueProvider` as a 
parameter, which is not a functional interface and cannot be implemented with a 
lambda like `Measurable`, rather than its child `Gauge<T>`.



-- 
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