[GitHub] [spark] tgravescs commented on a change in pull request #30096: [SPARK-33185][YARN] Set up yarn.Client to print direct links to driver stdout/stderr

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

tgravescs commented on a change in pull request #30096:
URL: https://github.com/apache/spark/pull/30096#discussion_r517591670



##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -1172,6 +1189,26 @@ private[spark] class Client(
     }.mkString("")
   }
 
+  /**
+   * Fetch links to the logs of the driver for the given application ID. This requires hitting the
+   * RM REST API. Returns `None` if the links could not be fetched.
+   */
+  private def getDriverLogsLink(appId: ApplicationId): Option[(String, String)] = {
+    val baseRmUrl = WebAppUtils.getRMWebAppURLWithScheme(hadoopConf)
+    val response = ClientBuilder.newClient()
+      .target(baseRmUrl)
+      .path("ws").path("v1").path("cluster").path("apps").path(appId.toString).path("appattempts")
+      .request(MediaType.APPLICATION_JSON)
+      .get()
+    response.getStatusInfo.getFamily match {

Review comment:
       what happens when you query and the driver container hasn't started yet?

##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -1172,6 +1189,26 @@ private[spark] class Client(
     }.mkString("")
   }
 
+  /**
+   * Fetch links to the logs of the driver for the given application ID. This requires hitting the
+   * RM REST API. Returns `None` if the links could not be fetched.
+   */
+  private def getDriverLogsLink(appId: ApplicationId): Option[(String, String)] = {
+    val baseRmUrl = WebAppUtils.getRMWebAppURLWithScheme(hadoopConf)

Review comment:
       even though WebAppUtils is public it has an @Private annotation on it so I don't think we should use it.

##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
##########
@@ -1080,9 +1085,9 @@ private[spark] class Client(
         // If DEBUG is enabled, log report details every iteration
         // Otherwise, log them every time the application changes state
         if (log.isDebugEnabled) {
-          logDebug(formatReportDetails(report))
+          logDebug(formatReportDetails(report, getDriverLogsLink(report.getApplicationId)))
         } else if (lastState != state) {
-          logInfo(formatReportDetails(report))
+          logInfo(formatReportDetails(report, getDriverLogsLink(report.getApplicationId)))

Review comment:
       I would prefer to see this configurable as this could add quite a bit of load to the RM UI.  Also do we really need to print this every time we do the report details when the state changes?  It seems like you only need this when the driver starts, which would help with the load on RM.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org