bbejeck commented on code in PR #17021:
URL: https://github.com/apache/kafka/pull/17021#discussion_r1759227759
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -547,6 +551,8 @@ static KafkaAdminClient createInternal(
MetricsContext metricsContext = new KafkaMetricsContext(JMX_PREFIX,
config.originalsWithPrefix(CommonClientConfigs.METRICS_CONTEXT_PREFIX));
metrics = new Metrics(metricConfig, reporters, time,
metricsContext);
+ clientTelemetryReporter =
CommonClientConfigs.telemetryReporter(clientId, config);
+ clientTelemetryReporter.ifPresent(telemetryReporter ->
telemetryReporter.contextChange(metricsContext));
Review Comment:
I'm leaning toward not doing this, as I agree with @mjsax. Plus, there's
additional complexity in the case where metrics are unregistered. Should that
trigger the mirror action of disabling metrics pushing?
>Btw: I was just looking into the consumer/producer code, and it seem there
we just create the reporter and add to reporters list before new Metrics is
called -- this way, metricsContext will be "added" automatically to the
reporter. Should simplify the code a little bit?
I'm not sure I follow this comment. Are you referring to the reporters we
create in Kafka Streams to act as proxies for passing the metrics to the
clients and suggesting we create them in the consumer/producer clients
themselves?
Those `Reporter` instances are specific to Kafka Streams only. Also, the
`Metrics` API provides an `addReporter` method, so it seems it was designed to
add a reporter after instantiating the `Metrics` object.
--
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]
