[GitHub] [hadoop] goiri commented on a change in pull request #2363: HDFS-13293. RBF: The RouterRPCServer should transfer CallerContext an…

[GitBox <gi...@apache.org>]

goiri commented on a change in pull request #2363:
URL: https://github.com/apache/hadoop/pull/2363#discussion_r500431295



##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -519,6 +525,21 @@ private Object invokeMethod(
     }
   }
 
+  /**
+   * For Tracking which is the actual client address.
+   * It adds key/value (clientIp/"ip") pair to the caller context.
+   */
+  private void appendClientIp2CallerContext() {
+    final CallerContext ctx = CallerContext.getCurrent();
+    CallerContext.Builder builder;
+    String origContext = ctx == null ? null : ctx.getContext();
+    byte[] origSignature = ctx == null ? null : ctx.getSignature();
+    builder = new CallerContext.Builder(origContext, clientConf);
+    builder.append(CLIENT_IP_STR, Server.getRemoteAddress());
+    builder.setSignature(origSignature);
+    CallerContext.setCurrent(builder.build());

Review comment:
       Extract the builder.build()

##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java
##########
@@ -1901,4 +1903,19 @@ private DFSClient getFileDFSClient(final String path) {
     }
     return null;
   }
+
+  @Test
+  public void testCreateWithCallerContext() throws IOException {
+    GenericTestUtils.LogCapturer auditlog =
+        GenericTestUtils.LogCapturer.captureLogs(FSNamesystem.auditLog);
+
+    // Create a directory via the router
+    String dirPath = "/test_dir_with_callercontext";
+    FsPermission permission = new FsPermission("705");
+    routerProtocol.mkdirs(dirPath, permission, false);
+
+    // The audit log should contains "callerContext=clientIp:"
+    assertTrue(auditlog.getOutput().contains("callerContext=clientIp:"));

Review comment:
       Can we verify we still contain the old context we had?

##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -404,6 +409,7 @@ private Object invokeMethod(
           " with params " + Arrays.deepToString(params) + " from "
           + router.getRouterId());
     }
+    appendClientIp2CallerContext();

Review comment:
       I would do it more formal: appendClientIpToCallerContext

##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -519,6 +525,21 @@ private Object invokeMethod(
     }
   }
 
+  /**
+   * For Tracking which is the actual client address.
+   * It adds key/value (clientIp/"ip") pair to the caller context.
+   */
+  private void appendClientIp2CallerContext() {
+    final CallerContext ctx = CallerContext.getCurrent();
+    CallerContext.Builder builder;

Review comment:
       Do this in the moment you create it in line 537.

##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -116,10 +118,13 @@
   /** Optional perf monitor. */
   private final RouterRpcMonitor rpcMonitor;
 
+  private final Configuration clientConf;
+
   /** Pattern to parse a stack trace line. */
   private static final Pattern STACK_TRACE_PATTERN =
       Pattern.compile("\\tat (.*)\\.(.*)\\((.*):(\\d*)\\)");
 
+  private static final String CLIENT_IP_STR = "clientIp";

Review comment:
       Is there something else we need to add?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org