jeffkbkim commented on code in PR #18717: URL: https://github.com/apache/kafka/pull/18717#discussion_r1935906597
########## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ########## @@ -145,17 +133,14 @@ public void incrementNumOffsets() { } /** - * Increment the number of consumer groups. + * Set the number of consumer groups. The method is the only way to update + * the map and is called by the scheduled task that updates the metrics + * in {@link org.apache.kafka.coordinator.group.GroupCoordinatorShard}. * - * @param state the consumer group state. + * @param consumerGroupGauges The map counting the number of consumer groups in each state. Review Comment: nit: can we update the classicGroupGauges parameter similarly to this? ########## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ########## @@ -145,17 +133,14 @@ public void incrementNumOffsets() { } /** - * Increment the number of consumer groups. + * Set the number of consumer groups. The method is the only way to update + * the map and is called by the scheduled task that updates the metrics + * in {@link org.apache.kafka.coordinator.group.GroupCoordinatorShard}. Review Comment: nit: "This method should be the only way.." and add "..breaking this will result in inconsistent behavior" as well as for `setClassicGroupGauges` -- 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