chia7712 commented on code in PR #15863:
URL: https://github.com/apache/kafka/pull/15863#discussion_r1592508554
##########
core/src/main/scala/kafka/log/LogCleaner.scala:
##########
@@ -101,6 +101,7 @@ class LogCleaner(initialConfig: CleanerConfig,
time: Time = Time.SYSTEM) extends Logging with
BrokerReconfigurable {
// Visible for test.
private[log] val metricsGroup = new KafkaMetricsGroup(this.getClass)
+ activateMetrics()
Review Comment:
As it gets called in `startup`, do we need to call it in construction?
##########
core/src/test/scala/unit/kafka/log/LogCleanerTest.scala:
##########
@@ -118,6 +118,23 @@ class LogCleanerTest extends Logging {
}
}
+ @Test
+ def testMetricsActiveAfterReconfiguration(): Unit = {
+ val logCleaner = new LogCleaner(new CleanerConfig(true),
+ logDirs = Array(TestUtils.tempDir()),
+ logs = new Pool[TopicPartition, UnifiedLog](),
+ logDirFailureChannel = new LogDirFailureChannel(1),
+ time = time)
+
+ try {
+ logCleaner.reconfigure(new KafkaConfig(TestUtils.createBrokerConfig(1,
"localhost:2181")),
+ new KafkaConfig(TestUtils.createBrokerConfig(1, "localhost:2181")))
+
+ LogCleaner.MetricNames.foreach(name =>
assertNotNull(KafkaYammerMetrics.defaultRegistry.allMetrics().get(logCleaner.metricsGroup
Review Comment:
We can use `MetricName#getName` to simplify the code. For example:
```scala
val nonexistent =
LogCleaner.MetricNames.diff(KafkaYammerMetrics.defaultRegistry.allMetrics().keySet().asScala.map(_.getName))
assertEquals(0, nonexistent.size, s"$nonexistent should be existent")
```
##########
core/src/test/scala/unit/kafka/log/LogCleanerTest.scala:
##########
@@ -118,6 +118,23 @@ class LogCleanerTest extends Logging {
}
}
+ @Test
+ def testMetricsActiveAfterReconfiguration(): Unit = {
+ val logCleaner = new LogCleaner(new CleanerConfig(true),
+ logDirs = Array(TestUtils.tempDir()),
+ logs = new Pool[TopicPartition, UnifiedLog](),
+ logDirFailureChannel = new LogDirFailureChannel(1),
+ time = time)
+
Review Comment:
Could you please add `startup` and then check the metrics get created?
--
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
