[jira] [Commented] (KAFKA-6123) Give client MetricsReporter auto-generated client.id
[ https://issues.apache.org/jira/browse/KAFKA-6123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16476361#comment-16476361 ] ASF GitHub Bot commented on KAFKA-6123: --- KevinLiLu closed pull request #4637: KAFKA-6123: Give MetricsReporter auto-generated client.id URL: https://github.com/apache/kafka/pull/4637 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java index 3cd034eff76..ed2fa84407d 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java @@ -672,15 +672,16 @@ private KafkaConsumer(ConsumerConfig 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); +// Make sure metric reporters and interceptors get clientId +Map userProvidedConfigs = config.originals(); +userProvidedConfigs.put(ConsumerConfig.CLIENT_ID_CONFIG, clientId); List reporters = config.getConfiguredInstances(ConsumerConfig.METRIC_REPORTER_CLASSES_CONFIG, -MetricsReporter.class); +MetricsReporter.class, userProvidedConfigs); reporters.add(new JmxReporter(JMX_PREFIX)); this.metrics = new Metrics(metricConfig, reporters, time); this.retryBackoffMs = config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG); -// load interceptors and make sure they get clientId -Map userProvidedConfigs = config.originals(); -userProvidedConfigs.put(ConsumerConfig.CLIENT_ID_CONFIG, clientId); +// load interceptors List> interceptorList = (List) (new ConsumerConfig(userProvidedConfigs, false)).getConfiguredInstances(ConsumerConfig.INTERCEPTOR_CLASSES_CONFIG, ConsumerInterceptor.class); this.interceptors = new ConsumerInterceptors<>(interceptorList); diff --git a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java index 5fc9a1b9b38..53e5565454d 100644 --- a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java +++ b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java @@ -350,8 +350,10 @@ public KafkaProducer(Properties properties, Serializer keySerializer, Seriali .timeWindow(config.getLong(ProducerConfig.METRICS_SAMPLE_WINDOW_MS_CONFIG), TimeUnit.MILLISECONDS) .recordLevel(Sensor.RecordingLevel.forName(config.getString(ProducerConfig.METRICS_RECORDING_LEVEL_CONFIG))) .tags(metricTags); +// Make sure metric reporters and interceptors get clientId +userProvidedConfigs.put(ProducerConfig.CLIENT_ID_CONFIG, clientId); List reporters = config.getConfiguredInstances(ProducerConfig.METRIC_REPORTER_CLASSES_CONFIG, -MetricsReporter.class); +MetricsReporter.class, userProvidedConfigs); reporters.add(new JmxReporter(JMX_PREFIX)); this.metrics = new Metrics(metricConfig, reporters, time); ProducerMetrics metricsRegistry = new ProducerMetrics(this.metrics); @@ -374,8 +376,7 @@ public KafkaProducer(Properties properties, Serializer keySerializer, Seriali this.valueSerializer = ensureExtended(valueSerializer); } -// load interceptors and make sure they get clientId -userProvidedConfigs.put(ProducerConfig.CLIENT_ID_CONFIG, clientId); +// load interceptors List> interceptorList = (List) (new ProducerConfig(userProvidedConfigs, false)).getConfiguredInstances(ProducerConfig.INTERCEPTOR_CLASSES_CONFIG, ProducerInterceptor.class); this.interceptors = new ProducerInterceptors<>(interceptorList); diff --git a/clients/src/test/java/org/apache/kafka/common/metrics/AutoGeneratedClientIdTestMetricsReporter.java b/clients/src/test/java/org/apache/kafka/common/metrics/AutoGeneratedClientIdTestMetricsReporter.java new file mode 100644 index 000..5bf22aa97eb --- /dev/null +++ b/clients/src/test/java/org/apache/kafka/common/metrics/AutoGeneratedClientIdTestMetricsReporter.java @@ -0,0 +1,34 @@ +
[jira] [Commented] (KAFKA-6123) Give client MetricsReporter auto-generated client.id
[ https://issues.apache.org/jira/browse/KAFKA-6123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16540996#comment-16540996 ] Kevin Lu commented on KAFKA-6123: - [~cmccabe] Since you worked on the mentioned issue, what are your thoughts on this? > Give client MetricsReporter auto-generated client.id > > > Key: KAFKA-6123 > URL: https://issues.apache.org/jira/browse/KAFKA-6123 > Project: Kafka > Issue Type: Bug > Components: clients, metrics >Reporter: Kevin Lu >Assignee: Kevin Lu >Priority: Minor > Labels: clients, metrics, newbie++ > > KAFKA-4756 bugfix resolved the broker's KafkaMetricsReporter missing auto > generated broker ids, but this was not fixed on the client side. > > Metric reporters configured for clients should also be given the > auto-generated client id in the `configure` method. > The interceptors already receive the auto-generated client id. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6123) Give client MetricsReporter auto-generated client.id
[ https://issues.apache.org/jira/browse/KAFKA-6123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16547275#comment-16547275 ] ASF GitHub Bot commented on KAFKA-6123: --- KevinLiLu opened a new pull request #5383: KAFKA-6123: Give client MetricsReporter auto-generated client.id URL: https://github.com/apache/kafka/pull/5383 - Give auto generated `client.id` as a config override to `MetricsReporter` (same way [KAFKA-4756](https://issues.apache.org/jira/browse/KAFKA-4756) handles it) ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Give client MetricsReporter auto-generated client.id > > > Key: KAFKA-6123 > URL: https://issues.apache.org/jira/browse/KAFKA-6123 > Project: Kafka > Issue Type: Bug > Components: clients, metrics >Reporter: Kevin Lu >Assignee: Kevin Lu >Priority: Minor > Labels: clients, metrics, newbie++ > > KAFKA-4756 bugfix resolved the broker's KafkaMetricsReporter missing auto > generated broker ids, but this was not fixed on the client side. > > Metric reporters configured for clients should also be given the > auto-generated client id in the `configure` method. > The interceptors already receive the auto-generated client id. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6123) Give client MetricsReporter auto-generated client.id
[ https://issues.apache.org/jira/browse/KAFKA-6123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16550350#comment-16550350 ] Kevin Lu commented on KAFKA-6123: - Created [KIP-344|https://cwiki.apache.org/confluence/display/KAFKA/KIP-344%3A+The+auto-generated+client+id+should+be+passed+to+MetricsReporter]. > Give client MetricsReporter auto-generated client.id > > > Key: KAFKA-6123 > URL: https://issues.apache.org/jira/browse/KAFKA-6123 > Project: Kafka > Issue Type: Bug > Components: clients, metrics >Reporter: Kevin Lu >Assignee: Kevin Lu >Priority: Minor > Labels: clients, metrics, newbie++ > > KAFKA-4756 bugfix resolved the broker's KafkaMetricsReporter missing auto > generated broker ids, but this was not fixed on the client side. > > Metric reporters configured for clients should also be given the > auto-generated client id in the `configure` method. > The interceptors already receive the auto-generated client id. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6123) Give client MetricsReporter auto-generated client.id
[
https://issues.apache.org/jira/browse/KAFKA-6123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16637198#comment-16637198
]
ASF GitHub Bot commented on KAFKA-6123:
---
cmccabe closed pull request #5383: KAFKA-6123: Give client MetricsReporter
auto-generated client.id
URL: https://github.com/apache/kafka/pull/5383
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
index 7abe7efd15b..ceebc58301c 100644
--- a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
+++ b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
@@ -337,7 +337,8 @@ static KafkaAdminClient createInternal(AdminClientConfig
config, TimeoutProcesso
config.getLong(AdminClientConfig.RETRY_BACKOFF_MS_CONFIG),
config.getLong(AdminClientConfig.METADATA_MAX_AGE_CONFIG));
List reporters =
config.getConfiguredInstances(AdminClientConfig.METRIC_REPORTER_CLASSES_CONFIG,
-MetricsReporter.class);
+MetricsReporter.class,
+Collections.singletonMap(AdminClientConfig.CLIENT_ID_CONFIG,
clientId));
Map metricTags =
Collections.singletonMap("client-id", clientId);
MetricConfig metricConfig = new
MetricConfig().samples(config.getInt(AdminClientConfig.METRICS_NUM_SAMPLES_CONFIG))
.timeWindow(config.getLong(AdminClientConfig.METRICS_SAMPLE_WINDOW_MS_CONFIG),
TimeUnit.MILLISECONDS)
diff --git
a/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
b/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
index 4cdc4f862ec..04b8ec21591 100644
--- a/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
+++ b/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
@@ -677,7 +677,8 @@ private KafkaConsumer(ConsumerConfig config,
.recordLevel(Sensor.RecordingLevel.forName(config.getString(ConsumerConfig.METRICS_RECORDING_LEVEL_CONFIG)))
.tags(metricsTags);
List reporters =
config.getConfiguredInstances(ConsumerConfig.METRIC_REPORTER_CLASSES_CONFIG,
-MetricsReporter.class);
+MetricsReporter.class,
+Collections.singletonMap(ConsumerConfig.CLIENT_ID_CONFIG,
clientId));
reporters.add(new JmxReporter(JMX_PREFIX));
this.metrics = new Metrics(metricConfig, reporters, time);
this.retryBackoffMs =
config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG);
@@ -2211,4 +2212,8 @@ private void throwIfNoAssignorsConfigured() {
ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG + "
configuration property");
}
+// Visible for testing
+String getClientId() {
+return clientId;
+}
}
diff --git
a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
index b3f76eee49f..e249c12d5cc 100644
--- a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
+++ b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
@@ -352,7 +352,8 @@ public KafkaProducer(Properties properties, Serializer
keySerializer, Seriali
.recordLevel(Sensor.RecordingLevel.forName(config.getString(ProducerConfig.METRICS_RECORDING_LEVEL_CONFIG)))
.tags(metricTags);
List reporters =
config.getConfiguredInstances(ProducerConfig.METRIC_REPORTER_CLASSES_CONFIG,
-MetricsReporter.class);
+MetricsReporter.class,
+Collections.singletonMap(ProducerConfig.CLIENT_ID_CONFIG,
clientId));
reporters.add(new JmxReporter(JMX_PREFIX));
this.metrics = new Metrics(metricConfig, reporters, time);
ProducerMetrics metricsRegistry = new
ProducerMetrics(this.metrics);
@@ -1202,6 +1203,11 @@ private void throwIfNoTransactionManager() {
"by setting the " + ProducerConfig.TRANSACTIONAL_ID_CONFIG
+ " configuration property");
}
+// Visible for testing
+String getClientId() {
+return clientId;
+}
+
private static class ClusterAndWaitTime {
final Cluster cluster;
final long waitedOnMetadataMs;
diff --git
a/clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java
b/clients/src/test/java/org/apache/kafka/clients/consumer/KafkaCons
