Skip to content

Commit 9d5c371

Browse files
authored
Migrate from Timber to logcat (#6133)
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1210367591024989 ### Description This pull request replaces the `Timber` logging library with Square's `logcat` library across the codebase. The change improves logging readability, consistency, and performance by adopting Kotlin string interpolation and `logcat`'s structured logging features. - Updated the `STYLEGUIDE.md` to reflect the switch from `Timber` to `logcat`, including examples of logging with different priorities and handling exceptions using the `asLog()` extension. - Replaced the `Timber` dependency with the `logcat` library in all classes ### Steps to test this PR - [ ] Build and run all variants ### UI changes N/A
1 parent 5c843ad commit 9d5c371

File tree

446 files changed

+2325
-1898
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

446 files changed

+2325
-1898
lines changed

STYLEGUIDE.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,21 @@ If you want to do this automatically upon commit we recommend the existing [pre-
1515
## Code conventions
1616

1717
### Logging
18-
When logging with Timber we use the new Kotlin styles strings
18+
We use Square's logcat library for logging. When logging, we use Kotlin string interpolation:
1919

20-
```Timber.w("Loading $url")```
20+
```logcat { "Loading $url" }```
2121

22-
Rather than C style strings
22+
By default, calling logcat without a priority logs at DEBUG level.
2323

24-
```Timber.w("Loading %s", url)```
24+
For different log levels, specify the priority:
2525

26-
Mixing the two styles within a single statement can lead to crashes so we have standardized on the more readable Kotlin style. This is slightly less efficient - should efficiency become an issue we can use proguard to optimize away log statements for releases.
26+
```logcat(WARN) { "Loading $url" }```
27+
28+
For logging exceptions, use the `asLog()` extension:
29+
30+
```logcat(ERROR) { "Failed to load: ${exception.asLog()}" }```
31+
32+
This approach provides better performance and readability compared to traditional string formatting.
2733

2834
### Package Names
2935
Case in package names is problematic as some file system and tools do not handle case sensitive file changes well. For this reason, we opt for lowercase packages in our project. Thus we have:

ad-click/ad-click-impl/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ dependencies {
8383
exclude group: "org.jetbrains.kotlinx", module: "kotlinx-coroutines-debug"
8484
}
8585

86-
implementation JakeWharton.timber
86+
implementation "com.squareup.logcat:logcat:_"
8787

8888
implementation Square.retrofit2.converter.moshi
8989
implementation Square.okHttp3.okHttp

ad-click/ad-click-impl/src/main/java/com/duckduckgo/adclick/impl/DataRemovalAdClickWorker.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ import com.squareup.anvil.annotations.ContributesMultibinding
3333
import java.util.concurrent.TimeUnit
3434
import javax.inject.Inject
3535
import kotlinx.coroutines.withContext
36-
import timber.log.Timber
36+
import logcat.LogPriority.VERBOSE
37+
import logcat.logcat
3738

3839
@ContributesWorker(AppScope::class)
3940
class DataRemovalAdClickWorker(context: Context, workerParameters: WorkerParameters) :
@@ -64,7 +65,7 @@ class DataRemovalAdClickWorkerScheduler @Inject constructor(
6465

6566
override fun onCreate(owner: LifecycleOwner) {
6667
super.onCreate(owner)
67-
Timber.v("Scheduling ad click data removal worker")
68+
logcat(VERBOSE) { "Scheduling ad click data removal worker" }
6869
val workerRequest = PeriodicWorkRequestBuilder<DataRemovalAdClickWorker>(12, TimeUnit.HOURS)
6970
.addTag(DATA_REMOVAL_AD_CLICK_WORKER_TAG)
7071
.setBackoffCriteria(BackoffPolicy.LINEAR, 10, TimeUnit.MINUTES)

ad-click/ad-click-impl/src/main/java/com/duckduckgo/adclick/impl/DuckDuckGoAdClickData.kt

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import com.duckduckgo.common.utils.DispatcherProvider
2424
import java.util.concurrent.ConcurrentHashMap
2525
import kotlinx.coroutines.CoroutineScope
2626
import kotlinx.coroutines.launch
27-
import timber.log.Timber
27+
import logcat.logcat
2828

2929
interface AdClickData {
3030

@@ -72,30 +72,30 @@ class DuckDuckGoAdClickData(
7272

7373
override fun setAdDomainTldPlusOne(adDomainTldPlusOne: String) {
7474
tabAdDomains[activeTabId] = adDomainTldPlusOne
75-
Timber.d("Detected ad. Tab ad domains: $tabAdDomains")
75+
logcat { "Detected ad. Tab ad domains: $tabAdDomains" }
7676
}
7777

7878
override fun removeAdDomain() {
7979
tabAdDomains.remove(activeTabId)
80-
Timber.d("Removed previously detected ad. Tab ad domains: $tabAdDomains")
80+
logcat { "Removed previously detected ad. Tab ad domains: $tabAdDomains" }
8181
}
8282

8383
override fun removeAdDomain(tabId: String) {
8484
tabAdDomains.remove(tabId)
85-
Timber.d("Removed previously detected ad for $tabId. Current active tabId is $activeTabId. Tab ad domains: $tabAdDomains")
85+
logcat { "Removed previously detected ad for $tabId. Current active tabId is $activeTabId. Tab ad domains: $tabAdDomains" }
8686
}
8787

8888
override fun setActiveTab(tabId: String) {
8989
this.activeTabId = tabId
9090
}
9191

9292
override fun getAdDomainTldPlusOne(): String? {
93-
Timber.d("Is ad? ${tabAdDomains[activeTabId] != null} Tab ad domains: $tabAdDomains")
93+
logcat { "Is ad? ${tabAdDomains[activeTabId] != null} Tab ad domains: $tabAdDomains" }
9494
return tabAdDomains[activeTabId]
9595
}
9696

9797
override fun getAdDomainTldPlusOne(tabId: String): String? {
98-
Timber.d("Is ad for tabId $tabId? ${tabAdDomains[tabId] != null}. Active tab is $activeTabId. Tab ad domains: $tabAdDomains")
98+
logcat { "Is ad for tabId $tabId? ${tabAdDomains[tabId] != null}. Active tab is $activeTabId. Tab ad domains: $tabAdDomains" }
9999
return tabAdDomains[tabId]
100100
}
101101

@@ -106,7 +106,7 @@ class DuckDuckGoAdClickData(
106106
adClickExemptionsDao.deleteTabExemption(activeTabId)
107107
}
108108
}
109-
Timber.d("Removed exemption for active tab $activeTabId. Tab exemptions: $tabExemptions")
109+
logcat { "Removed exemption for active tab $activeTabId. Tab exemptions: $tabExemptions" }
110110
}
111111

112112
override fun addExemption(exemption: Exemption) {
@@ -124,7 +124,7 @@ class DuckDuckGoAdClickData(
124124
)
125125
}
126126
}
127-
Timber.d("Added exemption for active tab $activeTabId. Tab exemptions: $tabExemptions")
127+
logcat { "Added exemption for active tab $activeTabId. Tab exemptions: $tabExemptions" }
128128
}
129129

130130
override fun addExemption(tabId: String, exemption: Exemption) {
@@ -142,11 +142,11 @@ class DuckDuckGoAdClickData(
142142
)
143143
}
144144
}
145-
Timber.d("Added exemption for tab $tabId. Tab exemptions: $tabExemptions")
145+
logcat { "Added exemption for tab $tabId. Tab exemptions: $tabExemptions" }
146146
}
147147

