[ https://issues.apache.org/jira/browse/HDFS-16946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17729782#comment-17729782 ]
ASF GitHub Bot commented on HDFS-16946: --------------------------------------- ayushtkn commented on code in PR #5696: URL: https://github.com/apache/hadoop/pull/5696#discussion_r1219823209 ########## hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/security/TestRouterSecurityManager.java: ########## @@ -259,6 +262,15 @@ private static String[] getUserGroupForTesting() { return groupsForTesting; } + @Test + public void testGetTopTokenRealOwners() throws Exception { Review Comment: this test passes only when run with the entire class, but fails if run alone ########## hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java: ########## @@ -712,13 +715,21 @@ public long getCurrentTokensCount() { @Override public String getTopTokenRealOwners() { - RouterSecurityManager mgr = - this.router.getRpcServer().getRouterSecurityManager(); - if (mgr != null && mgr.getSecretManager() != null) { - return JSON.toString(mgr.getSecretManager() - .getTopTokenRealOwners(this.topTokenRealOwners)); + String topTokenRealOwnersString = ""; + RouterSecurityManager routerSecurityManager = + this.router.getRpcServer().getRouterSecurityManager(); + if (routerSecurityManager == null || routerSecurityManager.getSecretManager() == null) + return topTokenRealOwnersString; + try { + List<Metrics2Util.NameValuePair> topOwners = routerSecurityManager.getSecretManager() + .getTopTokenRealOwners(this.topTokenRealOwners); + if (!topOwners.isEmpty()) { + topTokenRealOwnersString = new ObjectMapper().writeValueAsString(topOwners); + } + } catch (JsonProcessingException jpe) { + LOG.error("Unable to parse the top token real owners to string {}", jpe.getMessage()); } - return ""; + return topTokenRealOwnersString; Review Comment: Can we use a method from ``JsonUtil.java`` ########## hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/security/TestRouterSecurityManager.java: ########## @@ -259,6 +262,15 @@ private static String[] getUserGroupForTesting() { return groupsForTesting; } + @Test + public void testGetTopTokenRealOwners() throws Exception { + RouterMBean bean = FederationTestUtils.getBean( + ROUTER_BEAN, RouterMBean.class); + String topTokenRealOwners = bean.getTopTokenRealOwners(); + assertNotNull("The value of output should not be null " + topTokenRealOwners, topTokenRealOwners); + assertTrue("The value contains incorrect message " + topTokenRealOwners, !topTokenRealOwners.contains("NameValuePair")); Review Comment: can we assert some output which should be there post this fix rather than what shouldn't be there? btw. rather than ``!``, you could have used ``assertFalse`` > RBF: top real owners metrics can't been parsed json string > ---------------------------------------------------------- > > Key: HDFS-16946 > URL: https://issues.apache.org/jira/browse/HDFS-16946 > Project: Hadoop HDFS > Issue Type: Bug > Components: rbf > Affects Versions: 3.4.0 > Reporter: Max Xie > Assignee: Nishtha Shah > Priority: Minor > Labels: pull-request-available > Attachments: image-2023-03-09-22-24-39-833.png > > > After HDFS-15447, Add top real owners metrics for delegation tokens. But the > metrics can't been parsed json string. > RBFMetrics$getTopTokenRealOwners method just return > `org.apache.hadoop.metrics2.util.Metrics2Util$NameValuePair@1` > !image-2023-03-09-22-24-39-833.png! -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org