Move from java.util logging to SLF4J (#178)

Author: gabrielfeo

examples/example-project/app/build.gradle.kts

@@ -9,7 +9,7 @@ application {
 
 java {
     toolchain {
-        languageVersion.set(JavaLanguageVersion.of(8))
+        languageVersion.set(JavaLanguageVersion.of(11))
     }
 }
 

library/build.gradle.kts

@@ -153,7 +153,7 @@ java {
     withSourcesJar()
     withJavadocJar()
     toolchain {
-        languageVersion.set(JavaLanguageVersion.of(8))
+        languageVersion.set(JavaLanguageVersion.of(11))
         vendor.set(JvmVendorSpec.AZUL)
     }
 }
@@ -170,7 +170,7 @@ tasks.withType<DokkaTask>().configureEach {
             remoteUrl.set(URL("$repoUrl/blob/$version/src/main/kotlin"))
             remoteLineSuffix.set("#L")
         }
-        jdkVersion.set(8)
+        jdkVersion.set(11)
         suppressGeneratedFiles.set(false)
         documentedVisibilities.set(setOf(PUBLIC))
         perPackageOption {
@@ -230,6 +230,7 @@ tasks.named("check") {
 
 tasks.named<Test>("integrationTest") {
     jvmArgs("-Xmx512m")
+    environment("GRADLE_ENTERPRISE_API_LOG_LEVEL", "DEBUG")
 }
 
 java {
@@ -250,6 +251,8 @@ dependencies {
     implementation("com.squareup.retrofit2:converter-moshi:2.11.0")
     implementation("com.squareup.retrofit2:converter-scalars:2.11.0")
     api("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.0")
+    implementation("org.slf4j:slf4j-api:2.0.11")
+    implementation("ch.qos.logback:logback-classic:1.4.14")
 }
 
 publishing {

library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApiIntegrationTest.kt

@@ -15,8 +15,12 @@ class GradleEnterpriseApiIntegrationTest {
     @Test
     fun canFetchBuildsWithDefaultConfig() = runTest {
         env = RealEnv
-        keychain = RealKeychain(RealSystemProperties)
-        val api = GradleEnterpriseApi.newInstance()
+        keychain = realKeychain()
+        val api = GradleEnterpriseApi.newInstance(
+            config = Config(
+                cacheConfig = Config.CacheConfig(cacheEnabled = false)
+            )
+        )
         val builds = api.buildsApi.getBuilds(
             since = 0,
             maxBuilds = 5,

library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsIntegrationTest.kt

@@ -16,7 +16,7 @@ class BuildsApiExtensionsIntegrationTest {
 
     init {
         env = RealEnv
-        keychain = RealKeychain(RealSystemProperties)
+        keychain = realKeychain()
     }
 
     private val recorder = RequestRecorder()

library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/KeychainIntegrationTest.kt

@@ -8,7 +8,7 @@ internal class KeychainIntegrationTest {
     @Test
     fun getApiToken() {
         env = RealEnv
-        keychain = RealKeychain(RealSystemProperties)
+        keychain = realKeychain()
         val result = keychain.get("gradle-enterprise-api-token")
         assertIs<KeychainResult.Success>(result)
         assertFalse(result.token.isNullOrBlank(), "Keychain returned null or blank")

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Config.kt

@@ -1,10 +1,10 @@
 package com.gabrielfeo.gradle.enterprise.api
 
+import com.gabrielfeo.gradle.enterprise.api.Config.CacheConfig
 import com.gabrielfeo.gradle.enterprise.api.internal.*
 import okhttp3.Dispatcher
 import okhttp3.OkHttpClient
 import java.io.File
-import java.util.logging.Logger
 import kotlin.time.Duration.Companion.days
 
 /**
@@ -14,11 +14,14 @@ import kotlin.time.Duration.Companion.days
 data class Config(
 
     /**
-     * Enables debug logging from the library. All logging is output to stderr. By default, uses
-     * environment variable `GRADLE_ENTERPRISE_API_DEBUG_LOGGING` or `false`.
+     * Forces a log level for internal library classes. By default, the library includes
+     * logback-classic with logging disabled, aimed at notebook and script usage. Logging may be
+     * enabled for troubleshooting by changing log level to "INFO", "DEBUG", etc.
+     *
+     * To use different SLF4J bindings, simply exclude the logback dependency.
      */
-    val debugLoggingEnabled: Boolean =
-        env["GRADLE_ENTERPRISE_API_DEBUG_LOGGING"].toBoolean(),
+    val logLevel: String? =
+        env["GRADLE_ENTERPRISE_API_LOG_LEVEL"],
 
     /**
      * Provides the URL of a Gradle Enterprise API instance REST API. By default, uses
@@ -33,7 +36,7 @@ data class Config(
      * `gradle-enterprise-api-token` or environment variable `GRADLE_ENTERPRISE_API_TOKEN`.
      */
     val apiToken: () -> String = {
-        requireEnvOrKeychainToken(debugLoggingEnabled = debugLoggingEnabled)
+        requireEnvOrKeychainToken()
     },
 
     /**
@@ -192,16 +195,11 @@ data class Config(
     )
 }
 
-internal fun requireEnvOrKeychainToken(debugLoggingEnabled: Boolean): String {
+internal fun requireEnvOrKeychainToken(): String {
     if (systemProperties["os.name"] == "Mac OS X") {
         when (val result = keychain.get("gradle-enterprise-api-token")) {
             is KeychainResult.Success -> return result.token
-            is KeychainResult.Error -> {
-                if (debugLoggingEnabled) {
-                    val logger = Logger.getGlobal()
-                    logger.info("Failed to get key from keychain (${result.description})")
-                }
-            }
+            is KeychainResult.Error -> {}
         }
     }
     return env["GRADLE_ENTERPRISE_API_TOKEN"]

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApi.kt

@@ -1,5 +1,6 @@
 package com.gabrielfeo.gradle.enterprise.api
 
+import com.gabrielfeo.gradle.enterprise.api.internal.RealLoggerFactory
 import com.gabrielfeo.gradle.enterprise.api.internal.buildOkHttpClient
 import com.gabrielfeo.gradle.enterprise.api.internal.buildRetrofit
 import com.gabrielfeo.gradle.enterprise.api.internal.infrastructure.Serializer
@@ -67,7 +68,7 @@ internal class RealGradleEnterpriseApi(
 ) : GradleEnterpriseApi {
 
     private val okHttpClient by lazy {
-        buildOkHttpClient(config = config)
+        buildOkHttpClient(config = config, RealLoggerFactory(config))
     }
 
     private val retrofit: Retrofit by lazy {

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/Keychain.kt

@@ -1,6 +1,15 @@
 package com.gabrielfeo.gradle.enterprise.api.internal
 
-internal var keychain: Keychain = RealKeychain(systemProperties)
+import com.gabrielfeo.gradle.enterprise.api.Config
+import org.slf4j.Logger
+
+internal var keychain: Keychain = realKeychain()
+internal fun realKeychain() = RealKeychain(
+    RealSystemProperties,
+    // Setting level via env will work, via code won't. Not worth fixing, since keychain will
+    // be removed soon.
+    RealLoggerFactory(Config()).newLogger(RealKeychain::class),
+)
 
 internal interface Keychain {
     fun get(entry: String): KeychainResult
@@ -13,6 +22,7 @@ internal sealed interface KeychainResult {
 
 internal class RealKeychain(
     private val systemProperties: SystemProperties,
+    private val logger: Logger,
 ) : Keychain {
     override fun get(
         entry: String,
@@ -23,6 +33,7 @@ internal class RealKeychain(
             "security", "find-generic-password", "-w", "-a", login, "-s", entry
         ).start()
         val status = process.waitFor()
+        logger.debug("Keychain exit status: $status)")
         if (status != 0) {
             return KeychainResult.Error("exit $status")
         }

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/LoggerFactory.kt

@@ -0,0 +1,22 @@
+package com.gabrielfeo.gradle.enterprise.api.internal
+
+import ch.qos.logback.classic.Level
+import com.gabrielfeo.gradle.enterprise.api.Config
+import org.slf4j.Logger
+import kotlin.reflect.KClass
+
+interface LoggerFactory {
+    fun newLogger(cls: KClass<*>): Logger
+}
+
+class RealLoggerFactory(
+    private val config: Config,
+) : LoggerFactory {
+
+    override fun newLogger(cls: KClass<*>): Logger {
+        val impl = org.slf4j.LoggerFactory.getLogger(cls.java) as ch.qos.logback.classic.Logger
+        return impl.apply {
+            level = Level.valueOf(config.logLevel)
+        }
+    }
+}

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt

@@ -5,12 +5,13 @@ import com.gabrielfeo.gradle.enterprise.api.internal.auth.HttpBearerAuth
 import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheEnforcingInterceptor
 import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheHitLoggingInterceptor
 import okhttp3.Cache
+import okhttp3.Interceptor
 import okhttp3.OkHttpClient
 import okhttp3.logging.HttpLoggingInterceptor
+import okhttp3.logging.HttpLoggingInterceptor.Level.BASIC
 import okhttp3.logging.HttpLoggingInterceptor.Level.BODY
 import java.time.Duration
-import java.util.logging.Level
-import java.util.logging.Logger
+import org.slf4j.Logger
 
 /**
  * Base instance just so that multiple created [Config]s will share resources by default.
@@ -24,13 +25,14 @@ internal val basicOkHttpClient by lazy {
  */
 internal fun buildOkHttpClient(
     config: Config,
+    loggerFactory: LoggerFactory,
 ) = with(config.clientBuilder) {
     readTimeout(Duration.ofMillis(config.readTimeoutMillis))
     if (config.cacheConfig.cacheEnabled) {
-        cache(buildCache(config))
+        cache(buildCache(config, loggerFactory))
     }
-    addInterceptors(config)
-    addNetworkInterceptors(config)
+    addInterceptors(config, loggerFactory)
+    addNetworkInterceptors(config, loggerFactory)
     build().apply {
         config.maxConcurrentRequests?.let {
             dispatcher.maxRequests = it
@@ -39,31 +41,44 @@ internal fun buildOkHttpClient(
     }
 }
 
-private fun OkHttpClient.Builder.addInterceptors(config: Config) {
-    if (config.debugLoggingEnabled && config.cacheConfig.cacheEnabled) {
-        addInterceptor(CacheHitLoggingInterceptor())
+private fun OkHttpClient.Builder.addInterceptors(
+    config: Config,
+    loggerFactory: LoggerFactory,
+) {
+    if (config.cacheConfig.cacheEnabled) {
+        val logger = loggerFactory.newLogger(CacheHitLoggingInterceptor::class)
+        addInterceptor(CacheHitLoggingInterceptor(logger))
     }
 }
 
-private fun OkHttpClient.Builder.addNetworkInterceptors(config: Config) {
+private fun OkHttpClient.Builder.addNetworkInterceptors(
+    config: Config,
+    loggerFactory: LoggerFactory,
+) {
     if (config.cacheConfig.cacheEnabled) {
         addNetworkInterceptor(buildCacheEnforcingInterceptor(config))
     }
-    if (config.debugLoggingEnabled) {
-        addNetworkInterceptor(HttpLoggingInterceptor().apply { level = BODY })
+    val httpLogger = loggerFactory.newLogger(HttpLoggingInterceptor::class)
+    getHttpLoggingInterceptorForLogger(httpLogger)?.let {
+        addNetworkInterceptor(it)
     }
     addNetworkInterceptor(HttpBearerAuth("bearer", config.apiToken()))
 }
 
+private fun getHttpLoggingInterceptorForLogger(logger: Logger): Interceptor? = when {
+    logger.isDebugEnabled -> HttpLoggingInterceptor(logger = logger::debug).apply { level = BASIC }
+    logger.isTraceEnabled -> HttpLoggingInterceptor(logger = logger::debug).apply { level = BODY }
+    else -> null
+}
+
 internal fun buildCache(
-    config: Config
+    config: Config,
+    loggerFactory: LoggerFactory,
 ): Cache {
     val cacheDir = config.cacheConfig.cacheDir
     val maxSize = config.cacheConfig.maxCacheSize
-    if (config.debugLoggingEnabled) {
-        val logger = Logger.getGlobal()
-        logger.log(Level.INFO, "HTTP cache dir: $cacheDir (max ${maxSize}B)")
-    }
+    val logger = loggerFactory.newLogger(Cache::class)
+    logger.debug("HTTP cache dir: {} (max {}B)", cacheDir, maxSize)
     return Cache(cacheDir, maxSize)
 }