You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/04/19 05:57:02 UTC
[PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
panbingkun opened a new pull request, #46130:
URL: https://github.com/apache/spark/pull/46130
(no comment)
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1575209607
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala:
##########
@@ -1225,7 +1231,8 @@ private[spark] class Client(
if (log.isDebugEnabled) {
logDebug(formatReportDetails(report, getDriverLogsLink(report)))
} else if (lastState != state) {
- logInfo(formatReportDetails(report, getDriverLogsLink(report)))
+ logInfo(log"${MDC(LogKey.REPORT_DETAILS,
Review Comment:
It would be nice to expand this. But I am also fine to keep it as it is now.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #46130:
URL: https://github.com/apache/spark/pull/46130#issuecomment-2071445935
@panbingkun Thanks. Please resolve the conflicts so that I can merge this one later.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #46130:
URL: https://github.com/apache/spark/pull/46130#issuecomment-2071671520
> @panbingkun Thanks. Please resolve the conflicts so that I can merge this one later.
Done.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574574747
##########
resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala:
##########
@@ -306,8 +308,8 @@ private[spark] abstract class YarnSchedulerBackend(
.recover {
case NonFatal(e) =>
logWarning(log"Attempted to get executor loss reason for executor id " +
- log"${MDC(EXECUTOR_ID, executorId)} at RPC address " +
- log"${MDC(HOST_PORT, executorRpcAddress)}, but got no response. " +
+ log"${MDC(LogKey.EXECUTOR_ID, executorId)} at RPC address " +
Review Comment:
If imported in the form of `import org.apache.spark.internal.LogKey.{EXECUTOR_ID, HOST_PORT, REASON ...}` in the header, it will be very `long`, so it is directly imported as `import org.apache.spark.internal.LogKey` here and prefixed in the code.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574590082
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala:
##########
@@ -464,7 +465,7 @@ private[spark] class ApplicationMaster(
val dummyRunner = new ExecutorRunnable(None, yarnConf, _sparkConf, driverUrl, "<executorId>",
"<hostname>", executorMemory, executorCores, appId, securityMgr, localResources,
ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID)
- dummyRunner.launchContextDebugInfo()
+ log"${MDC(LogKey.LAUNCH_CONTEXT_DEBUG_INFO, dummyRunner.launchContextDebugInfo())}"
Review Comment:
I'm not sure if I want to expand the variables in `dummyRunner.launchContextDebugInfo()`, or is this acceptable?
https://github.com/apache/spark/blob/61dc9d991373c01d449a8ed26d9bfd7eb93f9301/resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala#L71-L86
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #46130: [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework
URL: https://github.com/apache/spark/pull/46130
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574616663
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala:
##########
@@ -1367,8 +1374,10 @@ private[spark] class Client(
if (!launcherBackend.isConnected() && fireAndForget) {
val report = getApplicationReport()
val state = report.getYarnApplicationState
- logInfo(s"Application report for $appId (state: $state)")
- logInfo(formatReportDetails(report, getDriverLogsLink(report)))
+ logInfo(log"Application report for ${MDC(LogKey.APP_ID, appId)} " +
+ log"(state: ${MDC(LogKey.APP_STATE, state)})")
+ logInfo(log"${MDC(LogKey.REPORT_DETAILS,
Review Comment:
dito
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574551452
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/SparkRackResolver.scala:
##########
@@ -77,8 +78,8 @@ private[spark] class SparkRackResolver(conf: Configuration) extends Logging {
val rNameList = dnsToSwitchMapping.resolve(hostNames.toList.asJava).asScala
if (rNameList == null || rNameList.isEmpty) {
hostNames.foreach(nodes += new NodeBase(_, NetworkTopology.DEFAULT_RACK))
- logInfo(s"Got an error when resolving hostNames. " +
- s"Falling back to ${NetworkTopology.DEFAULT_RACK} for all")
+ logInfo(log"Got an error when resolving hostNames. " +
+ log"Falling back to ${MDC(NODE_LOCATION, NetworkTopology.DEFAULT_RACK)} for all")
Review Comment:
Based on `NodeBase(String name, String location)`, it is called `NODE_LOCATION`
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574595951
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/execution/ExecuteResponseObserver.scala:
##########
@@ -246,7 +246,7 @@ private[connect] class ExecuteResponseObserver[T <: Message](val executeHolder:
// scalastyle:off line.size.limit
logInfo(
log"Release all for opId=${MDC(LogKey.OP_ID, executeHolder.operationId)}. Execution stats: " +
- log"total=${MDC(LogKey.TOTAL_SIZE, totalSize)} " +
+ log"total=${MDC(LogKey.TOTAL, totalSize)} " +
Review Comment:
Why change `TOTAL_SIZE` to `TOTAL` will be explained below.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574616185
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala:
##########
@@ -1225,7 +1231,8 @@ private[spark] class Client(
if (log.isDebugEnabled) {
logDebug(formatReportDetails(report, getDriverLogsLink(report)))
} else if (lastState != state) {
- logInfo(formatReportDetails(report, getDriverLogsLink(report)))
+ logInfo(log"${MDC(LogKey.REPORT_DETAILS,
Review Comment:
I am not sure if it is necessary to expand all variables in `ormatReportDetails(report, getDriverLogsLink(report))`, or is it acceptable in its current form?
https://github.com/apache/spark/blob/61dc9d991373c01d449a8ed26d9bfd7eb93f9301/resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala#L1307-L1326
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574577420
##########
resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala:
##########
@@ -395,7 +397,8 @@ private[spark] abstract class YarnSchedulerBackend(
override def onDisconnected(remoteAddress: RpcAddress): Unit = {
if (amEndpoint.exists(_.address == remoteAddress)) {
- logWarning(log"ApplicationMaster has disassociated: ${MDC(HOST_PORT, remoteAddress)}")
+ logWarning(log"ApplicationMaster has disassociated: " +
+ log"${MDC(LogKey.HOST_PORT, remoteAddress)}")
Review Comment:
The `remoteAddress` is `RpcAddress`.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #46130:
URL: https://github.com/apache/spark/pull/46130#issuecomment-2070607646
@panbingkun Thanks for the works! LGTM except for one comment.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1575532352
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala:
##########
@@ -464,7 +465,7 @@ private[spark] class ApplicationMaster(
val dummyRunner = new ExecutorRunnable(None, yarnConf, _sparkConf, driverUrl, "<executorId>",
"<hostname>", executorMemory, executorCores, appId, securityMgr, localResources,
ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID)
- dummyRunner.launchContextDebugInfo()
+ log"${MDC(LogKey.LAUNCH_CONTEXT_DEBUG_INFO, dummyRunner.launchContextDebugInfo())}"
Review Comment:
Done.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #46130:
URL: https://github.com/apache/spark/pull/46130#issuecomment-2069206033
cc @gengliangwang
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574563160
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientUtils.scala:
##########
@@ -141,8 +141,8 @@ private[spark] object KubernetesClientUtils extends Logging {
}
}
if (truncatedMap.nonEmpty) {
- logInfo(s"Spark configuration files loaded from $confDir :" +
- s" ${truncatedMap.keys.mkString(",")}")
+ logInfo(log"Spark configuration files loaded from ${MDC(DIR, confDir)} : " +
Review Comment:
I'm `not sure` if this is appropriate to call `PATH` `PATHS`, maybe it's more appropriate to call `DIR` `FILES`?
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #46130:
URL: https://github.com/apache/spark/pull/46130#issuecomment-2073052204
Thanks, merging to master.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1575170584
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala:
##########
@@ -464,7 +465,7 @@ private[spark] class ApplicationMaster(
val dummyRunner = new ExecutorRunnable(None, yarnConf, _sparkConf, driverUrl, "<executorId>",
"<hostname>", executorMemory, executorCores, appId, securityMgr, localResources,
ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID)
- dummyRunner.launchContextDebugInfo()
+ log"${MDC(LogKey.LAUNCH_CONTEXT_DEBUG_INFO, dummyRunner.launchContextDebugInfo())}"
Review Comment:
This one seems long. Let's expand it.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574557636
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala:
##########
@@ -255,9 +257,10 @@ private[spark] class KubernetesClusterSchedulerBackend(
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, executorIds: _*)
- if (!running.list().getItems().isEmpty()) {
- logInfo(s"Forcefully deleting ${running.list().getItems().size()} pods " +
- s"(out of ${executorIds.size}) that are still running after graceful shutdown period.")
+ if (!running.list().getItems.isEmpty) {
+ logInfo(log"Forcefully deleting ${MDC(COUNT, running.list().getItems.size())} pods " +
+ log"(out of ${MDC(TOTAL, executorIds.size)}) that are still running after graceful " +
Review Comment:
In order to reuse `TOTAL_SIZE` in `LogKey`, it was changed to `TOTAL`. At the same time, `TOTAL` has already expressed the `all` concept, so in my opinion, the suffix `_SIZE` has been removed.
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574563160
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientUtils.scala:
##########
@@ -141,8 +141,8 @@ private[spark] object KubernetesClientUtils extends Logging {
}
}
if (truncatedMap.nonEmpty) {
- logInfo(s"Spark configuration files loaded from $confDir :" +
- s" ${truncatedMap.keys.mkString(",")}")
+ logInfo(log"Spark configuration files loaded from ${MDC(DIR, confDir)} : " +
Review Comment:
I'm `not sure` if this is appropriate to call `PATH`, maybe it's more appropriate to call `DIR`?
--
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: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [WIP][SPARK-47604][CORE] Resource managers: Migrate logInfo with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46130:
URL: https://github.com/apache/spark/pull/46130#discussion_r1574592703
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala:
##########
@@ -656,8 +657,9 @@ private[spark] class ApplicationMaster(
t.setDaemon(true)
t.setName("Reporter")
t.start()
- logInfo(s"Started progress reporter thread with (heartbeat : $heartbeatInterval, " +
- s"initial allocation : $initialAllocationInterval) intervals")
+ logInfo(log"Started progress reporter thread with " +
+ log"(heartbeat: ${MDC(LogKey.HEARTBEAT_INTERVAL, heartbeatInterval)}, initial allocation: " +
Review Comment:
The time unit is `microseconds`
--
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: reviews-unsubscribe@spark.apache.org
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