Enable logging OkHttp traffic based on the current log level (#5750)

* Use `LogLevel` to decide whether to log the HTTP requests and responses

Added `DynamicHttpLoggingInterceptor` for this.

* Code cleanup.

* Use Timber.d

* OutOfMemoryError should not be caught. They are considered unrecoverable.

* Improve code in DefaultBugReporter.

---------

Co-authored-by: Benoit Marty <benoit@matrix.org>
This commit is contained in:
Jorge Martin Espinosa
2025-11-18 15:18:27 +01:00
committed by GitHub
parent 5a5c0b16ca
commit 396edbce3d
8 changed files with 72 additions and 40 deletions

View File

@@ -45,5 +45,5 @@ interface BugReporter {
/** /**
* Save the logcat. * Save the logcat.
*/ */
fun saveLogCat() fun saveLogCat(): File?
} }

View File

@@ -95,8 +95,6 @@ class DefaultBugReporter(
private val logcatCommandDebug = arrayOf("logcat", "-d", "-v", "threadtime", "*:*") private val logcatCommandDebug = arrayOf("logcat", "-d", "-v", "threadtime", "*:*")
private var currentTracingLogLevel: String? = null private var currentTracingLogLevel: String? = null
private val logCatErrFile: File
get() = File(logDirectory(), LOG_CAT_FILENAME)
private val baseLogDirectory = File(context.cacheDir, LOG_DIRECTORY_NAME) private val baseLogDirectory = File(context.cacheDir, LOG_DIRECTORY_NAME)
private var currentLogDirectory: File = baseLogDirectory private var currentLogDirectory: File = baseLogDirectory
@@ -160,8 +158,12 @@ class DefaultBugReporter(
} }
if (withCrashLogs || withDevicesLogs) { if (withCrashLogs || withDevicesLogs) {
saveLogCat() saveLogCat()
val gzippedLogcat = compressFile(logCatErrFile) ?.let { logCatFile ->
if (gzippedLogcat != null) { compressFile(logCatFile).also {
logCatFile.safeDelete()
}
}
?.let { gzippedLogcat ->
gzippedFiles.add(0, gzippedLogcat) gzippedFiles.add(0, gzippedLogcat)
} }
} }
@@ -387,7 +389,8 @@ class DefaultBugReporter(
onException = { Timber.e(it, "## getLogFiles() failed") } onException = { Timber.e(it, "## getLogFiles() failed") }
) { ) {
val logDirectory = logDirectory() val logDirectory = logDirectory()
logDirectory.listFiles()?.toList() logDirectory.listFiles()
?.filter { it.isFile && !it.name.endsWith(LOG_CAT_FILENAME) }
}.orEmpty() }.orEmpty()
} }
@@ -400,19 +403,19 @@ class DefaultBugReporter(
* *
* @return the file if the operation succeeds * @return the file if the operation succeeds
*/ */
override fun saveLogCat() { override fun saveLogCat(): File? {
val file = logCatErrFile val file = File(baseLogDirectory, LOG_CAT_FILENAME)
if (file.exists()) { if (file.exists()) {
file.safeDelete() file.safeDelete()
} }
try { return try {
file.writer().use { file.writer().use {
getLogCatError(it) getLogCatContent(it)
} }
} catch (error: OutOfMemoryError) { file
Timber.e(error, "## saveLogCat() : fail to write logcat OOM")
} catch (e: Exception) { } catch (e: Exception) {
Timber.e(e, "## saveLogCat() : fail to write logcat") Timber.e(e, "## saveLogCat() : fail to write logcat")
null
} }
} }
@@ -421,15 +424,10 @@ class DefaultBugReporter(
* *
* @param streamWriter the stream writer * @param streamWriter the stream writer
*/ */
private fun getLogCatError(streamWriter: OutputStreamWriter) { private fun getLogCatContent(streamWriter: OutputStreamWriter) {
val logcatProcess: Process val logcatProcess = tryOrNull {
Runtime.getRuntime().exec(logcatCommandDebug)
try { } ?: return
logcatProcess = Runtime.getRuntime().exec(logcatCommandDebug)
} catch (e1: IOException) {
return
}
try { try {
val separator = System.lineSeparator() val separator = System.lineSeparator()
logcatProcess.inputStream logcatProcess.inputStream
@@ -440,7 +438,7 @@ class DefaultBugReporter(
streamWriter.append(separator) streamWriter.append(separator)
} }
} catch (e: IOException) { } catch (e: IOException) {
Timber.e(e, "getLog fails") Timber.e(e, "getLogCatContent fails")
} }
} }
} }

View File

@@ -59,7 +59,7 @@ class FakeBugReporter(val mode: Mode = Mode.Success) : BugReporter {
// No op // No op
} }
override fun saveLogCat() { override fun saveLogCat(): File? {
// No op return null
} }
} }

View File

