[
https://issues.apache.org/jira/browse/KAFKA-1481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14191115#comment-14191115
]
Jun Rao commented on KAFKA-1481:
--------------------------------
Thanks for the patch. Looks good overall. Some minor comments below.
40. KafkaMetricsGroup:
40.1 Since we already match the metric name directly, shouldn't the following
pattern
val pattern = (".*" + metric.getName + ".*" + clientId + ".*").r
be
val pattern = (".*clientId=" + clientId + ".*").r
40.2 Can we make toMBeanName() private?
40.3 According to
http://docs.oracle.com/javase/7/docs/api/javax/management/ObjectName.html, the
key properties in an Mbean objectname are a set of unordered keys. So, we don't
need the toMap() method. The caller can just create the Map using
Map("tag1"->"val1", "tag1"->va2").
41. FetchRequestAndResponseMetrics: The following two lines should be in the
same line.
val tags = metricId
match {
42. Log, Partition,AbstractFetcherThread,ProducerRequestPurgatory:
"partitionId" => "partition"
43. TopicPartitionRequestKey: We don't need keyLabel() any more. Instead, we
can override the toString() method to get the string in the same way as that in
TopicAndPartition.toString().
44. RequestChannel: Lower case "processor" in the following.
toMap("Processor" -> i.toString)
45. SocketServer: "NetworkProcessor" => "networkProcessor"
46. ZookeeperConsumerConnector: Could we put the new Gauge and the toMap() into
a separate line?
newGauge("OwnedPartitionsCount", new Gauge[Int] {
def value() = allTopicsOwnedPartitionsCount
}, toMap("clientId" -> config.clientId, "groupId" -> config.groupId,
"allTopics" -> "true"))
newGauge("OwnedPartitionsCount", new Gauge[Int] {
def value() = partitionThreadPairs.size
}, ownedPartitionsCountMetricName(topic))
> Stop using dashes AND underscores as separators in MBean names
> --------------------------------------------------------------
>
> Key: KAFKA-1481
> URL: https://issues.apache.org/jira/browse/KAFKA-1481
> Project: Kafka
> Issue Type: Bug
> Components: core
> Affects Versions: 0.8.1.1
> Reporter: Otis Gospodnetic
> Priority: Critical
> Labels: patch
> Fix For: 0.8.3
>
> Attachments: KAFKA-1481_2014-06-06_13-06-35.patch,
> KAFKA-1481_2014-10-13_18-23-35.patch, KAFKA-1481_2014-10-14_21-53-35.patch,
> KAFKA-1481_2014-10-15_10-23-35.patch, KAFKA-1481_2014-10-20_23-14-35.patch,
> KAFKA-1481_2014-10-21_09-14-35.patch, KAFKA-1481_2014-10-30_21-35-43.patch,
> KAFKA-1481_IDEA_IDE_2014-10-14_21-53-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-15_10-23-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-20_20-14-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-20_23-14-35.patch
>
>
> MBeans should not use dashes or underscores as separators because these
> characters are allowed in hostnames, topics, group and consumer IDs, etc.,
> and these are embedded in MBeans names making it impossible to parse out
> individual bits from MBeans.
> Perhaps a pipe character should be used to avoid the conflict.
> This looks like a major blocker because it means nobody can write Kafka 0.8.x
> monitoring tools unless they are doing it for themselves AND do not use
> dashes AND do not use underscores.
> See: http://search-hadoop.com/m/4TaT4lonIW
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
