dajac commented on code in PR #11998:
URL: https://github.com/apache/kafka/pull/11998#discussion_r844038546
##########
core/src/main/scala/kafka/server/DynamicBrokerConfig.scala:
##########
@@ -800,12 +800,15 @@ class DynamicMetricsReporters(brokerId: Int, server:
KafkaBroker) extends Reconf
updatedConfigs.forEach { (k, v) => props.put(k, v.asInstanceOf[AnyRef]) }
propsOverride.forKeyValue { (k, v) => props.put(k, v) }
val reporters =
dynamicConfig.currentKafkaConfig.getConfiguredInstances(reporterClasses,
classOf[MetricsReporter], props)
+ // call notifyMetricsReporters first to satisfy the contract for
MetricsReporter.contextChange,
+ // which provides that MetricsReporter.contextChange must be called before
the first call to MetricsReporter.init().
+ // The first call to init() is done when we call metrics.addReporter below.
Review Comment:
nit: `call` -> `Call`. Why did you put `()` for `MetricsReporter.init()` but
not for `notifyMetricsReporters`, `MetricsReporter.contextChange` and
`metrics.addReporter`? We could perhaps remove the `()` to stay consistent.
--
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
