-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[logging] Upgrade Log4j2 version to 2.14.0 and replace legacy log4j dependency #8880
Conversation
…ndency with log4j-1.2-api - import Log4j2's BOM into Pulsar's dependency management instead of defining versions separately for each Log4j2 library - distribution/server/pom.xml included log4j:log4j dependency. - replace with org.apache.logging.log4j:log4j-1.2-api to keep supporting the legacy logging api in the case something depends on it
@@ -97,8 +97,8 @@ | |||
</dependency> | |||
|
|||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This depedency could be possibly removed alltogether, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check that after investigating why the legacy log4j dependency was there previously.
@@ -95,8 +95,8 @@ | |||
</dependency> | |||
|
|||
<dependency> | |||
<artifactId>log4j</artifactId> | |||
<groupId>log4j</groupId> | |||
<groupId>org.apache.logging.log4j</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this we need the read depedency, since this determines what goes inside the distribution tar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review.
I'm just trying to understand that how the legacy log4j dependency is used. The log4j-1.2-api as part of Log4j2 provides the legacy log4j API but routes the log events to log4j2. I'd assume that this would be the correct approach if legacy log4j and log4j2 co-exist.
However is legacy log4j even needed? I see that legacy log4j is excluded from many dependencies. mvn dependency:tree
didn't give a direct answer, but I can try to investigate by temporarily removing the exclusions locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I got confused with log4j2. I think here we're adding this dependency because of ZK. Your change is probably the correct one then.
…ndency with log4j-1.2-api (#8880) - import Log4j2's BOM into Pulsar's dependency management instead of defining versions separately for each Log4j2 library - distribution/server/pom.xml included log4j:log4j dependency. - replace with org.apache.logging.log4j:log4j-1.2-api to keep supporting the legacy logging api in the case something depends on it (cherry picked from commit 2e1fb05)
…t. Replace legacy log4j with log4j-1.2-api backports PR #8880 to branch-2.7 [logging] Upgrade Log4j2 version to 2.14.0, replace legacy log4j dependency with log4j-1.2-api (#8880) - import Log4j2's BOM into Pulsar's dependency management instead of defining versions separately for each Log4j2 library - distribution/server/pom.xml included log4j:log4j dependency. - replace with org.apache.logging.log4j:log4j-1.2-api to keep supporting the legacy logging api in the case something depends on it (cherry picked from commit 2e1fb05)
…ndency with log4j-1.2-api (apache#8880) - import Log4j2's BOM into Pulsar's dependency management instead of defining versions separately for each Log4j2 library - distribution/server/pom.xml included log4j:log4j dependency. - replace with org.apache.logging.log4j:log4j-1.2-api to keep supporting the legacy logging api in the case something depends on it (cherry picked from commit 2e1fb05) (cherry picked from commit fe963bc)
…t. Replace legacy log4j with log4j-1.2-api backports PR apache#8880 to branch-2.7 [logging] Upgrade Log4j2 version to 2.14.0, replace legacy log4j dependency with log4j-1.2-api (apache#8880) - import Log4j2's BOM into Pulsar's dependency management instead of defining versions separately for each Log4j2 library - distribution/server/pom.xml included log4j:log4j dependency. - replace with org.apache.logging.log4j:log4j-1.2-api to keep supporting the legacy logging api in the case something depends on it (cherry picked from commit 2e1fb05) (cherry picked from commit 6fee20f)
Motivation
Upgrade to most recent Log4j2 version to keep it updated. Replace legacy log4j dependency with API facade for legacy API
Modifications
Upgrade Log4j2 version to 2.14.0
replace legacy log4j dependency with log4j-1.2-api
import Log4j2's BOM into Pulsar's dependency management instead of
defining versions separately for each Log4j2 library
distribution/server/pom.xml included log4j:log4j dependency.
keep supporting the legacy logging api in the case something
depends on it