Replace slf4j/k with Kotlin bindings for Log4j2

There are several problems with slf4j/k: - slf4k has seen no updates for 3 years now. - Due to its implementation via extensions functions, slf4k requires wildcard imports in order to be used conveniently. - slf4j does not offer a way to get the current log level, which is something we need for the mapping to Plexus log levels. The latter has been worked around so far by casting the logger to "ch.qos.logback.classic.Logger", which reduced the intended API / implementation separation by slf4j to absurdity ("logback-classic" should be an implementation dependency only, so consumers of ORT libraries can plug-in their own logging framework). To solve all of the above, use Log4j2 [1] and apply proper API / implementation separation, which avoids the need of consumers of ORT libraries to depend on "logback-classic". Kotlin bindings are described at [2], and see [3] for a generally very good article about the history of Java logging libraries. Logger implementations are now only used in two places: The cli module, and the test-utils module to get (debug) log output when running tests. [1] https://logging.apache.org/log4j/2.x/ [2] https://logging.apache.org/log4j/kotlin/ [3] https://www.marcobehler.com/guides/a-guide-to-logging-in-java Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Aug 21, 2019 · bac29f6 · bac29f6
1 parent b09384a
commit bac29f6
Show file tree

Hide file tree

Showing 64 changed files with 61 additions and 154 deletions.
diff --git a/.detekt.yml b/.detekt.yml
@@ -50,7 +50,6 @@ style:
   WildcardImport:
     excludes: ''
     excludeImports: '
-      ch.frankel.slf4k.*,
       com.here.ort.analyzer.managers.*,
       com.here.ort.commands.*,
       com.here.ort.downloader.vcs.*,

diff --git a/analyzer/src/main/kotlin/Analyzer.kt b/analyzer/src/main/kotlin/Analyzer.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.managers.Unmanaged
 import com.here.ort.downloader.VersionControlSystem
 import com.here.ort.downloader.vcs.GitRepo
@@ -92,7 +90,7 @@ class Analyzer(private val config: AnalyzerConfiguration) {
             Pair(manager, mappedFiles).takeIf { mappedFiles.isNotEmpty() }
         }.toMap()
 
