[SPARK-48131][Core] Unify MDC key mdc.taskName and task_name

[gengliangwang]

What changes were proposed in this pull request?

Currently there are two MDC keys for task name:

• mdc.taskName, which is introduced in [SPARK-31970][CORE] Make MDC configuration step be consistent between setLocalProperty and log4j.properties #28801. Before the change, it was taskName.
• task_name: introduce from the structured logging framework project.

To make the MDC keys unified, this PR renames the mdc.taskName as task_name. This MDC is showing frequently in logs when running Spark application.
Before change:

"context":{"mdc.taskName":"task 19.0 in stage 0.0 (TID 19)”}

after change

"context":{“task_name":"task 19.0 in stage 0.0 (TID 19)”}

Why are the changes needed?

1. Make the MDC names consistent
2. Minor upside: this will allow users to query task names with SELECT * FROM logs where context.task_name = .... Otherwise, querying with context.mdc.task_name will result in an analysis exception. Users will have to query with context['mdc.task_name']

Does this PR introduce any user-facing change?

No really. The MDC key is used by developers for debugging purpose.

How was this patch tested?

Manual test

Was this patch authored or co-authored using generative AI tooling?

No

[gengliangwang]

I suggest having this merged before the Spark 4.0 preview release.

[dongjoon-hyun]

+1, LGTM, @gengliangwang .

[dongjoon-hyun]

Merged to master for Apache Spark 4.0.0-preview.

[mridulm]

Wouldn't this not impact/break existing log config files where users are customizing the template ?

[gengliangwang]

Wouldn't this not impact/break existing log config files where users are customizing the template ?

Yes, and this is documented on docs/configuration.md. If necessary, I can add this to the migration guide as well.
BTW, the key was changed from taskName to mdc.taskName in #28801. So this is not the first time it has been changed, and hopefully, we can stay with task_name this time.

[mridulm]

That change was not visible to end users, as there was not release made - right ?

[gengliangwang]

That change was not visible to end users, as there was not release made - right ?

Yes you are right.
Are you ok with the changes in this PR? If not, please let me know your preference and I can check with @cloud-fan to see if we still have time to make further changes.

[mridulm]

Is this change strictly necessary would be the question ... if it is, we can evaluate it in that context.
If not and is a nice to have, it is better not to make breaking changes which can impact users.

Whichever direction we decide on, would be good to conclude before preview release :-)

[gengliangwang]

@mridulm As the task name MDC is frequently showing in the logs; I would say this is necessary for the new logging framework. After the renaming, the MDC names are consistent and simpler to use. Otherwise, it will be executor_id/worker_id/task_id/etc and an odd mdc.taskName.

[mridulm]

So if I understood right, this as a backwardly incompatible change done for consistency of naming for Structured logging ?
If yes, IMO it is preferable to revert this and not break existing users.

+CC @dongjoon-hyun as well for thoughts.

[dongjoon-hyun]

I understand your point, @mridulm . Do you use the following syntax?

spark.sparkContext.setLocalProperty("mdc." + name, "value")

Initially, I thought it's okay because MDC is not used by default and this PR is for Apache Spark 4.0.0.

However, if there is any concern on the plain logging part, I agree with @mridulm because MDC has been supported since Apache Spark 3.1.0.

[dongjoon-hyun]

WDYT, @gengliangwang ?

If we still want to unify this, we need to do with a configuration .

[gengliangwang]

@dongjoon-hyun Thanks, I will add a configuration about the unification.