[SPARK-47603][KUBERNETES][YARN] Resource managers: Migrate logWarn with variables to structured logging framework#45957
[SPARK-47603][KUBERNETES][YARN] Resource managers: Migrate logWarn with variables to structured logging framework#45957panbingkun wants to merge 8 commits intoapache:masterfrom
Conversation
… to structured logging framework
panbingkun
changed the title
| import org.apache.spark.deploy.k8s.submit._ | ||
| import org.apache.spark.internal.Logging | ||
| import org.apache.spark.internal.{Logging, MDC} | ||
| import org.apache.spark.internal.LogKey.{CONFIG, CONFIG2, CONFIG3, CONFIG4, CONFIG5} |
There was a problem hiding this comment.
Although this looks a bit ugly, there seems to be no better way.
| } else { | ||
| logError(log"Driver terminated with exit code ${MDC(EXIT_CODE, exitCode)}! " + | ||
| log"Shutting down. ${MDC(REMOTE_ADDRESS, remoteAddress)}") | ||
| log"Shutting down. ${MDC(RPC_ADDRESS, remoteAddress)}") |
There was a problem hiding this comment.
This seems unrelated to this PR, but I found that the LogKey REMOTE_ADDRESS and RPC_ADDRESS have the same meaning, so I removed REMOTE_ADDRESS from LogKey and only kept RPC_ADDRESS
| val REDUCE_ID = Value | ||
| val RELATION_NAME = Value | ||
| val REMAINING_PARTITIONS = Value | ||
| val REMOTE_ADDRESS = Value |
There was a problem hiding this comment.
I found that the LogKey REMOTE_ADDRESS and RPC_ADDRESS have the same meaning, so I removed REMOTE_ADDRESS from LogKey and only kept RPC_ADDRESS
|
|
||
| args.foreach { mdc => | ||
| sb.append(mdc.value.toString) | ||
| val value = if (mdc.value != null) mdc.value.toString else null |
| s" Kubernetes cluster after $podCreationTimeout ms despite the fact that a previous" + | ||
| " allocation attempt tried to create them. The executors may have been deleted but the" + | ||
| " application missed the deletion event.") | ||
| logWarning(log"Executors with ids ${MDC(RESOURCE_NAME, timedOut.mkString(","))}} were not " + |
There was a problem hiding this comment.
Should this be EXECUTOR_IDS?
There was a problem hiding this comment.
Oh, yes, I misread the position of the tuple, updated
| val newFailedExecutorIds = currentFailedExecutorIds.diff(failedExecutorIds) | ||
| if (newFailedExecutorIds.nonEmpty) { | ||
| logWarning(s"${newFailedExecutorIds.size} new failed executors.") | ||
| logWarning(log"${MDC(RESOURCE_COUNT, newFailedExecutorIds.size)} new failed executors.") |
There was a problem hiding this comment.
Let's make it simply "COUNT"
|
@panbingkun Thanks for the work. LGTM except for two comments. |
Updated, done. |
|
Thanks, merging to master |
2 participants
What changes were proposed in this pull request?
The pr aims to migrate
logWanrningin moduleResource managerswith variables tostructured logging framework.Why are the changes needed?
To enhance Apache Spark's logging system by implementing structured logging.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.