squah-confluent commented on code in PR #22498:
URL: https://github.com/apache/kafka/pull/22498#discussion_r3383975863


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetrics.java:
##########
@@ -104,6 +142,13 @@ public class GroupCoordinatorMetrics extends 
CoordinatorMetrics implements AutoC
     private final MetricName streamsGroupCountDeadMetricName;
     private final MetricName streamsGroupCountNotReadyMetricName;
 
+    private final MetricName offsetCountMetricName;
+    private final MetricName classicGroupCountPreparingRebalanceMetricName;
+    private final MetricName classicGroupCountCompletingRebalanceMetricName;
+    private final MetricName classicGroupCountStableMetricName;
+    private final MetricName classicGroupCountDeadMetricName;
+    private final MetricName classicGroupCountEmptyMetricName;
+

Review Comment:
   nit: The existing `MetricName`s are organized by group type. To keep the 
organization the change should be
   
   ```diff
   +     private final MetricName offsetCountMetricName;
         private final MetricName classicGroupCountMetricName;
   +     private final MetricName classicGroupCountPreparingRebalanceMetricName;
   +     private final MetricName 
classicGroupCountCompletingRebalanceMetricName;
   +     private final MetricName classicGroupCountStableMetricName;
   +     private final MetricName classicGroupCountDeadMetricName;
   +     private final MetricName classicGroupCountEmptyMetricName;
         private final MetricName consumerGroupCountMetricName;
   ```
   
   and the same for the rest of the changes below in this file.



##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsTest.java:
##########
@@ -150,7 +150,30 @@ public void testMetricNames() {
             metrics.metricName(
                 "streams-group-count",
                 GroupCoordinatorMetrics.METRICS_GROUP,
-                Map.of("state", StreamsGroupState.NOT_READY.toString()))
+                Map.of("state", StreamsGroupState.NOT_READY.toString())),
+            metrics.metricName(
+                GroupCoordinatorMetrics.OFFSET_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP),
+            metrics.metricName(
+                GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP,
+                Map.of(GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_STATE_TAG, 
ClassicGroupState.PREPARING_REBALANCE.toString())),
+            metrics.metricName(
+                GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP,
+                Map.of(GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_STATE_TAG, 
ClassicGroupState.COMPLETING_REBALANCE.toString())),
+            metrics.metricName(
+                GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP,
+                Map.of(GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_STATE_TAG, 
ClassicGroupState.STABLE.toString())),
+            metrics.metricName(
+                GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP,
+                Map.of(GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_STATE_TAG, 
ClassicGroupState.DEAD.toString())),
+            metrics.metricName(
+                GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_METRIC_NAME,
+                GroupCoordinatorMetrics.METRICS_GROUP,
+                Map.of(GroupCoordinatorMetrics.CLASSIC_GROUP_COUNT_STATE_TAG, 
ClassicGroupState.EMPTY.toString()))

Review Comment:
   same comment about the ordering here



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to