-        if (log.isInfoEnabled) {
+        if (log.delegate.isInfoEnabled) {
             // Log the summary of projects found per package manager.
             managedFiles.forEach { (manager, files) ->
                 // No need to use curly-braces-syntax for logging here as the log level check is already done above.

diff --git a/analyzer/src/main/kotlin/PackageManager.kt b/analyzer/src/main/kotlin/PackageManager.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.downloader.VersionControlSystem
 import com.here.ort.model.Identifier
 import com.here.ort.model.OrtIssue

diff --git a/analyzer/src/main/kotlin/managers/Bower.kt b/analyzer/src/main/kotlin/managers/Bower.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 
 import com.here.ort.analyzer.AbstractPackageManagerFactory

diff --git a/analyzer/src/main/kotlin/managers/Bundler.kt b/analyzer/src/main/kotlin/managers/Bundler.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.module.kotlin.readValue
 
 import com.here.ort.analyzer.AbstractPackageManagerFactory

diff --git a/analyzer/src/main/kotlin/managers/Cargo.kt b/analyzer/src/main/kotlin/managers/Cargo.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 
 import com.here.ort.analyzer.AbstractPackageManagerFactory

diff --git a/analyzer/src/main/kotlin/managers/GoDep.kt b/analyzer/src/main/kotlin/managers/GoDep.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.PackageManager
 import com.here.ort.downloader.VersionControlSystem

diff --git a/analyzer/src/main/kotlin/managers/Gradle.kt b/analyzer/src/main/kotlin/managers/Gradle.kt
@@ -22,8 +22,6 @@ package com.here.ort.analyzer.managers
 import Dependency
 import DependencyTreeModel
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.PackageManager
 import com.here.ort.analyzer.managers.utils.MavenSupport

diff --git a/analyzer/src/main/kotlin/managers/Maven.kt b/analyzer/src/main/kotlin/managers/Maven.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.PackageManager
 import com.here.ort.analyzer.managers.utils.MavenSupport

diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.node.ObjectNode
 

diff --git a/analyzer/src/main/kotlin/managers/PhpComposer.kt b/analyzer/src/main/kotlin/managers/PhpComposer.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.node.ObjectNode
 

diff --git a/analyzer/src/main/kotlin/managers/Pip.kt b/analyzer/src/main/kotlin/managers/Pip.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.node.ArrayNode
 

diff --git a/analyzer/src/main/kotlin/managers/Pub.kt b/analyzer/src/main/kotlin/managers/Pub.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.databind.JsonNode
 
 import com.here.ort.analyzer.AbstractPackageManagerFactory

diff --git a/analyzer/src/main/kotlin/managers/Sbt.kt b/analyzer/src/main/kotlin/managers/Sbt.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.PackageManager
 import com.here.ort.model.config.AnalyzerConfiguration

diff --git a/analyzer/src/main/kotlin/managers/Stack.kt b/analyzer/src/main/kotlin/managers/Stack.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.HTTP_CACHE_PATH
 import com.here.ort.analyzer.PackageManager

diff --git a/analyzer/src/main/kotlin/managers/Unmanaged.kt b/analyzer/src/main/kotlin/managers/Unmanaged.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.analyzer.AbstractPackageManagerFactory
 import com.here.ort.analyzer.PackageManager
 import com.here.ort.downloader.VersionControlSystem

diff --git a/analyzer/src/main/kotlin/managers/utils/MavenLogger.kt b/analyzer/src/main/kotlin/managers/utils/MavenLogger.kt
@@ -19,15 +19,15 @@
 
 package com.here.ort.analyzer.managers.utils
 
-import ch.qos.logback.classic.Level
-
 import com.here.ort.utils.log
 
+import org.apache.logging.log4j.Level
+
 import org.codehaus.plexus.logging.AbstractLogger
 import org.codehaus.plexus.logging.Logger
 
 /**
- * Map a logback-classic log Level to a Plexus Logger level.
+ * Map a Log4j2 Level to a Plexus Logger level.
  */
 private fun toPlexusLoggerLevel(level: Level) =
     when (level) {
@@ -45,16 +45,16 @@ private fun toPlexusLoggerLevel(level: Level) =
  * Implementation of the Plexus [Logger] that forwards all logs to the [org.slf4j.Logger] [log] using the appropriate
  * log levels.
  */
-class MavenLogger(level: Level) : AbstractLogger(toPlexusLoggerLevel(level), log.name) {
+class MavenLogger(level: Level) : AbstractLogger(toPlexusLoggerLevel(level), log.delegate.name) {
     override fun getChildLogger(name: String?) = this
 
-    override fun debug(message: String?, throwable: Throwable?) = log.debug(message, throwable)
+    override fun debug(message: String, throwable: Throwable?) = log.debug(message, throwable)
 
-    override fun error(message: String?, throwable: Throwable?) = log.error(message, throwable)
+    override fun error(message: String, throwable: Throwable?) = log.error(message, throwable)
 
-    override fun fatalError(message: String?, throwable: Throwable?) = log.error(message, throwable)
+    override fun fatalError(message: String, throwable: Throwable?) = log.error(message, throwable)
 
-    override fun info(message: String?, throwable: Throwable?) = log.info(message, throwable)
+    override fun info(message: String, throwable: Throwable?) = log.info(message, throwable)
 
-    override fun warn(message: String?, throwable: Throwable?) = log.warn(message, throwable)
+    override fun warn(message: String, throwable: Throwable?) = log.warn(message, throwable)
 }
diff --git a/analyzer/src/main/kotlin/managers/utils/MavenSupport.kt b/analyzer/src/main/kotlin/managers/utils/MavenSupport.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers.utils
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.module.kotlin.readValue
 
 import com.here.ort.analyzer.PackageManager
@@ -113,7 +111,7 @@ class MavenSupport(workspaceReader: WorkspaceReader) {
 
             return DefaultPlexusContainer(configuration).apply {
                 loggerManager = object : BaseLoggerManager() {
-                    override fun createLogger(name: String) = MavenLogger(log.effectiveLevel)
+                    override fun createLogger(name: String) = MavenLogger(log.delegate.level)
                 }
             }
         }
@@ -333,7 +331,7 @@ class MavenSupport(workspaceReader: WorkspaceReader) {
             it.url.startsWith("file:/")
         }
 
-        if (log.isDebugEnabled) {
+        if (log.delegate.isDebugEnabled) {
             val localRepositories = allRepositories - remoteRepositories
             if (localRepositories.isNotEmpty()) {
                 // No need to use curly-braces-syntax for logging here as the log level check is already done above.

diff --git a/analyzer/src/main/kotlin/managers/utils/PackageJsonUtils.kt b/analyzer/src/main/kotlin/managers/utils/PackageJsonUtils.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.analyzer.managers.utils
 
-import ch.frankel.slf4k.*
-
 import com.fasterxml.jackson.core.JsonProcessingException
 import com.fasterxml.jackson.databind.node.ArrayNode
 import com.fasterxml.jackson.databind.node.ObjectNode

diff --git a/build.gradle.kts b/build.gradle.kts
@@ -168,7 +168,7 @@ subprojects {
         }
 
         externalDocumentationLink {
-            url = URL("https://logback.qos.ch/apidocs/")
+            url = URL("https://logging.apache.org/log4j/2.x/log4j-api/apidocs/")
         }
     }
 

diff --git a/cli/build.gradle.kts b/cli/build.gradle.kts
@@ -1,5 +1,6 @@
 val jcommanderVersion: String by project
 val kotlintestVersion: String by project
+val log4jCoreVersion: String by project
 val reflectionsVersion: String by project
 
 plugins {
@@ -30,10 +31,9 @@ dependencies {
     compile(project(":utils"))
 
     compile("com.beust:jcommander:$jcommanderVersion")
-
+    compile("org.apache.logging.log4j:log4j-core:$log4jCoreVersion")
     compile("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
     compile("org.jetbrains.kotlin:kotlin-reflect")
-
     compile("org.reflections:reflections:$reflectionsVersion")
 
     testCompile(project(":test-utils"))

diff --git a/cli/src/main/kotlin/Main.kt b/cli/src/main/kotlin/Main.kt
@@ -26,11 +26,13 @@ import com.here.ort.commands.*
 import com.here.ort.model.Environment
 import com.here.ort.utils.PARAMETER_ORDER_LOGGING
 import com.here.ort.utils.PARAMETER_ORDER_OPTIONAL
-import com.here.ort.utils.log
 import com.here.ort.utils.printStackTrace
 
 import kotlin.system.exitProcess
 
+import org.apache.logging.log4j.Level
+import org.apache.logging.log4j.core.config.Configurator
+
 const val TOOL_NAME = "ort"
 
 /**
@@ -98,8 +100,8 @@ object Main : CommandWithHelp() {
 
     override fun runCommand(jc: JCommander): Int {
         when {
-            debug -> log.level = ch.qos.logback.classic.Level.DEBUG
-            info -> log.level = ch.qos.logback.classic.Level.INFO
+            debug -> Configurator.setRootLevel(Level.DEBUG)
+            info -> Configurator.setRootLevel(Level.INFO)
         }
 
         // Make the parameter globally available.

diff --git a/cli/src/main/kotlin/commands/AnalyzerCommand.kt b/cli/src/main/kotlin/commands/AnalyzerCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.IStringConverter
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameter

diff --git a/cli/src/main/kotlin/commands/DownloaderCommand.kt b/cli/src/main/kotlin/commands/DownloaderCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameter
 import com.beust.jcommander.Parameters

diff --git a/cli/src/main/kotlin/commands/EvaluatorCommand.kt b/cli/src/main/kotlin/commands/EvaluatorCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameter
 import com.beust.jcommander.Parameters
@@ -143,7 +141,7 @@ object EvaluatorCommand : CommandWithHelp() {
 
         val evaluatorRun by lazy { evaluator.run(script) }
 
-        if (log.isErrorEnabled) {
+        if (log.delegate.isErrorEnabled) {
             evaluatorRun.violations.forEach { violation ->
                 log.error(violation.toString())
             }

diff --git a/cli/src/main/kotlin/commands/ReporterCommand.kt b/cli/src/main/kotlin/commands/ReporterCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.IStringConverter
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameter

diff --git a/cli/src/main/kotlin/commands/RequirementsCommand.kt b/cli/src/main/kotlin/commands/RequirementsCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameters
 

diff --git a/cli/src/main/kotlin/commands/ScannerCommand.kt b/cli/src/main/kotlin/commands/ScannerCommand.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.commands
 
-import ch.frankel.slf4k.*
-
 import com.beust.jcommander.IStringConverter
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.Parameter
@@ -157,10 +155,10 @@ object ScannerCommand : CommandWithHelp() {
 
         val localFileStorageLogFunction: ((String) -> Unit)? = when {
             // If the local file storage is in use, log about it already at info level.
-            log.isInfoEnabled && ScanResultsStorage.storage == localFileStorage -> log::info
+            log.delegate.isInfoEnabled && ScanResultsStorage.storage == localFileStorage -> log::info
 
             // Otherwise log about the local file storage only at debug level.
-            log.isDebugEnabled -> log::debug
+            log.delegate.isDebugEnabled -> log::debug
 
             else -> null
         }

diff --git a/cli/src/main/resources/log4j2.xml b/cli/src/main/resources/log4j2.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Configuration status="WARN">
+  <Appenders>
+    <Console name="Console" target="SYSTEM_OUT">
+      <PatternLayout pattern="%d{HH:mm:ss.SSS} %-5level - %msg%n"/>
+    </Console>
+  </Appenders>
+  <Loggers>
+    <Root level="warn">
+      <AppenderRef ref="Console"/>
+    </Root>
+  </Loggers>
+</Configuration>
diff --git a/cli/src/main/resources/logback.xml b/cli/src/main/resources/logback.xml
diff --git a/downloader/src/main/kotlin/Downloader.kt b/downloader/src/main/kotlin/Downloader.kt
@@ -19,8 +19,6 @@
 
 package com.here.ort.downloader
 
-import ch.frankel.slf4k.*
-
 import com.here.ort.downloader.vcs.GitRepo
 import com.here.ort.model.Identifier
 import com.here.ort.model.Package
-Original file line number
+Diff line change
@@ Expand Up / @@ -168,7 +168,7 @@ subprojects { @@
             }
             externalDocumentationLink {
-                url = URL("https://logback.qos.ch/apidocs/")
+                url = URL("https://logging.apache.org/log4j/2.x/log4j-api/apidocs/")
             }
         }
@@ Expand Down @@