From 9c2c9a9c7a83b7f929a73743d858fbe51ae463c4 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 11 Dec 2024 16:40:46 +0100 Subject: [PATCH 1/4] feat: disable countly view tracking --- .../AnonymousAnalyticsManagerImpl.kt | 11 +++++++ .../AnonymousAnalyticsManagerTest.kt | 33 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt index 7e78362974f..0eba82a61ae 100644 --- a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt @@ -42,6 +42,9 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { private val mutex = Mutex() private lateinit var coroutineScope: CoroutineScope + // TODO: Sync with product, when we want to enable view tracking, var for testing purposes + internal var VIEW_TRACKING_ENABLED: Boolean = false + override fun init( context: Context, analyticsSettings: AnalyticsSettings, @@ -172,6 +175,10 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { } override fun recordView(screen: String) { + if (!VIEW_TRACKING_ENABLED) { + Log.d(TAG, "View tracking is disabled for this build.") + return + } coroutineScope.launch { mutex.withLock { if (!isAnonymousUsageDataEnabled) return@withLock @@ -181,6 +188,10 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { } override fun stopView(screen: String) { + if (!VIEW_TRACKING_ENABLED) { + Log.d(TAG, "View tracking is disabled for this build.") + return + } coroutineScope.launch { mutex.withLock { if (!isAnonymousUsageDataEnabled) return@withLock diff --git a/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerTest.kt b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerTest.kt index ec45685727f..691810ab5a3 100644 --- a/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerTest.kt +++ b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerTest.kt @@ -294,6 +294,37 @@ class AnonymousAnalyticsManagerTest { } } + @Test + fun givenManagerInitialized_whenRecordingViewAndFlagDisabled_thenScreenIsNOTRecorded() = runTest(dispatcher) { + // given + val (arrangement, manager) = Arrangement() + .withAnonymousAnalyticsRecorderConfigure() + .arrange(shouldTrackViews = false) + + val screen = "screen" + arrangement.withAnalyticsResult(Arrangement.existingIdentifierResult) + + // when + manager.init( + context = arrangement.context, + analyticsSettings = Arrangement.analyticsSettings, + analyticsResultFlow = arrangement.analyticsResultChannel.consumeAsFlow(), + anonymousAnalyticsRecorder = arrangement.anonymousAnalyticsRecorder, + migrationHandler = arrangement.migrationHandler, + propagationHandler = arrangement.propagationHandler, + dispatcher = dispatcher + ) + advanceUntilIdle() + + manager.recordView(screen) + advanceUntilIdle() + + // then + verify(exactly = 0) { + arrangement.anonymousAnalyticsRecorder.recordView(eq(screen)) + } + } + @Test fun givenManagerInitialized_whenStoppingView_thenScreenIsStoppedToRecord() = runTest(dispatcher) { // given @@ -387,7 +418,7 @@ class AnonymousAnalyticsManagerTest { AnonymousAnalyticsManagerImpl } - fun arrange() = this to manager + fun arrange(shouldTrackViews: Boolean = true) = this to manager.apply { VIEW_TRACKING_ENABLED = shouldTrackViews } fun withAnonymousAnalyticsRecorderConfigure() = apply { every { anonymousAnalyticsRecorder.configure(any(), any()) } returns Unit From 3bed9e1ac14e59b7e236c4d82cf4fe0dbbd493c4 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 11 Dec 2024 17:29:53 +0100 Subject: [PATCH 2/4] fix: use removeconsenAll instead of halt --- .../android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt index bee5ab38855..0bb72c5545d 100644 --- a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt @@ -85,7 +85,7 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { override fun halt() { isConfigured = false - Countly.sharedInstance().halt() + Countly.sharedInstance().consent().removeConsentAll() } override suspend fun setTrackingIdentifierWithMerge( From 7bd5560d2ab0a5509581c7b5320829ca8363723c Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 12 Dec 2024 02:37:52 +0100 Subject: [PATCH 3/4] fix: harden countly sdk integration --- .../AnonymousAnalyticsRecorderImpl.kt | 68 ++++++++++++------- kalium | 2 +- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt index 0bb72c5545d..6baadc5b295 100644 --- a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsRecorderImpl.kt @@ -20,6 +20,7 @@ package com.wire.android.feature.analytics import android.app.Activity import android.app.Application import android.content.Context +import android.util.Log import com.wire.android.feature.analytics.model.AnalyticsEvent import com.wire.android.feature.analytics.model.AnalyticsEventConstants import com.wire.android.feature.analytics.model.AnalyticsSettings @@ -34,8 +35,8 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { override fun configure( context: Context, analyticsSettings: AnalyticsSettings - ) { - if (isConfigured) return + ) = wrapCountlyRequest { + if (isConfigured) return@wrapCountlyRequest val countlyConfig = CountlyConfig( context, @@ -54,24 +55,24 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { } } - Countly.sharedInstance().init(countlyConfig) - Countly.sharedInstance().consent().giveConsent(arrayOf("apm")) + Countly.sharedInstance()?.init(countlyConfig) + Countly.sharedInstance()?.consent()?.giveConsent(arrayOf("apm")) val packageInfo = context.packageManager.getPackageInfo(context.packageName, 0) val globalSegmentations = mapOf( AnalyticsEventConstants.APP_NAME to AnalyticsEventConstants.APP_NAME_ANDROID, AnalyticsEventConstants.APP_VERSION to packageInfo.versionName ) - Countly.sharedInstance().views().setGlobalViewSegmentation(globalSegmentations) + Countly.sharedInstance()?.views()?.setGlobalViewSegmentation(globalSegmentations) isConfigured = true } - override fun onStart(activity: Activity) { - Countly.sharedInstance().onStart(activity) + override fun onStart(activity: Activity) = wrapCountlyRequest { + Countly.sharedInstance()?.onStart(activity) } - override fun onStop() { - Countly.sharedInstance().onStop() + override fun onStop() = wrapCountlyRequest { + Countly.sharedInstance()?.onStop() } /** @@ -79,13 +80,13 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { * Countly is doing additional operations on it. * See [UtilsInternalLimits.removeUnsupportedDataTypes] */ - override fun sendEvent(event: AnalyticsEvent) { - Countly.sharedInstance().events().recordEvent(event.key, event.toSegmentation().toMutableMap()) + override fun sendEvent(event: AnalyticsEvent) = wrapCountlyRequest { + Countly.sharedInstance()?.events()?.recordEvent(event.key, event.toSegmentation().toMutableMap()) } - override fun halt() { + override fun halt() = wrapCountlyRequest { isConfigured = false - Countly.sharedInstance().consent().removeConsentAll() + Countly.sharedInstance()?.consent()?.removeConsentAll() } override suspend fun setTrackingIdentifierWithMerge( @@ -93,7 +94,9 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { isTeamMember: Boolean, migrationComplete: suspend () -> Unit ) { - Countly.sharedInstance().deviceId().changeWithMerge(identifier).also { + wrapCountlyRequest { + Countly.sharedInstance()?.deviceId()?.changeWithMerge(identifier) + }.also { migrationComplete() } @@ -106,7 +109,9 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { isTeamMember: Boolean, propagateIdentifier: suspend () -> Unit ) { - Countly.sharedInstance().deviceId().changeWithoutMerge(identifier) + wrapCountlyRequest { + Countly.sharedInstance()?.deviceId()?.changeWithoutMerge(identifier) + } setUserProfileProperties(isTeamMember = isTeamMember) @@ -115,27 +120,42 @@ class AnonymousAnalyticsRecorderImpl : AnonymousAnalyticsRecorder { } } - private fun setUserProfileProperties(isTeamMember: Boolean) { - Countly.sharedInstance().userProfile().setProperty( + private fun setUserProfileProperties(isTeamMember: Boolean) = wrapCountlyRequest { + Countly.sharedInstance()?.userProfile()?.setProperty( AnalyticsEventConstants.TEAM_IS_TEAM, isTeamMember ) - Countly.sharedInstance().userProfile().save() + Countly.sharedInstance()?.userProfile()?.save() } override fun isAnalyticsInitialized(): Boolean = Countly.sharedInstance().isInitialized - override fun applicationOnCreate() { - if (isConfigured) return + override fun applicationOnCreate() = wrapCountlyRequest { + if (isConfigured) return@wrapCountlyRequest Countly.applicationOnCreate() } - override fun recordView(screen: String) { - Countly.sharedInstance().views().startAutoStoppedView(screen) + override fun recordView(screen: String) = wrapCountlyRequest { + Countly.sharedInstance()?.views()?.startAutoStoppedView(screen) + } + + override fun stopView(screen: String) = wrapCountlyRequest { + Countly.sharedInstance()?.views()?.stopViewWithName(screen) + } + + @Suppress("TooGenericExceptionCaught") + private fun wrapCountlyRequest(block: () -> Unit) { + try { + block() + } catch (e: Exception) { + // Countly SDK throws exceptions on some cases, just log it + // We don't want to crash the app because of that. + Log.wtf(TAG, "Countly SDK request failed", e) + } } - override fun stopView(screen: String) { - Countly.sharedInstance().views().stopViewWithName(screen) + companion object { + private const val TAG = "AnonymousAnalyticsRecorderImpl" } } diff --git a/kalium b/kalium index 1fd873293a4..7fd041a8f14 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 1fd873293a418acc8360798aed324f065c487b6e +Subproject commit 7fd041a8f1436509611446d3f30447b761142893 From 381b34a59cf52dba354613d0084a5bce4592ca4f Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 12 Dec 2024 08:35:36 +0100 Subject: [PATCH 4/4] chore: update kalium ref --- kalium | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kalium b/kalium index 1fd873293a4..c9f091b3923 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 1fd873293a418acc8360798aed324f065c487b6e +Subproject commit c9f091b392378afa02a1ca0df92d41052b0439a1