[SPARK-40747][CORE] Support setting driver log url using env vars on other resource managers

[pan3793]

What changes were proposed in this pull request?

This PR pulls out the getDriverLogUrls from StandaloneSchedulerBackend to superclass SchedulerBackend, to make it support setting driver log url by env vars w/ prefix SPARK_DRIVER_LOG_URL_ on other resource managers, especially for K8s.

Why are the changes needed?

Since it has such an ability for the executor on K8s mode, we want to align the ability for the driver.

The related code in CoarseGrainedExecutorBackend

  def extractLogUrls: Map[String, String] = {
    val prefix = "SPARK_LOG_URL_"
    sys.env.filterKeys(_.startsWith(prefix))
      .map(e => (e._1.substring(prefix.length).toLowerCase(Locale.ROOT), e._2)).toMap
  }

Does this PR introduce any user-facing change?

Yes, the user could set the log urls by env vars.

How was this patch tested?

Existing UT, if it's not sufficient and the approach is accepted, will add more ut later.

And MT in local mode.

export SPARK_DRIVER_LOG_URL_kibana=https://kibana.svc:8080
export SPARK_DRIVER_LOG_URL_s3_archive=http://log-archive:80
bin/spark-shell --master=local

[pan3793]

cc @HeartSaVioR, would you please take a look? BTW, it does not work on Yarn since YarnClusterSchedulerBackend/YarnCoarseGrainedExecutorBackend override the methods getDriverLogUrls/getExecutorLogUrls.

[pan3793]

To make it flexible, I'm planning to add the variable substitution support to the log URL just like SPARK-26311 does(SHS only), and let the cluster manager exposes some attributes.

For example, suppose exposing APP_ID, KUBERNETES_POD_NAME in K8s, then the user could integrate to external log service easily.

spark-submit \
  --master k8s://https://<k8s-apiserver-host>:<k8s-apiserver-port> \
  --deployMode=cluster \
  --conf spark.kubernetes.driverEnv.SPARK_DRIVER_LOG_URL_log=https://spark-log-svc:8080/?app_id={{APP_ID}}&pod_name={{KUBERNETES_POD_NAME}} \
  --conf spark.executorEnv.SPARK_LOG_URL_log=https://spark-log-svc:8080/?app_id={{APP_ID}}&pod_name={{KUBERNETES_POD_NAME}} \
  ...

[AmplabJenkins]

Can one of the admins verify this patch?

[pan3793]

@dongjoon-hyun @holdenk what do think about the plan? is it the right direction to support the external log service on K8s?

[dongjoon-hyun]

Shall we explicitly mention the target resource managers instead of Support setting driver log url using env vars on other resource managers because YarnClusterSchedulerBackend will not use this implementation?

spark/resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClusterSchedulerBackend.scala

Lines 38 to 40 in b9998cf

	override def getDriverLogUrls: Option[Map[String, String]] = {
	YarnContainerInfoHelper.getLogUrls(sc.hadoopConfiguration, container = None)
	}

[pan3793]

It makes sense if we want to keep YARN as-is, another direction is to let Yarn support it then it works on all resource managers.

The pseudo-code would like

override def getDriverLogUrls: Option[Map[String, String]] = { 
   YarnContainerInfoHelper.getLogUrls(sc.hadoopConfiguration, container = None) ++
     super.getDriverLogUrls.getOrElse(Map.empty))
 } 

[dongjoon-hyun]

Do you happen to know any instances where the production YARN clusters use the external log services?

[pan3793]

it was mentioned in SPARK-26311

[dongjoon-hyun]

This PR seems to aiming only K8s because YARN is not using this and Mesos is not used in these days. May I ask why don't we have this in KubernetesClusterSchedulerBackend only?

[pan3793]

This PR seems to aiming only K8s because YARN is not using this and Mesos is not used in these days. May I ask why don't we have this in KubernetesClusterSchedulerBackend only?

I think it could be a generic way for all cluster managers, and if it's overkill, it's fine to support this feature in KubernetesClusterSchedulerBackend.

[pan3793]

Hi @dongjoon-hyun, in #38357, I implement the proposed idea, would you please take a look when you have time?

PS: design doc https://docs.google.com/document/d/1MfB39LD4B4Rp7MDRxZbMKMbdNSe6V6mBmMQ-gkCnM-0/edit?usp=sharing

[holdenk]

LGTM but needs consensus from @dongjoon-hyun

[mridulm]

I tagged @tgravescs on #38357, assuming that is the version that was proposed - or is this what we are looking at ?

[pan3793]

@mridulm Yes, SPARK-40887(#38357) is the proposed version.

I opened this PR mostly for collecting feedback in case the community has another idea.

[tgravescs]

Please link the 2 and I would prefer to see this in draft if it isn't meant to be the real solution and just looking for feedback. The jira's should ideally be linked as well.

[pan3793]

Close and in favor #38357