You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mimaison (via GitHub)" <gi...@apache.org> on 2026/07/29 12:40:28 UTC
[PR] MINOR: Move RequestConvertToJsonTest to server module [kafka]
mimaison opened a new pull request, #22988:
URL: https://github.com/apache/kafka/pull/22988
In https://github.com/apache/kafka/commit/18340c973378610cd5930974201088b6aca8a8ad we only moved part of this class as some tests still depended on core classes.
We can now move the remaining of the tests and delete the old Scala file.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move RequestConvertToJsonTest to server module [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison merged PR #22988:
URL: https://github.com/apache/kafka/pull/22988
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move RequestConvertToJsonTest to server module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22988:
URL: https://github.com/apache/kafka/pull/22988#discussion_r3820350966
##########
server/src/test/java/org/apache/kafka/network/RequestConvertToJsonTest.java:
##########
@@ -127,4 +146,93 @@ public void testClientInfoNode() {
JsonNode actualNode = RequestConvertToJson.clientInfoNode(clientInfo);
assertEquals(expectedNode, actualNode);
}
+
+ @Test
+ public void testRequestHeaderNode() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ RequestHeader header = req.header();
+
+ ObjectNode expectedNode = (ObjectNode) RequestHeaderDataJsonConverter.write(header.data(), header.headerVersion(), false);
+ expectedNode.set("requestApiKeyName", new TextNode(header.apiKey().toString()));
+
+ JsonNode actualNode = RequestConvertToJson.requestHeaderNode(header);
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDesc() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+
+ ObjectNode expectedNode = new ObjectNode(JsonNodeFactory.instance);
+ expectedNode.set("isForwarded", req.isForwarded() ? BooleanNode.TRUE : BooleanNode.FALSE);
+ expectedNode.set("requestHeader", RequestConvertToJson.requestHeaderNode(req.header()));
+ expectedNode.set("request", req.requestLog().orElse(NullNode.getInstance()));
+
+ JsonNode actualNode = RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDescMetrics() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ NetworkSend send = new NetworkSend(req.context().connectionId, alterIsrRequest.toSend(req.header()));
+ JsonNode headerLog = RequestConvertToJson.requestHeaderNode(req.header());
+ SendResponse res = new SendResponse(req, send, Optional.of(headerLog));
+
+ int totalTimeMs = 1;
+ int requestQueueTimeMs = 2;
+ int apiLocalTimeMs = 3;
+ int apiRemoteTimeMs = 4;
+ int apiThrottleTimeMs = 5;
+ int responseQueueTimeMs = 6;
+ int responseSendTimeMs = 7;
+ int temporaryMemoryBytes = 8;
+ int messageConversionsTimeMs = 9;
+
+ ObjectNode expectedNode = (ObjectNode) RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+ expectedNode.set("response", res.responseLog().orElse(NullNode.getInstance()));
Review Comment:
I prefer to verify the fields one-by-one instead of creating a whole JSON object. It looks a bit like a copy-paste from production code😆
```java
assertEquals(false, actualNode.get("isForwarded").asBoolean());
assertEquals("connection-id", actualNode.get("connection").asText());
assertEquals(1.0, actualNode.get("totalTimeMs").asDouble());
assertEquals(2.0, actualNode.get("requestQueueTimeMs").asDouble());
assertEquals(3.0, actualNode.get("localTimeMs").asDouble());
assertEquals(4.0, actualNode.get("remoteTimeMs").asDouble());
```
##########
server/src/test/java/org/apache/kafka/network/RequestConvertToJsonTest.java:
##########
@@ -127,4 +146,93 @@ public void testClientInfoNode() {
JsonNode actualNode = RequestConvertToJson.clientInfoNode(clientInfo);
assertEquals(expectedNode, actualNode);
}
+
+ @Test
+ public void testRequestHeaderNode() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ RequestHeader header = req.header();
+
+ ObjectNode expectedNode = (ObjectNode) RequestHeaderDataJsonConverter.write(header.data(), header.headerVersion(), false);
+ expectedNode.set("requestApiKeyName", new TextNode(header.apiKey().toString()));
+
+ JsonNode actualNode = RequestConvertToJson.requestHeaderNode(header);
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDesc() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+
+ ObjectNode expectedNode = new ObjectNode(JsonNodeFactory.instance);
+ expectedNode.set("isForwarded", req.isForwarded() ? BooleanNode.TRUE : BooleanNode.FALSE);
+ expectedNode.set("requestHeader", RequestConvertToJson.requestHeaderNode(req.header()));
+ expectedNode.set("request", req.requestLog().orElse(NullNode.getInstance()));
+
+ JsonNode actualNode = RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDescMetrics() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ NetworkSend send = new NetworkSend(req.context().connectionId, alterIsrRequest.toSend(req.header()));
+ JsonNode headerLog = RequestConvertToJson.requestHeaderNode(req.header());
+ SendResponse res = new SendResponse(req, send, Optional.of(headerLog));
+
+ int totalTimeMs = 1;
+ int requestQueueTimeMs = 2;
+ int apiLocalTimeMs = 3;
+ int apiRemoteTimeMs = 4;
+ int apiThrottleTimeMs = 5;
+ int responseQueueTimeMs = 6;
+ int responseSendTimeMs = 7;
+ int temporaryMemoryBytes = 8;
+ int messageConversionsTimeMs = 9;
+
+ ObjectNode expectedNode = (ObjectNode) RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+ expectedNode.set("response", res.responseLog().orElse(NullNode.getInstance()));
+ expectedNode.set("connection", new TextNode(req.context().connectionId));
+ expectedNode.set("totalTimeMs", new DoubleNode(totalTimeMs));
+ expectedNode.set("requestQueueTimeMs", new DoubleNode(requestQueueTimeMs));
+ expectedNode.set("localTimeMs", new DoubleNode(apiLocalTimeMs));
+ expectedNode.set("remoteTimeMs", new DoubleNode(apiRemoteTimeMs));
+ expectedNode.set("throttleTimeMs", new LongNode(apiThrottleTimeMs));
+ expectedNode.set("responseQueueTimeMs", new DoubleNode(responseQueueTimeMs));
+ expectedNode.set("sendTimeMs", new DoubleNode(responseSendTimeMs));
+ expectedNode.set("securityProtocol", new TextNode(req.context().securityProtocol.name));
+ expectedNode.set("principal", new TextNode(req.session().principal.toString()));
+ expectedNode.set("listener", new TextNode(req.context().listenerName.value()));
+ expectedNode.set("clientInformation", RequestConvertToJson.clientInfoNode(req.context().clientInformation));
+ expectedNode.set("temporaryMemoryBytes", new LongNode(temporaryMemoryBytes));
Review Comment:
We could have a follow-up to include a test for the following behavior
```java
if (temporaryMemoryBytes > 0) {
node.set("temporaryMemoryBytes", new LongNode(temporaryMemoryBytes));
}
if (messageConversionsTimeMs > 0) {
node.set("messageConversionsTime", new DoubleNode(messageConversionsTimeMs));
}
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move RequestConvertToJsonTest to server module [kafka]
Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #22988:
URL: https://github.com/apache/kafka/pull/22988#discussion_r3823454759
##########
server/src/test/java/org/apache/kafka/network/RequestConvertToJsonTest.java:
##########
@@ -127,4 +146,93 @@ public void testClientInfoNode() {
JsonNode actualNode = RequestConvertToJson.clientInfoNode(clientInfo);
assertEquals(expectedNode, actualNode);
}
+
+ @Test
+ public void testRequestHeaderNode() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ RequestHeader header = req.header();
+
+ ObjectNode expectedNode = (ObjectNode) RequestHeaderDataJsonConverter.write(header.data(), header.headerVersion(), false);
+ expectedNode.set("requestApiKeyName", new TextNode(header.apiKey().toString()));
+
+ JsonNode actualNode = RequestConvertToJson.requestHeaderNode(header);
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDesc() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+
+ ObjectNode expectedNode = new ObjectNode(JsonNodeFactory.instance);
+ expectedNode.set("isForwarded", req.isForwarded() ? BooleanNode.TRUE : BooleanNode.FALSE);
+ expectedNode.set("requestHeader", RequestConvertToJson.requestHeaderNode(req.header()));
+ expectedNode.set("request", req.requestLog().orElse(NullNode.getInstance()));
+
+ JsonNode actualNode = RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDescMetrics() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ NetworkSend send = new NetworkSend(req.context().connectionId, alterIsrRequest.toSend(req.header()));
+ JsonNode headerLog = RequestConvertToJson.requestHeaderNode(req.header());
+ SendResponse res = new SendResponse(req, send, Optional.of(headerLog));
+
+ int totalTimeMs = 1;
+ int requestQueueTimeMs = 2;
+ int apiLocalTimeMs = 3;
+ int apiRemoteTimeMs = 4;
+ int apiThrottleTimeMs = 5;
+ int responseQueueTimeMs = 6;
+ int responseSendTimeMs = 7;
+ int temporaryMemoryBytes = 8;
+ int messageConversionsTimeMs = 9;
+
+ ObjectNode expectedNode = (ObjectNode) RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+ expectedNode.set("response", res.responseLog().orElse(NullNode.getInstance()));
Review Comment:
I opened a follow-up minor PR for it: https://github.com/apache/kafka/pull/23218.
##########
server/src/test/java/org/apache/kafka/network/RequestConvertToJsonTest.java:
##########
@@ -127,4 +146,93 @@ public void testClientInfoNode() {
JsonNode actualNode = RequestConvertToJson.clientInfoNode(clientInfo);
assertEquals(expectedNode, actualNode);
}
+
+ @Test
+ public void testRequestHeaderNode() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ RequestHeader header = req.header();
+
+ ObjectNode expectedNode = (ObjectNode) RequestHeaderDataJsonConverter.write(header.data(), header.headerVersion(), false);
+ expectedNode.set("requestApiKeyName", new TextNode(header.apiKey().toString()));
+
+ JsonNode actualNode = RequestConvertToJson.requestHeaderNode(header);
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDesc() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+
+ ObjectNode expectedNode = new ObjectNode(JsonNodeFactory.instance);
+ expectedNode.set("isForwarded", req.isForwarded() ? BooleanNode.TRUE : BooleanNode.FALSE);
+ expectedNode.set("requestHeader", RequestConvertToJson.requestHeaderNode(req.header()));
+ expectedNode.set("request", req.requestLog().orElse(NullNode.getInstance()));
+
+ JsonNode actualNode = RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+
+ assertEquals(expectedNode, actualNode);
+ }
+
+ @Test
+ public void testRequestDescMetrics() {
+ AlterPartitionRequest alterIsrRequest = new AlterPartitionRequest(new AlterPartitionRequestData(), ApiKeys.ALTER_PARTITION.latestVersion());
+ Request req = request(alterIsrRequest);
+ NetworkSend send = new NetworkSend(req.context().connectionId, alterIsrRequest.toSend(req.header()));
+ JsonNode headerLog = RequestConvertToJson.requestHeaderNode(req.header());
+ SendResponse res = new SendResponse(req, send, Optional.of(headerLog));
+
+ int totalTimeMs = 1;
+ int requestQueueTimeMs = 2;
+ int apiLocalTimeMs = 3;
+ int apiRemoteTimeMs = 4;
+ int apiThrottleTimeMs = 5;
+ int responseQueueTimeMs = 6;
+ int responseSendTimeMs = 7;
+ int temporaryMemoryBytes = 8;
+ int messageConversionsTimeMs = 9;
+
+ ObjectNode expectedNode = (ObjectNode) RequestConvertToJson.requestDesc(req.header(), req.requestLog(), req.isForwarded());
+ expectedNode.set("response", res.responseLog().orElse(NullNode.getInstance()));
+ expectedNode.set("connection", new TextNode(req.context().connectionId));
+ expectedNode.set("totalTimeMs", new DoubleNode(totalTimeMs));
+ expectedNode.set("requestQueueTimeMs", new DoubleNode(requestQueueTimeMs));
+ expectedNode.set("localTimeMs", new DoubleNode(apiLocalTimeMs));
+ expectedNode.set("remoteTimeMs", new DoubleNode(apiRemoteTimeMs));
+ expectedNode.set("throttleTimeMs", new LongNode(apiThrottleTimeMs));
+ expectedNode.set("responseQueueTimeMs", new DoubleNode(responseQueueTimeMs));
+ expectedNode.set("sendTimeMs", new DoubleNode(responseSendTimeMs));
+ expectedNode.set("securityProtocol", new TextNode(req.context().securityProtocol.name));
+ expectedNode.set("principal", new TextNode(req.session().principal.toString()));
+ expectedNode.set("listener", new TextNode(req.context().listenerName.value()));
+ expectedNode.set("clientInformation", RequestConvertToJson.clientInfoNode(req.context().clientInformation));
+ expectedNode.set("temporaryMemoryBytes", new LongNode(temporaryMemoryBytes));
Review Comment:
I also cover it in https://github.com/apache/kafka/pull/23218.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org