bboyleonp666 commented on PR #16752:
URL: https://github.com/apache/kafka/pull/16752#issuecomment-2267273503
> Hi @bboyleonp666, I found some packages are still using
`KafkaMetricsGroup#explicitMetricName`. I think we can refactor them and maybe
update the function as private. WDYT? cc @chia7712
>
> * BrokerServerMetrics
> * SocketServer
> * KafkaMetricsGroupTest
>
> ```diff
> diff --git a/core/src/main/scala/kafka/network/SocketServer.scala
b/core/src/main/scala/kafka/network/SocketServer.scala
> index c9d6a86c54..ce117da0e2 100644
> --- a/core/src/main/scala/kafka/network/SocketServer.scala
> +++ b/core/src/main/scala/kafka/network/SocketServer.scala
> @@ -608,11 +608,9 @@ private[kafka] abstract class Acceptor(val
socketServer: SocketServer,
>
> private[network] val processors = new ArrayBuffer[Processor]()
> // Build the metric name explicitly in order to keep the existing name
for compatibility
> - private val blockedPercentMeterMetricName =
KafkaMetricsGroup.explicitMetricName(
> - "kafka.network",
> - "Acceptor",
> - s"${metricPrefix()}AcceptorBlockedPercent",
> - Map(ListenerMetricTag -> endPoint.listenerName.value).asJava)
> + private val backwardCompatibilityMetricGroup = new
KafkaMetricsGroup("kafka.network", "Acceptor")
> + private val blockedPercentMeterMetricName =
backwardCompatibilityMetricGroup.metricName(
> + s"${metricPrefix()}AcceptorBlockedPercent", Map(ListenerMetricTag ->
endPoint.listenerName.value).asJava)
> private val blockedPercentMeter =
metricsGroup.newMeter(blockedPercentMeterMetricName,"blocked time",
TimeUnit.NANOSECONDS)
> private var currentProcessorIndex = 0
> private[network] val throttledSockets = new
mutable.PriorityQueue[DelayedCloseSocket]()
> diff --git
a/core/src/main/scala/kafka/server/metadata/BrokerServerMetrics.scala
b/core/src/main/scala/kafka/server/metadata/BrokerServerMetrics.scala
> index ad8c21f960..33ddcf9c70 100644
> --- a/core/src/main/scala/kafka/server/metadata/BrokerServerMetrics.scala
> +++ b/core/src/main/scala/kafka/server/metadata/BrokerServerMetrics.scala
> @@ -33,8 +33,9 @@ final class BrokerServerMetrics private (
> ) extends AutoCloseable {
> import BrokerServerMetrics._
>
> - private val batchProcessingTimeHistName =
KafkaMetricsGroup.explicitMetricName("kafka.server",
> - "BrokerMetadataListener",
> + private val metricGroup = new KafkaMetricsGroup("kafka.server",
"BrokerMetadataListener")
> +
> + private val batchProcessingTimeHistName = metricGroup.metricName(
> "MetadataBatchProcessingTimeUs",
> Collections.emptyMap())
>
> @@ -44,8 +45,7 @@ final class BrokerServerMetrics private (
> private val batchProcessingTimeHist =
>
KafkaYammerMetrics.defaultRegistry().newHistogram(batchProcessingTimeHistName,
true)
>
> - private val batchSizeHistName =
KafkaMetricsGroup.explicitMetricName("kafka.server",
> - "BrokerMetadataListener",
> + private val batchSizeHistName = metricGroup.metricName(
> "MetadataBatchSizes",
> Collections.emptyMap())
>
> diff --git
a/core/src/test/scala/unit/kafka/metrics/KafkaMetricsGroupTest.scala
b/core/src/test/scala/unit/kafka/metrics/KafkaMetricsGroupTest.scala
> index 2b0c46c5c4..a237b9dd7c 100644
> --- a/core/src/test/scala/unit/kafka/metrics/KafkaMetricsGroupTest.scala
> +++ b/core/src/test/scala/unit/kafka/metrics/KafkaMetricsGroupTest.scala
> @@ -29,9 +29,8 @@ class KafkaMetricsGroupTest {
>
> @Test
> def testUntaggedMetricName(): Unit = {
> - val metricName = KafkaMetricsGroup.explicitMetricName(
> - "kafka.metrics",
> - "TestMetrics",
> + val metricGroup = new KafkaMetricsGroup("kafka.metrics",
"TestMetrics")
> + val metricName = metricGroup.metricName(
> "TaggedMetric",
> Collections.emptyMap()
> )
> @@ -47,9 +46,8 @@ class KafkaMetricsGroupTest {
> @Test
> def testTaggedMetricName(): Unit = {
> val tags = Map("foo" -> "bar", "bar" -> "baz", "baz" ->
"raz.taz").asJava
> - val metricName = KafkaMetricsGroup.explicitMetricName(
> - "kafka.metrics",
> - "TestMetrics",
> + val metricGroup = new KafkaMetricsGroup("kafka.metrics",
"TestMetrics")
> + val metricName = metricGroup.metricName(
> "TaggedMetric",
> tags
> )
> @@ -65,9 +63,8 @@ class KafkaMetricsGroupTest {
> @Test
> def testTaggedMetricNameWithEmptyValue(): Unit = {
> val tags = Map("foo" -> "bar", "bar" -> "", "baz" -> "raz.taz").asJava
> - val metricName = KafkaMetricsGroup.explicitMetricName(
> - "kafka.metrics",
> - "TestMetrics",
> + val metricGroup = new KafkaMetricsGroup("kafka.metrics",
"TestMetrics")
> + val metricName = metricGroup.metricName(
> "TaggedMetric",
> tags
> )
> ```
--
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