148148
override fun getExemption(): Exemption? {
149-
Timber.d("Get exemption for tab $activeTabId. Tab exemptions: $tabExemptions")
149+
logcat { "Get exemption for tab $activeTabId. Tab exemptions: $tabExemptions" }
150150
return tabExemptions[activeTabId]
151151
}
152152

@@ -167,7 +167,7 @@ class DuckDuckGoAdClickData(
167167
adClickExemptionsDao.deleteTabExemption(tabId)
168168
}
169169
}
170-
Timber.d("Removed data for tab $tabId. Tab ad domains: $tabAdDomains. Tab exemptions: $tabExemptions")
170+
logcat { "Removed data for tab $tabId. Tab ad domains: $tabAdDomains. Tab exemptions: $tabExemptions" }
171171
}
172172

173173
override fun removeAll() {
@@ -178,7 +178,7 @@ class DuckDuckGoAdClickData(
178178
adClickExemptionsDao.deleteAllTabExemptions()
179179
}
180180
}
181-
Timber.d("Removed all data. Ad clicked map is empty ${tabAdDomains.isEmpty()}. Empty tab exemptions? ${tabExemptions.isEmpty()}")
181+
logcat { "Removed all data. Ad clicked map is empty ${tabAdDomains.isEmpty()}. Empty tab exemptions? ${tabExemptions.isEmpty()}" }
182182
}
183183

