Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-50773][Core] Disable structured logging by default #49421

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gengliangwang
Copy link
Member

What changes were proposed in this pull request?

This PR restores the default Spark logging format to plain text instead of JSON.
After changes, compared to Spark 3.x releases, uses can optionally enable structured logging by setting configuration spark.log.structuredLogging.enabled to true (default is false). For additional customization, users can copy log4j2-json-layout.properties.template to conf/log4j2.properties and adjust as needed.

Why are the changes needed?

After discussions on the dev mailing list, Spark developers decided to revert to the previous plain text logging format for two main reasons:

  • Readability: JSON logs are verbose and not easily human-readable.
  • Setup Requirements: Structured logging requires a logging pipeline to collect JSON logs from both drivers and executors. Enabling it by default doesn’t provide much benefit without this infrastructure in place.

Does this PR introduce any user-facing change?

No, Spark 4.0 has not yet been released.

How was this patch tested?

Existing tests. Also, manually tested on enable/disable the configuration spark.log.structuredLogging.enabled, as well as verified the logging behavior with different log4j2 templates.

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

No

@gengliangwang gengliangwang changed the title Disable structured logging [SPARK-50773][Core] Disable structured logging by default Jan 8, 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.

Well, the link of the PR description was a discussion thread, not a vote.

I believe we need an official vote in order to overthrow your existing vote (24+1s including 13 binding +1s), @gengliangwang .

https://lists.apache.org/thread/kc4mcxoxz4gw0qh2v6rxxf0lyjlx9k5p ([VOTE][RESULT] SPIP: Structured Logging Framework for Apache Spark)

The vote passes with 24+1s (13 binding +1s).

For the record, I don't think new vote will fail. Since the existing plain text is also the traditional way, I believe new vote will pass. I'm just saying the discussion thread is insufficient in this case.

@HyukjinKwon
Copy link
Member

Oh, yeah +1 for vote.

@gengliangwang
Copy link
Member Author

@dongjoon-hyun sure, let's start a vote.

@dongjoon-hyun
Copy link
Member

Thank you, @HyukjinKwon and @gengliangwang .

Copy link
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM and +1 for the vote

Copy link
Contributor

@allisonwang-db allisonwang-db left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Successfully merging this pull request may close these issues.

6 participants