Skip to content

Conversation

@jiangzho
Copy link
Contributor

@jiangzho jiangzho commented Jul 4, 2025

What changes were proposed in this pull request?

This PR updates default log4j properties in Operator helm chart, to make sure it surfaces MDC keys and thread name as needed

Why are the changes needed?

The layout does not include all MDC keys which would provide critical information regarding the resource being reconciled.

Does this PR introduce any user-facing change?

Logging enhancements expected for operator

How was this patch tested?

CIs for lint, and validated logging in dev environments

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

No

@github-actions github-actions bot added the BUILD label Jul 4, 2025
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for proposing, @jiangzho .

However, please be careful because it could be considered as a regression to add a file-based logs by default for a user who already reduced the ephemeral storage into smaller values than 2GB. Currently, we don't generate log files by default.

it does not provide rolling file logger either.

appender.rolling.policies.size.size=100MB
appender.rolling.strategy.max=20

In addition, for the record, I remember that we had discussed the built-in log4j2 file already although you seem to forget about our previous discussion and to re-try to dump the old files again to the Apache Spark project.

In the Apache Spark community, we are very careful because there is no silver bullet for all productions environments. Some production users don't want log files at all because they are using different ways to collect logs. We had better stick to recommend their customized log4j2 file.

@jiangzho
Copy link
Contributor Author

jiangzho commented Jul 7, 2025

Thanks for the detailed background walkthough Dongjoon! I'll reduce the scope of this to update the layout for console logging only

@jiangzho jiangzho force-pushed the logging branch 2 times, most recently from 00e03b8 to fa80555 Compare July 7, 2025 18:23
…de MDC keys

### What changes were proposed in this pull request?

This PR updates default log4j properties in  Operator helm chart, to make sure it surfaces MDC keys as needed

### Why are the changes needed?

The layout does not include all MDC keys which would provide critical information regarding the resource being reconciled.

### Does this PR introduce _any_ user-facing change?

Logging enhancements expected for operator

### How was this patch tested?

CIs for lint, and validated logging in dev environments

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

No
@jiangzho jiangzho changed the title [SPARK-52679] Bundle default log4j2 properties file in Helm chart [SPARK-52679] Update default log4j2 properties in Helm chart to include MDC keys and thread name Jul 7, 2025
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for narrowing down this PR, @jiangzho .

+1, LGTM (Pending CIs).

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-52679] Update default log4j2 properties in Helm chart to include MDC keys and thread name [SPARK-52679] Revise appender.console.layout.pattern to include MDC keys and thread name Jul 7, 2025
@dongjoon-hyun
Copy link
Member

Merged to main.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @jiangzho . Unfortunately, this is too intrusive like the following.

25/07/16 22:47:03 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00553, resource.namespace=default, resource.resourceVersion=77206, resource.uid=c8c40a69-7397-4230-8bef-701c25436b54} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-73] Cleaning up resources for SparkApp:test-00553
25/07/16 22:47:03 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00554, resource.namespace=default, resource.resourceVersion=77208, resource.uid=e007ca5f-94c0-4c30-8e1e-543aafb65c67} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-52] Cleaning up resources for SparkApp:test-00554
25/07/16 22:47:03 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00555, resource.namespace=default, resource.resourceVersion=77211, resource.uid=7ea87cb4-6dca-43be-be08-ac9dca335912} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-69] Cleaning up resources for SparkApp:test-00555
25/07/16 22:47:04 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00556, resource.namespace=default, resource.resourceVersion=77213, resource.uid=839f1797-3d92-44dd-8fc0-8446a72705a1} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-159] Cleaning up resources for SparkApp:test-00556
25/07/16 22:47:04 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00557, resource.namespace=default, resource.resourceVersion=77215, resource.uid=fb230df3-77c0-426c-95f4-9cf6261eec47} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-91] Cleaning up resources for SparkApp:test-00557
25/07/16 22:47:04 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00558, resource.namespace=default, resource.resourceVersion=77218, resource.uid=01f3c114-d67d-4199-ac32-1f8e55d13cfe} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-75] Cleaning up resources for SparkApp:test-00558
25/07/16 22:47:04 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00559, resource.namespace=default, resource.resourceVersion=77220, resource.uid=978f5df3-f0f4-454c-952a-55216e02fe6a} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-89] Cleaning up resources for SparkApp:test-00559
25/07/16 22:47:04 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00560, resource.namespace=default, resource.resourceVersion=77223, resource.uid=ae48e7fc-6bbb-42da-8d30-073158cdfc1a} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-74] Cleaning up resources for SparkApp:test-00560
25/07/16 22:47:05 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00561, resource.namespace=default, resource.resourceVersion=77225, resource.uid=b84d4912-914d-4999-9ce9-b1f49779a6e3} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-59] Cleaning up resources for SparkApp:test-00561
25/07/16 22:47:05 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00562, resource.namespace=default, resource.resourceVersion=77228, resource.uid=f043891f-b935-4096-9c85-8d292a88df95} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-58] Cleaning up resources for SparkApp:test-00562
25/07/16 22:47:05 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00563, resource.namespace=default, resource.resourceVersion=77230, resource.uid=d9220ccc-a4af-4f70-9bee-f1575a43495d} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-85] Cleaning up resources for SparkApp:test-00563
25/07/16 22:47:05 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00564, resource.namespace=default, resource.resourceVersion=77232, resource.uid=3885c361-65f4-414b-8b78-d2a7e884403d} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-96] Cleaning up resources for SparkApp:test-00564
25/07/16 22:47:05 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00565, resource.namespace=default, resource.resourceVersion=77234, resource.uid=3e9dbe3e-dee2-4257-ba79-491d294f1db1} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-48] Cleaning up resources for SparkApp:test-00565
25/07/16 22:47:06 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00566, resource.namespace=default, resource.resourceVersion=77236, resource.uid=d4a65d18-2c19-4e06-85e0-25a1a19936bd} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-51] Cleaning up resources for SparkApp:test-00566
25/07/16 22:47:06 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00567, resource.namespace=default, resource.resourceVersion=77238, resource.uid=3cae1727-5a53-4e22-82ab-cee380f13bc0} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-71] Cleaning up resources for SparkApp:test-00567
25/07/16 22:47:06 INFO {resource.apiVersion=spark.apache.org/v1, resource.app.attemptId=0, resource.generation=2, resource.kind=SparkApplication, resource.name=test-00568, resource.namespace=default, resource.resourceVersion=77241, resource.uid=42dec21b-263b-4f5e-843e-54654637607c} o.a.s.k.o.r.SparkAppReconciler [ReconcilerExecutor-sparkappreconciler-62] Cleaning up resources for SparkApp:test-00568

@dongjoon-hyun
Copy link
Member

Also, FYI, cc @peter-toth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants