[HUDI-1176]Upgrade tp log4j2 #1946
Conversation
hddong
force-pushed
the
fix-log4j2
branch
2 times, most recently
from
52638b7 to
5b62ec4
Compare
|
@wangxianghu Can you help to verify and review this PR? |
sure, will do |
| appender.console.name = consoleLogger | ||
| # CONSOLE uses PatternLayout. | ||
| appender.console.layout.type = PatternLayout | ||
| appender.console.layout.pattern = %-4r [%t] %-5p %c %x - %m%n |
There was a problem hiding this comment.
Hi @hddong , thanks for your contribution.
Can you explain why you configured two formats? If there is no special reason, keep them in the same format might be better.
The rest LGTM cc@yanghua
|
Should we just standardize on log4j instead? (wondering what happened to our slf4j efforts. ) |
Yes, it would be better to support slf4j as a facade of multiple log frameworks. @hddong Do you have time to support slf4j for hudi? If not, @wangxianghu can you take over this work? |
|
@yanghua yes, I will address this these two days. |
|
@yanghua @vinothchandar @wangxianghu : make it only use log4j now, please have a review when free. |
@wangxianghu please help to verify, when you have time. |
Ack, will do |
|
I am bit confused about this PR. |
|
@vinothchandar : yes, +1 for move to log4j2. I will do it if necessary. |
+1 on moving to log4j2. |
|
We have discussed log4j, slf4j and log4j2 now. Should we just upgrade to log4j2 across the board and be done with it? :) Is that what this PR does |
|
@vinothchandar: this PR just upgrade some modules, I will update this PR for all modules. |
Codecov Report
@@ Coverage Diff @@
## master #1946 +/- ##
============================================
- Coverage 52.26% 52.19% -0.07%
+ Complexity 3679 3678 -1
============================================
Files 484 484
Lines 23079 23079
Branches 2457 2457
============================================
- Hits 12063 12047 -16
- Misses 9952 9969 +17
+ Partials 1064 1063 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
hddong
force-pushed
the
fix-log4j2
branch
2 times, most recently
from
c625d8b to
2fb88d1
Compare
|
@yanghua @vinothchandar this PR is ready for review. |
|
@yanghua @vinothchandar: pls have a review when free. |
|
Hi @hddong，I found the newest version of |
|
@wangxianghu: had upgrade to |
Hi @hddong thanks for your patient. |
Yes, dependent on log4j2 config files now. |
|
cc @xushiyan might be good to have this done at some point. |
|
@hddong sorry that this PR was delayed and resulted in this many conflicts. would you take a look at the conflicts see if resolvable? or you mind opening a new one? |
|
@xushiyan : I will resolve these conflicts. |
|
@hddong are you still going to work on this ? If you don't have the bandwidth, I would be happy to drive this to completion and upgrade this further to Log4j 2.17.1 which has fixed several of the major CVEs that have been encountered with Log4j2 recently. Please let me know. |
|
Closing in favor of #5366 |
Tips
What is the purpose of the pull request
Now, in some modules(like cli, utilities) use log4j2, and it cannot correct load config file with error log:
ERROR StatusLogger No log4j2 configuration file found. Using default configuration: logging only errors to the console.Brief change log
(for example:)
Verify this pull request
This pull request is a trivial rework / code cleanup without any test coverage.
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.