184184
override fun removeAllExpired() {
@@ -195,7 +195,7 @@ class DuckDuckGoAdClickData(
195195
adClickExemptionsDao.deleteAllExpiredTabExemptions(currentTime)
196196
}
197197
}
198-
Timber.d("Removed all expired data. Tab exemptions: $tabExemptions")
198+
logcat { "Removed all expired data. Tab exemptions: $tabExemptions" }
199199
}
200200

201201
override fun setCurrentPage(currentPageUrl: String) {

ad-click/ad-click-impl/src/main/java/com/duckduckgo/adclick/impl/DuckDuckGoAdClickManager.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import com.duckduckgo.di.scopes.AppScope
2424
import com.squareup.anvil.annotations.ContributesBinding
2525
import dagger.SingleInstanceIn
2626
import javax.inject.Inject
27+
import logcat.logcat
2728
import okhttp3.internal.publicsuffix.PublicSuffixDatabase
28-
import timber.log.Timber
2929

3030
@SingleInstanceIn(AppScope::class)
3131
@ContributesBinding(AppScope::class)
@@ -70,17 +70,17 @@ class DuckDuckGoAdClickManager @Inject constructor(
7070
}
7171

7272
override fun clearTabId(tabId: String) {
73-
Timber.d("Clear data for tab $tabId.")
73+
logcat { "Clear data for tab $tabId." }
7474
adClickData.remove(tabId)
7575
}
7676

7777
override fun clearAll() {
78-
Timber.d("Clear all data.")
78+
logcat { "Clear all data." }
7979
adClickData.removeAll()
8080
}
8181

8282
override fun clearAllExpiredAsync() {
83-
Timber.d("Clear all expired entries (asynchronous).")
83+
logcat { "Clear all expired entries (asynchronous)." }
8484
adClickData.removeAllExpired()
8585
}
8686

@@ -105,13 +105,13 @@ class DuckDuckGoAdClickManager @Inject constructor(
105105

106106
val expired = adClickData.getExemption()?.isExpired() ?: false
107107
if (expired) {
108-
Timber.d("isExemption: Url $url is EXPIRED.")
108+
logcat { "isExemption: Url $url is EXPIRED." }
109109
adClickData.removeExemption()
110110
return false
111111
}
112112

113113
if (adClickAttribution.isAllowed(url)) {
114-
Timber.d("isExemption: Url $url MATCHES the allow list")
114+
logcat { "isExemption: Url $url MATCHES the allow list" }
115115
val exemption = adClickData.getExemption()
116116
if (adClickData.getCurrentPage().isNotEmpty()) {
117117
adClickData.setCurrentPage("")

ad-click/ad-click-impl/src/main/java/com/duckduckgo/adclick/impl/pixels/AdClickDailyReportingWorker.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ import com.squareup.anvil.annotations.ContributesMultibinding
3232
import java.util.concurrent.TimeUnit
3333
import javax.inject.Inject
3434
import kotlinx.coroutines.withContext
35-
import timber.log.Timber
35+
import logcat.LogPriority.VERBOSE
36+
import logcat.logcat
3637

3738
@ContributesWorker(AppScope::class)
3839
class AdClickDailyReportingWorker(context: Context, workerParameters: WorkerParameters) :
@@ -62,7 +63,7 @@ class AdClickDailyReportingWorkerScheduler @Inject constructor(
6263

6364
override fun onCreate(owner: LifecycleOwner) {
6465
super.onCreate(owner)
65-
Timber.v("Scheduling ad click daily reporting worker")
66+
logcat(VERBOSE) { "Scheduling ad click daily reporting worker" }
6667
val workerRequest = PeriodicWorkRequestBuilder<AdClickDailyReportingWorker>(24, TimeUnit.HOURS)
6768
.addTag(DAILY_REPORTING_AD_CLICK_WORKER_TAG)
6869
.setBackoffCriteria(BackoffPolicy.LINEAR, 10, TimeUnit.MINUTES)

anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/GlobalUncaughtExceptionHandler.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import javax.inject.Inject
3030
import javax.inject.Qualifier
3131
import kotlinx.coroutines.CoroutineScope
3232
import kotlinx.coroutines.launch
33-
import logcat.LogPriority
33+
import logcat.LogPriority.ERROR
3434
import logcat.asLog
3535
import logcat.logcat
3636

@@ -82,7 +82,7 @@ class GlobalUncaughtExceptionHandler @Inject constructor(
8282
)
8383
}
8484
} catch (e: Throwable) {
85-
logcat(LogPriority.ERROR) { e.asLog() }
85+
logcat(ERROR) { e.asLog() }
8686
} finally {
8787
originalHandler.uncaughtException(thread, originalException)
8888
}

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/app/tracking/AppTPVpnConnectivityLossListener.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import javax.inject.Inject
3232
import kotlinx.coroutines.CoroutineScope
3333
import kotlinx.coroutines.launch
3434
import kotlinx.coroutines.withContext
35-
import logcat.LogPriority
35+
import logcat.LogPriority.WARN
3636
import logcat.logcat
3737

3838
@ContributesMultibinding(
@@ -73,11 +73,11 @@ class AppTPVpnConnectivityLossListener @Inject constructor(
7373
if (isActive() && connectivityLostEvents++ > 1) {
7474
connectivityLostEvents = 0
7575
if (reconnectAttempts++ > 2) {
76-
logcat(LogPriority.WARN) { "AppTP detected connectivity loss, again, give up..." }
76+
logcat(WARN) { "AppTP detected connectivity loss, again, give up..." }
7777
reconnectAttempts = 0
7878
appTrackingProtection.stop()
7979
} else {
80-
logcat(LogPriority.WARN) { "AppTP detected connectivity loss, re-configuring..." }
80+
logcat(WARN) { "AppTP detected connectivity loss, re-configuring..." }
8181
appTrackingProtection.restart()
8282
}
8383
}

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/blocklist/AppTrackerListDownloader.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import com.duckduckgo.mobile.android.vpn.trackers.AppTrackerPackage
2626
import com.duckduckgo.mobile.android.vpn.trackers.JsonAppBlockingList
2727
import com.squareup.anvil.annotations.ContributesBinding
2828
import javax.inject.Inject
29-
import logcat.LogPriority
29+
import logcat.LogPriority.WARN
30+
import logcat.asLog
3031
import logcat.logcat
3132
import okhttp3.ResponseBody.Companion.toResponseBody
3233
import retrofit2.Response
@@ -57,12 +58,12 @@ class RealAppTrackerListDownloader @Inject constructor(
5758
val response = runCatching {
5859
appTrackerListService.appTrackerBlocklist().execute()
5960
}.getOrElse {
60-
logcat(LogPriority.WARN) { "Error downloading tracker rules list: $it" }
61+
logcat(WARN) { "Error downloading tracker rules list: ${it.asLog()}" }
6162
Response.error(400, "".toResponseBody(null))
6263
}
6364

6465
if (!response.isSuccessful) {
65-
logcat(LogPriority.WARN) { "Fail to download the app tracker blocklist, error code: ${response.code()}" }
66+
logcat(WARN) { "Fail to download the app tracker blocklist, error code: ${response.code()}" }
6667
return AppTrackerBlocklist()
6768
}
6869

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/blocklist/AppTrackerListUpdateWorker.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import javax.inject.Inject
3838
import kotlinx.coroutines.sync.Mutex
3939
import kotlinx.coroutines.sync.withLock
4040
import kotlinx.coroutines.withContext
41-
import logcat.LogPriority
41+
import logcat.LogPriority.WARN
4242
import logcat.logcat
4343

4444
@ContributesWorker(AppScope::class)
@@ -78,7 +78,7 @@ class AppTrackerListUpdateWorker(context: Context, workerParameters: WorkerParam
7878

7979
val success = Result.success()
8080
if (updateBlocklistResult != success) {
81-
logcat(LogPriority.WARN) { "One of the app tracker list updates failed, scheduling a retry" }
81+
logcat(WARN) { "One of the app tracker list updates failed, scheduling a retry" }
8282
return@withContext Result.retry()
8383
}
8484

@@ -115,7 +115,7 @@ class AppTrackerListUpdateWorker(context: Context, workerParameters: WorkerParam
115115
return Result.success()
116116
}
117117
else -> {
118-
logcat(LogPriority.WARN) { "Received app tracker blocklist with invalid eTag" }
118+
logcat(WARN) { "Received app tracker blocklist with invalid eTag" }
119119
return Result.retry()
120120
}
121121
}

0 commit comments

Comments
 (0)