Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14894: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14894 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14894: URL: https://github.com/apache/kafka/pull/14894#issuecomment-1841016273 Closed followed merge of https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao merged PR #14811: URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1839774186 Just merged a related PR https://github.com/apache/kafka/pull/14767. Re-triggering the tests to make sure there are no new issues. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14894: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14894 -- 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
[PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield opened a new pull request, #14894: URL: https://github.com/apache/kafka/pull/14894 This adds the new ListClientMetricsResources RPC to the Kafka protocol and puts support into the Kafka admin client. The broker-side implementation in this PR is just to return an empty list. A future PR will obtain the list from the config store. Includes a few unit tests for what is a very simple RPC. There are additional tests already written and waiting for the PR that delivers the kafka-client-metrics.sh tool which builds on this PR. This PR supersedes https://github.com/apache/kafka/pull/14811. ### 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 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1835612791 @junrao This time Java 8, 11 and 21 completed with a handful of unrelated flaky test failures, but Java 17 timed out. The CI system seems to be a dice roll at the moment. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1834320812 I think so. After 8 hours. I will try resubmitting now (which means it will run overnight for me). In the event that it's still problematic, I'll make a fresh PR and try again. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1834296289 @AndrewJSchofield : It seems that the last build timed out? -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1832388672 Thanks for the analysis, @AndrewJSchofield. Does the following mean that some of the tests didn't run? Should we rerun the tests? ``` Build / JDK 21 and Scala 2.13 / Gradle Test Executor 92.failed to execute tests org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 92. at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:64) at java.base@21.0.1/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base@21.0.1/java.lang.reflect.Method.invoke(Method.java:580) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at jdk.proxy1/jdk.proxy1.$Proxy2.stop(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65) at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) Caused by: java.lang.OutOfMemoryError: GC overhead limit exceeded ``` -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1832366763 @junrao The failing tests are not related. Here's a summary: * Kafka Streams EOS integration tests * KRaft controller tests * Consumer coordinator failover test * Fetch from follower integration tests * Mirroring integration tests * Transactions with tiered storage tests I will file jiras for the 7 failing tests which are not currently being tracked. None of the failures appear related to the code in this PR. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1832192692 @AndrewJSchofield : Are the 40 failed tests related? Could you file jiras for failed tests not being tracked yet? -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1830378503 Closing to rerun tests -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield closed pull request #14811: KAFKA-15831: KIP-1000 protocol and admin client URL: https://github.com/apache/kafka/pull/14811 -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1830377789 The test failures are unrelated. I will rerun the tests as requested. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1830372339 Also, you could trigger a rerun of the tests by closing the PR, waiting for 20 secs and reopening it. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on code in PR #14811:
URL: https://github.com/apache/kafka/pull/14811#discussion_r1407458791
##
core/src/main/scala/kafka/network/RequestConvertToJson.scala:
##
@@ -101,6 +101,7 @@ object RequestConvertToJson {
case req: ConsumerGroupDescribeRequest =>
ConsumerGroupDescribeRequestDataJsonConverter.write(req.data, request.version)
case req: ControllerRegistrationRequest =>
ControllerRegistrationRequestDataJsonConverter.write(req.data, request.version)
case req: AssignReplicasToDirsRequest =>
AssignReplicasToDirsRequestDataJsonConverter.write(req.data, request.version)
+ case req: ListClientMetricsResourcesRequest =>
ListClientMetricsResourcesRequestDataJsonConverter.write(req.data,
request.version)
Review Comment:
Yes, it's mostly ordered with the latest added to the end. I've ordered the
lists. Thanks.
--
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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on code in PR #14811: URL: https://github.com/apache/kafka/pull/14811#discussion_r1407430988 ## clients/src/main/java/org/apache/kafka/clients/admin/ClientMetricsResourceListing.java: ## @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + *http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.admin; + +import java.util.Objects; Review Comment: Changed to match other related classes. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
apoorvmittal10 commented on code in PR #14811:
URL: https://github.com/apache/kafka/pull/14811#discussion_r1407386356
##
clients/src/main/java/org/apache/kafka/common/requests/ListClientMetricsResourcesResponse.java:
##
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.requests;
+
+import org.apache.kafka.clients.admin.ClientMetricsResourceListing;
+import
org.apache.kafka.common.message.ListClientMetricsResourcesResponseData.ClientMetricsResource;
+import org.apache.kafka.common.message.ListClientMetricsResourcesResponseData;
+import org.apache.kafka.common.protocol.ApiKeys;
+import org.apache.kafka.common.protocol.ByteBufferAccessor;
+import org.apache.kafka.common.protocol.Errors;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
+
+public class ListClientMetricsResourcesResponse extends AbstractResponse {
+private final ListClientMetricsResourcesResponseData data;
+
+public
ListClientMetricsResourcesResponse(ListClientMetricsResourcesResponseData data)
{
+super(ApiKeys.LIST_CLIENT_METRICS_RESOURCES);
+this.data = data;
+}
+
+public ListClientMetricsResourcesResponseData data() {
+return data;
+}
+
+public ApiError error() {
+return new ApiError(Errors.forCode(data.errorCode()));
+}
+
+@Override
+public Map errorCounts() {
+return errorCounts(Errors.forCode(data.errorCode()));
+}
+
+public static ListClientMetricsResourcesResponse parse(ByteBuffer buffer,
short version) {
+return new ListClientMetricsResourcesResponse(new
ListClientMetricsResourcesResponseData(
+new ByteBufferAccessor(buffer), version));
+}
+
+@Override
+public String toString() {
+return data.toString();
+}
+
+@Override
+public int throttleTimeMs() {
+return data.throttleTimeMs();
+}
+
+@Override
+public void maybeSetThrottleTimeMs(int throttleTimeMs) {
+data.setThrottleTimeMs(throttleTimeMs);
+}
+
+public Collection clientMetricsResources() {
+ArrayList resources = new
ArrayList<>(data.clientMetricsResources().size());
Review Comment:
nit
```suggestion
List resources = new
ArrayList<>(data.clientMetricsResources().size());
```
##
clients/src/main/java/org/apache/kafka/clients/admin/ClientMetricsResourceListing.java:
##
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.clients.admin;
+
+import java.util.Objects;
Review Comment:
nit: I have generally followed the convention where `java.*` packages are
after `org.*` and any `static import` are at last. I have generally seen that's
how code is structured but not sure if this is always followed.
##
clients/src/main/java/org/apache/kafka/common/requests/ListClientMetricsResourcesResponse.java:
##
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on PR #14811: URL: https://github.com/apache/kafka/pull/14811#issuecomment-1828718746 Build is almost green. A small number of test failures unrelated to this PR. -- 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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on code in PR #14811:
URL: https://github.com/apache/kafka/pull/14811#discussion_r1404634302
##
core/src/main/scala/kafka/server/KafkaApis.scala:
##
@@ -3759,6 +3760,21 @@ class KafkaApis(val requestChannel: RequestChannel,
CompletableFuture.completedFuture[Unit](())
}
+ // Just a placeholder for now.
+ def handleListClientMetricsResources(request: RequestChannel.Request):
CompletableFuture[Unit] = {
Review Comment:
Just giving the option of asynchronous completion. However, since I don't
strictly need it now, I am simplifying it to `Unit`.
##
clients/src/main/java/org/apache/kafka/clients/admin/ListClientMetricsResourcesResult.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.clients.admin;
+
+import org.apache.kafka.common.KafkaFuture;
+import org.apache.kafka.common.annotation.InterfaceStability;
+import org.apache.kafka.common.internals.KafkaFutureImpl;
+
+import java.util.Collection;
+
+/**
+ * The result of the {@link Admin#listClientMetricsResources()} call.
+ *
+ * The API of this class is evolving, see {@link Admin} for details.
+ */
+@InterfaceStability.Evolving
+public class ListClientMetricsResourcesResult {
+private final KafkaFuture> future;
+
+
ListClientMetricsResourcesResult(KafkaFuture>
future) {
+this.future = future;
+}
+
+/**
+ * Returns a future that yields either an exception, or the full set of
client metrics
+ * listings.
+ *
+ * In the event of a failure, the future yields nothing but the first
exception which
+ * occurred.
+ */
+public KafkaFuture> all() {
+final KafkaFutureImpl> result
= new KafkaFutureImpl<>();
+this.future.whenComplete((listings, throwable) -> {
Review Comment:
No. Removed.
--
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
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield commented on code in PR #14811:
URL: https://github.com/apache/kafka/pull/14811#discussion_r1404634074
##
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##
@@ -7090,6 +7093,51 @@ private static MemberDescription
convertToMemberDescriptions(DescribedGroupMembe
assignment);
}
+@Test
+public void testListClientMetricsResources() throws Exception {
+try (AdminClientUnitTestEnv env = mockClientEnv()) {
+List expected = Arrays.asList(
+new ClientMetricsResourceListing("one"),
+new ClientMetricsResourceListing("two")
+);
+
+ListClientMetricsResourcesResponseData responseData =
+new
ListClientMetricsResourcesResponseData().setErrorCode(Errors.NONE.code());
+
+responseData.clientMetricsResources()
+.add(new
ListClientMetricsResourcesResponseData.ClientMetricsResource().setName("one"));
+responseData.clientMetricsResources()
+.add((new
ListClientMetricsResourcesResponseData.ClientMetricsResource()).setName("two"));
+
+env.kafkaClient().prepareResponse(
+request -> request instanceof
ListClientMetricsResourcesRequest,
+new ListClientMetricsResourcesResponse(responseData));
+
+ListClientMetricsResourcesResult result =
env.adminClient().listClientMetricsResources();
+assertEquals(new HashSet<>(expected), new
HashSet<>(result.all().get()));
+}
+}
+
+@Test
+public void testListClientMetricsResourcesNotSupported() throws Exception {
Review Comment:
That appears true to me too, but all of the other tests in that source file
are the same pattern. I am going for consistency 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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
junrao commented on code in PR #14811:
URL: https://github.com/apache/kafka/pull/14811#discussion_r1401339000
##
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##
@@ -7090,6 +7093,51 @@ private static MemberDescription
convertToMemberDescriptions(DescribedGroupMembe
assignment);
}
+@Test
+public void testListClientMetricsResources() throws Exception {
+try (AdminClientUnitTestEnv env = mockClientEnv()) {
+List expected = Arrays.asList(
+new ClientMetricsResourceListing("one"),
+new ClientMetricsResourceListing("two")
+);
+
+ListClientMetricsResourcesResponseData responseData =
+new
ListClientMetricsResourcesResponseData().setErrorCode(Errors.NONE.code());
+
+responseData.clientMetricsResources()
+.add(new
ListClientMetricsResourcesResponseData.ClientMetricsResource().setName("one"));
+responseData.clientMetricsResources()
+.add((new
ListClientMetricsResourcesResponseData.ClientMetricsResource()).setName("two"));
+
+env.kafkaClient().prepareResponse(
+request -> request instanceof
ListClientMetricsResourcesRequest,
+new ListClientMetricsResourcesResponse(responseData));
+
+ListClientMetricsResourcesResult result =
env.adminClient().listClientMetricsResources();
+assertEquals(new HashSet<>(expected), new
HashSet<>(result.all().get()));
+}
+}
+
+@Test
+public void testListClientMetricsResourcesNotSupported() throws Exception {
Review Comment:
This method doesn't seem to throw.
##
core/src/main/scala/kafka/server/KafkaApis.scala:
##
@@ -3759,6 +3760,21 @@ class KafkaApis(val requestChannel: RequestChannel,
CompletableFuture.completedFuture[Unit](())
}
+ // Just a placeholder for now.
+ def handleListClientMetricsResources(request: RequestChannel.Request):
CompletableFuture[Unit] = {
Review Comment:
Why does this return `CompletableFuture[Unit]` instead of `Unit`?
##
clients/src/main/java/org/apache/kafka/clients/admin/ListClientMetricsResourcesResult.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.clients.admin;
+
+import org.apache.kafka.common.KafkaFuture;
+import org.apache.kafka.common.annotation.InterfaceStability;
+import org.apache.kafka.common.internals.KafkaFutureImpl;
+
+import java.util.Collection;
+
+/**
+ * The result of the {@link Admin#listClientMetricsResources()} call.
+ *
+ * The API of this class is evolving, see {@link Admin} for details.
+ */
+@InterfaceStability.Evolving
+public class ListClientMetricsResourcesResult {
+private final KafkaFuture> future;
+
+
ListClientMetricsResourcesResult(KafkaFuture>
future) {
+this.future = future;
+}
+
+/**
+ * Returns a future that yields either an exception, or the full set of
client metrics
+ * listings.
+ *
+ * In the event of a failure, the future yields nothing but the first
exception which
+ * occurred.
+ */
+public KafkaFuture> all() {
+final KafkaFutureImpl> result
= new KafkaFutureImpl<>();
+this.future.whenComplete((listings, throwable) -> {
Review Comment:
Do we need `this`?
--
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
[PR] KAFKA-15831: KIP-1000 protocol and admin client [kafka]
AndrewJSchofield opened a new pull request, #14811: URL: https://github.com/apache/kafka/pull/14811 This adds the new ListClientMetricsResources RPC to the Kafka protocol and puts support into the Kafka admin client. The broker-side implementation in this PR is just to return an empty list. A future PR will obtain the list from the config store. Includes a few unit tests for what is a very simple RPC. There are additional tests already written and waiting for the PR that delivers the `kafka-client-metrics.sh` tool which builds on this PR. ### 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 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
