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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
