mjsax commented on code in PR #14560:
URL: https://github.com/apache/kafka/pull/14560#discussion_r1362644942


##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1660,6 +1663,34 @@ default FenceProducersResult 
fenceProducers(Collection<String> transactionalIds)
     FenceProducersResult fenceProducers(Collection<String> transactionalIds,
                                         FenceProducersOptions options);
 
+    /**
+     * Determines the client's unique client instance ID used for telemetry. 
This ID is unique to
+     * this specific client instance and will not change after it is initially 
generated.
+     * The ID is useful for correlating client operations with telemetry sent 
to the broker and
+     * to its eventual monitoring destinations.
+     *
+     * If telemetry is enabled, this will first require a connection to the 
cluster to generate
+     * the unique client instance ID. This method waits up to {@code timeout} 
for the admin
+     * client to complete the request.
+     *
+     * If telemetry is disabled, the method will throw {@link 
IllegalStateException}.
+     *
+     * Client telemetry is controlled by the {@link 
AdminClientConfig#ENABLE_METRICS_PUSH_CONFIG}
+     * configuration option.
+     *
+     * @param timeout The maximum time to wait for admin client to determine 
its client instance ID.
+     *                The value should be non-negative. Specifying a timeout 
of zero means do not

Review Comment:
   `should be` -> `must be` ?



##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -4385,6 +4385,11 @@ public FenceProducersResult 
fenceProducers(Collection<String> transactionalIds,
         return new FenceProducersResult(future.all());
     }
 
+    @Override
+    public Uuid clientInstanceId(Duration timeout) {
+        throw new UnsupportedOperationException();

Review Comment:
   nit: `UnsupportedOperationException("Not implemented yet.");`



##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1660,6 +1663,34 @@ default FenceProducersResult 
fenceProducers(Collection<String> transactionalIds)
     FenceProducersResult fenceProducers(Collection<String> transactionalIds,
                                         FenceProducersOptions options);
 
+    /**
+     * Determines the client's unique client instance ID used for telemetry. 
This ID is unique to
+     * this specific client instance and will not change after it is initially 
generated.
+     * The ID is useful for correlating client operations with telemetry sent 
to the broker and
+     * to its eventual monitoring destinations.
+     *

Review Comment:
   Insert `<p>` tag to get proper JavaDocs rendering (similar below)



##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1660,6 +1663,34 @@ default FenceProducersResult 
fenceProducers(Collection<String> transactionalIds)
     FenceProducersResult fenceProducers(Collection<String> transactionalIds,
                                         FenceProducersOptions options);
 
+    /**
+     * Determines the client's unique client instance ID used for telemetry. 
This ID is unique to
+     * this specific client instance and will not change after it is initially 
generated.
+     * The ID is useful for correlating client operations with telemetry sent 
to the broker and
+     * to its eventual monitoring destinations.
+     *
+     * If telemetry is enabled, this will first require a connection to the 
cluster to generate
+     * the unique client instance ID. This method waits up to {@code timeout} 
for the admin
+     * client to complete the request.
+     *
+     * If telemetry is disabled, the method will throw {@link 
IllegalStateException}.
+     *
+     * Client telemetry is controlled by the {@link 
AdminClientConfig#ENABLE_METRICS_PUSH_CONFIG}
+     * configuration option.
+     *
+     * @param timeout The maximum time to wait for admin client to determine 
its client instance ID.
+     *                The value should be non-negative. Specifying a timeout 
of zero means do not
+     *                wait for the initial request to complete if it hasn't 
already.
+     * @throws InterruptException If the thread is interrupted while blocked.
+     * @throws KafkaException If an unexpected error occurs while trying to 
determine the client
+     *                        instance ID, though this error does not 
necessarily imply the
+     *                        admin client is otherwise unusable.
+     * @throws IllegalArgumentException If the {@code timeout} is negative.
+     * @throws IllegalStateException If telemetry is not enabled.

Review Comment:
   ```suggestion
        * @throws IllegalStateException If telemetry is not enabled, ie, config 
`{@code enable.metrics.push}` is set to `{@code false}`.
   ```



##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1660,6 +1663,34 @@ default FenceProducersResult 
fenceProducers(Collection<String> transactionalIds)
     FenceProducersResult fenceProducers(Collection<String> transactionalIds,
                                         FenceProducersOptions options);
 
+    /**
+     * Determines the client's unique client instance ID used for telemetry. 
This ID is unique to
+     * this specific client instance and will not change after it is initially 
generated.
+     * The ID is useful for correlating client operations with telemetry sent 
to the broker and
+     * to its eventual monitoring destinations.
+     *
+     * If telemetry is enabled, this will first require a connection to the 
cluster to generate
+     * the unique client instance ID. This method waits up to {@code timeout} 
for the admin
+     * client to complete the request.
+     *
+     * If telemetry is disabled, the method will throw {@link 
IllegalStateException}.

Review Comment:
   Do we need to add this? It's already explained below via `@throws...`



-- 
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

Reply via email to