Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Sep 12, 2022

What changes were proposed in this pull request?

As @Kimahriman mentioned, the default logging now goes to stdout instead of stderr, so this pr change it back to stderr.

Why are the changes needed?

Keep consistent with the behavior of log4j1.

Ref to

# Set everything to be logged to the console
log4j.rootCategory=INFO, console
log4j.appender.console=org.apache.log4j.ConsoleAppender
log4j.appender.console.target=System.err
log4j.appender.console.layout=org.apache.log4j.PatternLayout
log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n

and the log4j2.properties.template also points to SYSTEM_ERR.

rootLogger.appenderRef.stdout.ref = console
# In the pattern layout configuration below, we specify an explicit `%ex` conversion
# pattern for logging Throwables. If this was omitted, then (by default) Log4J would
# implicitly add an `%xEx` conversion pattern which logs stacktraces with additional
# class packaging information. That extra information can sometimes add a substantial
# performance overhead, so we disable it in our default logging config.
# For more information, see SPARK-39361.
appender.console.type = Console
appender.console.name = console
appender.console.target = SYSTEM_ERR
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = %d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n%ex

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

@LuciferYang
Copy link
Contributor Author

friendly ping @srowen @HyukjinKwon @dongjoon-hyun @viirya

@viirya
Copy link
Member

viirya commented Sep 13, 2022

Thanks for fixing it.

@viirya
Copy link
Member

viirya commented Sep 13, 2022

Thanks. Merging to master.

@viirya viirya closed this in 837bee5 Sep 13, 2022
@LuciferYang
Copy link
Contributor Author

thanks @HyukjinKwon @viirya

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.

3 participants