rondagostino commented on a change in pull request #11131:
URL: https://github.com/apache/kafka/pull/11131#discussion_r676985024



##########
File path: 
metadata/src/main/java/org/apache/kafka/controller/QuorumControllerMetrics.java
##########
@@ -22,22 +22,21 @@
 import com.yammer.metrics.core.MetricName;
 import com.yammer.metrics.core.MetricsRegistry;
 
-
 public final class QuorumControllerMetrics implements ControllerMetrics {
-    private final static MetricName ACTIVE_CONTROLLER_COUNT = new MetricName(
-        "kafka.controller", "KafkaController", "ActiveControllerCount", null);
-    private final static MetricName EVENT_QUEUE_TIME_MS = new MetricName(
-        "kafka.controller", "ControllerEventManager", "EventQueueTimeMs", 
null);
-    private final static MetricName EVENT_QUEUE_PROCESSING_TIME_MS = new 
MetricName(
-        "kafka.controller", "ControllerEventManager", 
"EventQueueProcessingTimeMs", null);
-    private final static MetricName GLOBAL_TOPIC_COUNT = new MetricName(
-        "kafka.controller", "KafkaController", "GlobalTopicCount", null);
-    private final static MetricName GLOBAL_PARTITION_COUNT = new MetricName(
-        "kafka.controller", "KafkaController", "GlobalPartitionCount", null);
-    private final static MetricName OFFLINE_PARTITION_COUNT = new MetricName(
-        "kafka.controller", "KafkaController", "OfflinePartitionCount", null);
-    private final static MetricName PREFERRED_REPLICA_IMBALANCE_COUNT = new 
MetricName(
-        "kafka.controller", "KafkaController", 
"PreferredReplicaImbalanceCount", null);
+    private final static MetricName ACTIVE_CONTROLLER_COUNT = getMetricName(
+        "kafka.controller", "KafkaController", "ActiveControllerCount");

Review comment:
       I'm wondering if using constants in the test case would be 
counter-productive -- the test would still succeed if the constant were to 
change.  It feels to me that hard-coding the expected string inside the test 
case as is done now is the safest thing.  Having a constant for de-registration 
makes sense, but maybe we should leave the rafactor for later if we aren't 
going to use the constant in multiple places right now.




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


Reply via email to