----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review92078 -----------------------------------------------------------
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 56) <https://reviews.apache.org/r/35867/#comment146010> "given `prefix` string and further qualifies the associated AppInfo mbean with the attribute `id`." Actually, sorry to bring this up again (we discussed it briefly in the first diff), but I think it would be better to register the app info explicitly in the consumer/producer/broker and remove this argument. It is convenient to have it instantiated as part of JmxReporter, but it is weird to have to pass in an `id` that will be used only for the app-info mbean. clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 148) <https://reviews.apache.org/r/35867/#comment146037> For consistency with our regular metrics registered via KafkaMetrics maybe we should make this `app-info` clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 247) <https://reviews.apache.org/r/35867/#comment146041> (for consistency) `getCommitId` Also, `getCommitFingerprint` (fp is one word) Also, how about these as alternates: `getCommit` and `getCommitHash` clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 254) <https://reviews.apache.org/r/35867/#comment146038> Can we also log the commit id and fingerprint? - Joel Koshy On July 14, 2015, 12:32 p.m., Manikumar Reddy O wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35867/ > ----------------------------------------------------------- > > (Updated July 14, 2015, 12:32 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1901 > https://issues.apache.org/jira/browse/KAFKA-1901 > > > Repository: kafka > > > Description > ------- > > Addresing Joel's comments > > > Diffs > ----- > > build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java > b4e8f7f0dceefaf94a3495f39f5783cce5ceb25f > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 > clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java > 6b9590c418aedd2727544c5dd23c017b4b72467a > clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java > 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b > clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java > 544e120594de78c43581a980b1e4087b4fb98ccb > core/src/main/scala/kafka/common/AppInfo.scala > d642ca555f83c41451d4fcaa5c01a1f86eff0a1c > core/src/main/scala/kafka/server/KafkaServer.scala > 18917bc4464b9403b16d85d20c3fd4c24893d1d3 > core/src/main/scala/kafka/server/KafkaServerStartable.scala > 1c1b75b4137a8b233b61739018e9cebcc3a34343 > > Diff: https://reviews.apache.org/r/35867/diff/ > > > Testing > ------- > > > Thanks, > > Manikumar Reddy O > >
