kirktrue commented on a change in pull request #9584:
URL: https://github.com/apache/kafka/pull/9584#discussion_r780603531
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
##########
@@ -858,14 +862,23 @@ public KafkaConsumer(Map<String, Object> configs,
this.kafkaConsumerMetrics = new KafkaConsumerMetrics(metrics,
"consumer");
}
- private static Metrics buildMetrics(ConsumerConfig config, Time time,
String clientId) {
- Map<String, String> metricsTags =
Collections.singletonMap(CLIENT_ID_METRIC_TAG, clientId);
+ private static Metrics buildMetrics(ConsumerConfig config, Time time,
String clientId, String groupId) {
+ Map<String, String> metricsTags = new HashMap<>();
+ metricsTags.put(CLIENT_ID_METRIC_TAG, clientId);
+ if (groupId != null && !groupId.trim().isEmpty()) {
Review comment:
The secondary `!groupId.trim().isEmpty()` clause of the `if` statement
shouldn't ever catch given that that case is checked for in the constructor
before calling `buildMetrics`, right?
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
##########
@@ -858,14 +862,23 @@ public KafkaConsumer(Map<String, Object> configs,
this.kafkaConsumerMetrics = new KafkaConsumerMetrics(metrics,
"consumer");
}
- private static Metrics buildMetrics(ConsumerConfig config, Time time,
String clientId) {
- Map<String, String> metricsTags =
Collections.singletonMap(CLIENT_ID_METRIC_TAG, clientId);
+ private static Metrics buildMetrics(ConsumerConfig config, Time time,
String clientId, String groupId) {
+ Map<String, String> metricsTags = new HashMap<>();
+ metricsTags.put(CLIENT_ID_METRIC_TAG, clientId);
+ if (groupId != null && !groupId.trim().isEmpty()) {
+ metricsTags.put(GROUP_ID_METRIC_TAG, groupId);
+ }
MetricConfig metricConfig = new
MetricConfig().samples(config.getInt(ConsumerConfig.METRICS_NUM_SAMPLES_CONFIG))
.timeWindow(config.getLong(ConsumerConfig.METRICS_SAMPLE_WINDOW_MS_CONFIG),
TimeUnit.MILLISECONDS)
.recordLevel(Sensor.RecordingLevel.forName(config.getString(ConsumerConfig.METRICS_RECORDING_LEVEL_CONFIG)))
.tags(metricsTags);
+ final Map<String, Object> reporterConfigOverrides = new HashMap<>();
+ reporterConfigOverrides.put(ConsumerConfig.CLIENT_ID_CONFIG, clientId);
+ if (groupId != null && !groupId.trim().isEmpty()) {
Review comment:
Same minor nitpick about whether or not we need to check for an empty
group ID.
--
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]