@@ -39,8 +39,5 @@ fun compressFile(file: File): File? {
} catch (e: Exception) { } catch (e: Exception) {
Timber.e(e, "## compressFile() failed") Timber.e(e, "## compressFile() failed")
null null
} catch (oom: OutOfMemoryError) {
Timber.e(oom, "## compressFile() failed")
null
} }
} }

View File

@@ -28,6 +28,7 @@ dependencies {
implementation(projects.libraries.core) implementation(projects.libraries.core)
implementation(projects.libraries.di) implementation(projects.libraries.di)
implementation(projects.libraries.matrix.api) implementation(projects.libraries.matrix.api)
implementation(projects.libraries.preferences.api)
implementation(platform(libs.network.okhttp.bom)) implementation(platform(libs.network.okhttp.bom))
implementation(libs.network.okhttp) implementation(libs.network.okhttp)
implementation(libs.network.okhttp.logging) implementation(libs.network.okhttp.logging)

View File

@@ -13,7 +13,7 @@ import dev.zacsweers.metro.BindingContainer
import dev.zacsweers.metro.ContributesTo import dev.zacsweers.metro.ContributesTo
import dev.zacsweers.metro.Provides import dev.zacsweers.metro.Provides
import dev.zacsweers.metro.SingleIn import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.network.interceptors.DynamicHttpLoggingInterceptor
import io.element.android.libraries.network.interceptors.FormattedJsonHttpLogger import io.element.android.libraries.network.interceptors.FormattedJsonHttpLogger
import io.element.android.libraries.network.interceptors.UserAgentInterceptor import io.element.android.libraries.network.interceptors.UserAgentInterceptor
import okhttp3.OkHttpClient import okhttp3.OkHttpClient
@@ -26,21 +26,20 @@ object NetworkModule {
@Provides @Provides
@SingleIn(AppScope::class) @SingleIn(AppScope::class)
fun providesOkHttpClient( fun providesOkHttpClient(
buildMeta: BuildMeta,
userAgentInterceptor: UserAgentInterceptor, userAgentInterceptor: UserAgentInterceptor,
dynamicHttpLoggingInterceptor: DynamicHttpLoggingInterceptor,
): OkHttpClient = OkHttpClient.Builder().apply { ): OkHttpClient = OkHttpClient.Builder().apply {
connectTimeout(30, TimeUnit.SECONDS) connectTimeout(30, TimeUnit.SECONDS)
readTimeout(60, TimeUnit.SECONDS) readTimeout(60, TimeUnit.SECONDS)
writeTimeout(60, TimeUnit.SECONDS) writeTimeout(60, TimeUnit.SECONDS)
addInterceptor(userAgentInterceptor) addInterceptor(userAgentInterceptor)
if (buildMeta.isDebuggable) addInterceptor(providesHttpLoggingInterceptor()) addInterceptor(dynamicHttpLoggingInterceptor)
}.build() }.build()
}
private fun providesHttpLoggingInterceptor(): HttpLoggingInterceptor { @Provides
val loggingLevel = HttpLoggingInterceptor.Level.BODY @SingleIn(AppScope::class)
val logger = FormattedJsonHttpLogger(loggingLevel) fun providesHttpLoggingInterceptor(): HttpLoggingInterceptor {
val interceptor = HttpLoggingInterceptor(logger) val logger = FormattedJsonHttpLogger(HttpLoggingInterceptor.Level.BODY)
interceptor.level = loggingLevel return HttpLoggingInterceptor(logger)
return interceptor }
} }

View File

@@ -0,0 +1,37 @@
/*
* Copyright (c) 2025 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.network.interceptors
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.Inject
import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.matrix.api.tracing.LogLevel
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import okhttp3.Interceptor
import okhttp3.Response
import okhttp3.logging.HttpLoggingInterceptor
import okhttp3.logging.HttpLoggingInterceptor.Level
/**
* HTTP logging interceptor that decides whether to display the HTTP logs or not based on the current log level.
*/
@Inject
@SingleIn(AppScope::class)
class DynamicHttpLoggingInterceptor(
private val appPreferencesStore: AppPreferencesStore,
private val loggingInterceptor: HttpLoggingInterceptor,
) : Interceptor by loggingInterceptor {
override fun intercept(chain: Interceptor.Chain): Response {
// This is called in a separate thread, so calling `runBlocking` here should be fine, it should be also instant after the value is cached
val logLevel = runBlocking { appPreferencesStore.getTracingLogLevelFlow().first() }
loggingInterceptor.level = if (logLevel >= LogLevel.DEBUG) Level.BODY else Level.NONE
return loggingInterceptor.intercept(chain)
}
}

View File

@@ -30,7 +30,7 @@ internal class FormattedJsonHttpLogger(
*/ */
@Synchronized @Synchronized
override fun log(message: String) { override fun log(message: String) {
Timber.v(message.ellipsize(200_000)) Timber.d(message.ellipsize(200_000))
// Try to log formatted Json only if there is a chance that [message] contains Json. // Try to log formatted Json only if there is a chance that [message] contains Json.
// It can be only the case if we log the bodies of Http requests. // It can be only the case if we log the bodies of Http requests.