Copilot commented on code in PR #8540:
URL: https://github.com/apache/hadoop/pull/8540#discussion_r3428021809
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -379,6 +385,37 @@ public String getRouters() {
return JSON.toString(info);
}
+ private static String getRouterWebAddress(Configuration conf, String
adminAddress) {
+ try {
+ if (isNullOrEmpty(adminAddress)) {
+ return "";
+ }
+ String scheme = DFSUtil.getHttpClientScheme(conf);
+ int webPort = getRouterWebAddressPort(conf, scheme);
+ InetSocketAddress adminSocketAddress =
NetUtils.createSocketAddr(adminAddress.trim());
+ return new URI(scheme, null, adminSocketAddress.getHostString(),
webPort, null, null,
+ null).toString();
+ } catch (Exception e) {
+ LOG.error("Cannot get router web address", e);
+ return "";
+ }
+ }
Review Comment:
`getRouterWebAddress` uses `isNullOrEmpty(adminAddress)` before trimming, so
whitespace-only values end up throwing and hitting the catch path. Also,
logging this best-effort derivation failure at ERROR (with stack trace) risks
noisy logs if JMX/UI polling is frequent or one router record is malformed;
this should be DEBUG (or at most WARN) and include the problematic address for
troubleshooting.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -361,6 +366,7 @@ public String getRouters() {
long dateModified = record.getDateModified();
long lastHeartbeat = getSecondsSince(dateModified);
innerInfo.put("lastHeartbeat", lastHeartbeat);
+ innerInfo.put("routerWebAddress", getRouterWebAddress(conf,
record.getAdminAddress()));
Review Comment:
New `routerWebAddress` output is added to the Routers JSON, but existing
unit coverage for `getRouters()` (e.g.,
`TestRBFMetrics#testRouterStatsDataSource`) doesn't validate it. Adding a small
assertion would help prevent regressions in scheme/port formatting and ensure
the field is present/empty as expected.
--
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]
