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

cli: Replace log4j2 with logback #5510

Conversation

porsche-rbieniek
Copy link

The log4j2 logging framework recently gained some bad publicity for being
the root cause for a couple of security issues / CVEs.

A permanent solution to the security issues related to Log4j2 is a
replacement with the well-known and battle-tested logback framework.

The usage of the command-line tools remains unchanged, no additional
options are introduced. Adding the capability to pass a pathname to a
configuration file is left for future action.

Signed-off-by: Rainer Bieniek extern.rainer.bieniek@porsche.de

The log4j2 logging framework recently gained some bad publicity for being
the root cause for a couple of security issues / CVEs.

A permanent solution to the security issues related to Log4j2 is a
replacement with the well-known and battle-tested logback framework.

The usage of the command-line tools remains unchanged, no additional
options are introduced. Adding the capability to pass a pathname to a
configuration file is left for future action.

Signed-off-by: Rainer Bieniek <extern.rainer.bieniek@porsche.de>
@codecov
Copy link

codecov bot commented Jun 30, 2022

Codecov Report

Merging #5510 (b3c5115) into main (852a0af) will decrease coverage by 0.31%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #5510      +/-   ##
============================================
- Coverage     73.19%   72.87%   -0.32%     
+ Complexity     2085     2081       -4     
============================================
  Files           268      268              
  Lines         14131    14131              
  Branches       2114     2114              
============================================
- Hits          10343    10298      -45     
- Misses         2704     2755      +51     
+ Partials       1084     1078       -6     
Flag Coverage Δ
funTest-analyzer-docker 72.82% <ø> (-0.43%) ⬇️
funTest-non-analyzer 47.10% <ø> (-0.11%) ⬇️
test 32.53% <ø> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...in/kotlin/config/ProvenanceStorageConfiguration.kt 66.66% <0.00%> (-11.12%) ⬇️
model/src/main/kotlin/utils/FileArchiver.kt 75.67% <0.00%> (-8.11%) ⬇️
model/src/main/kotlin/config/OrtConfiguration.kt 91.89% <0.00%> (-5.41%) ⬇️
...n/kotlin/experimental/PackageProvenanceResolver.kt 61.29% <0.00%> (-5.38%) ⬇️
...rc/main/kotlin/config/FileArchiverConfiguration.kt 43.47% <0.00%> (-4.35%) ⬇️
analyzer/src/main/kotlin/managers/Gradle.kt 70.17% <0.00%> (-3.51%) ⬇️
...in/kotlin/experimental/NestedProvenanceResolver.kt 73.33% <0.00%> (-3.34%) ⬇️
...lyzer/src/main/kotlin/managers/SpdxDocumentFile.kt 77.04% <0.00%> (-2.19%) ⬇️
...rc/main/kotlin/experimental/ExperimentalScanner.kt 51.44% <0.00%> (-2.03%) ⬇️
downloader/src/main/kotlin/vcs/Git.kt 66.97% <0.00%> (-1.84%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 852a0af...b3c5115. Read the comment docs.

@sschuberth
Copy link
Member

@porsche-rbieniek, while waiting for your references about why Logback is the better logging framework, I came across this performance comparison which does not make Logback look all that good compared to Log4j2...

@sschuberth
Copy link
Member

Something that's also worth considering in the choice of our logging implemenation is Graal compatibility. Esp. Log4j2 seems to have issues there, unfortunately 😞

@sschuberth
Copy link
Member

Superseded by #5673.

@sschuberth sschuberth closed this Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants