goiri commented on code in PR #4659:
URL: https://github.com/apache/hadoop/pull/4659#discussion_r933363776


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:
##########
@@ -1351,6 +1357,33 @@ private class ResponseParams {
 
     @Override
     public String toString() {
+      boolean isCallerContextEnabled = conf.getBoolean(
+          HADOOP_CALLER_CONTEXT_ENABLED_KEY,
+          HADOOP_CALLER_CONTEXT_ENABLED_DEFAULT);
+      CallerContext context = getCallerContext();
+      if (isCallerContextEnabled && context != null && 
context.isContextValid()) {
+        String cc = context.getContext();
+        String clientIp = "";
+        String ipKey = CallerContext.CLIENT_IP_STR +
+                CallerContext.Builder.KEY_VALUE_SEPARATOR;
+        int posn = cc.indexOf(ipKey);
+        if (posn != -1) {
+          posn += ipKey.length();
+          int end = cc.indexOf(",", posn);
+          clientIp = (end == -1 ? cc.substring(posn) : cc.substring(posn, 
end));
+        }
+        String clientPort = "";
+        String portKey = CallerContext.CLIENT_PORT_STR +

Review Comment:
   Single line.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:
##########
@@ -1351,6 +1357,33 @@ private class ResponseParams {
 
     @Override
     public String toString() {
+      boolean isCallerContextEnabled = conf.getBoolean(
+          HADOOP_CALLER_CONTEXT_ENABLED_KEY,
+          HADOOP_CALLER_CONTEXT_ENABLED_DEFAULT);
+      CallerContext context = getCallerContext();
+      if (isCallerContextEnabled && context != null && 
context.isContextValid()) {
+        String cc = context.getContext();
+        String clientIp = "";
+        String ipKey = CallerContext.CLIENT_IP_STR +

Review Comment:
   Single line.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java:
##########
@@ -2070,6 +2071,23 @@ public void testSetBalancerBandwidth() throws Exception {
     }, 100, 60 * 1000);
   }
 
+  @Test
+  public void testRecordRealClientIp() {
+    GenericTestUtils.LogCapturer logger =
+        GenericTestUtils.LogCapturer.captureLogs(Server.LOG);
+    CallerContext.setCurrent(
+        new CallerContext.Builder(
+            "clientContext,clientIp:2.2.2.2,clientPort:1234").build());
+    try {
+      String filePath = "/test/f.log";
+      routerProtocol.getBlockLocations(filePath, 0, 100);
+    } catch (Exception e) {
+      // do nothing
+    } finally {
+      assertTrue(logger.getOutput().contains("2.2.2.2:1234"));

Review Comment:
   Can we test with HADOOP_CALLER_CONTEXT_ENABLED_KEY set to true and false?



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:
##########
@@ -1351,6 +1357,33 @@ private class ResponseParams {
 
     @Override
     public String toString() {
+      boolean isCallerContextEnabled = conf.getBoolean(
+          HADOOP_CALLER_CONTEXT_ENABLED_KEY,
+          HADOOP_CALLER_CONTEXT_ENABLED_DEFAULT);
+      CallerContext context = getCallerContext();
+      if (isCallerContextEnabled && context != null && 
context.isContextValid()) {
+        String cc = context.getContext();

Review Comment:
   There is a lot of parsing logic here. It would be better to extract it and 
make it more general and add specific 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to