From 3c98a843e5f8a974a102613074575da30bdacb47 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 30 May 2024 17:25:28 +0200 Subject: [PATCH] Analytics | Add support for SuperProperties --- .../appnav/loggedin/LoggedInPresenter.kt | 13 ++ gradle/libs.versions.toml | 2 +- .../analytics/impl/DefaultAnalyticsService.kt | 5 + .../impl/DefaultAnalyticsServiceTest.kt | 23 ++++ .../api/trackers/AnalyticsTracker.kt | 7 ++ .../posthog/PosthogAnalyticsProvider.kt | 38 +++--- .../posthog/PosthogAnalyticsProviderTest.kt | 111 ++++++++++++++++++ .../test/FakeAnalyticsProvider.kt | 3 + 8 files changed, 187 insertions(+), 15 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt index 6362695772..8579a0e3e6 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt @@ -23,11 +23,13 @@ import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import im.vector.app.features.analytics.plan.CryptoSessionStateChange +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.features.networkmonitor.api.NetworkMonitor import io.element.android.features.networkmonitor.api.NetworkStatus import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.SdkMetadata import io.element.android.libraries.matrix.api.encryption.EncryptionService import io.element.android.libraries.matrix.api.encryption.RecoveryState import io.element.android.libraries.matrix.api.roomlist.RoomListService @@ -46,6 +48,7 @@ class LoggedInPresenter @Inject constructor( private val sessionVerificationService: SessionVerificationService, private val analyticsService: AnalyticsService, private val encryptionService: EncryptionService, + private val sdkMetadata: SdkMetadata, ) : Presenter { @Composable override fun present(): LoggedInState { @@ -92,6 +95,16 @@ class LoggedInPresenter @Inject constructor( reportCryptoStatusToAnalytics(verificationState, recoveryState) } + LaunchedEffect(Unit) { + analyticsService.updateSuperProperties( + SuperProperties( + cryptoSDK = SuperProperties.CryptoSDK.Rust, + appPlatform = SuperProperties.AppPlatform.EXA, + cryptoSDKVersion = sdkMetadata.sdkGitSha, + ) + ) + } + return LoggedInState( showSyncSpinner = showSyncSpinner, ) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 2617aa077e..3c922a4944 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -180,7 +180,7 @@ kotlinpoet = "com.squareup:kotlinpoet:1.17.0" posthog = "com.posthog:posthog-android:3.3.0" sentry = "io.sentry:sentry-android:7.9.0" # main branch can be tested replacing the version with main-SNAPSHOT -matrix_analytics_events = "com.github.matrix-org:matrix-analytics-events:0.21.0" +matrix_analytics_events = "com.github.matrix-org:matrix-analytics-events:0.23.0" # Emojibase matrix_emojibase_bindings = "io.element.android:emojibase-bindings:1.1.3" diff --git a/services/analytics/impl/src/main/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsService.kt b/services/analytics/impl/src/main/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsService.kt index e828d4c7ee..1b9c643dc7 100644 --- a/services/analytics/impl/src/main/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsService.kt +++ b/services/analytics/impl/src/main/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsService.kt @@ -19,6 +19,7 @@ package io.element.android.services.analytics.impl import com.squareup.anvil.annotations.ContributesBinding import im.vector.app.features.analytics.itf.VectorAnalyticsEvent import im.vector.app.features.analytics.itf.VectorAnalyticsScreen +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.libraries.di.AppScope import io.element.android.libraries.di.SingleIn @@ -148,6 +149,10 @@ class DefaultAnalyticsService @Inject constructor( } } + override fun updateSuperProperties(updatedProperties: SuperProperties) { + this.analyticsProviders.onEach { it.updateSuperProperties(updatedProperties) } + } + override fun trackError(throwable: Throwable) { if (userConsent.get()) { analyticsProviders.onEach { it.trackError(throwable) } diff --git a/services/analytics/impl/src/test/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsServiceTest.kt b/services/analytics/impl/src/test/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsServiceTest.kt index 5b685b6c16..9ab3082d27 100644 --- a/services/analytics/impl/src/test/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsServiceTest.kt +++ b/services/analytics/impl/src/test/kotlin/io/element/android/services/analytics/impl/DefaultAnalyticsServiceTest.kt @@ -23,6 +23,7 @@ import im.vector.app.features.analytics.itf.VectorAnalyticsEvent import im.vector.app.features.analytics.itf.VectorAnalyticsScreen import im.vector.app.features.analytics.plan.MobileScreen import im.vector.app.features.analytics.plan.PollEnd +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver @@ -255,6 +256,23 @@ class DefaultAnalyticsServiceTest { updateUserPropertiesLambda.assertions().isCalledOnce().with(value(aUserProperty)) } + @Test + fun `when super properties are updated, updateSuperProperties is sent to the provider`() = runCancellableScopeTest { + val updateSuperPropertiesLambda = lambdaRecorder { _ -> } + val sut = createDefaultAnalyticsService( + coroutineScope = it, + analyticsProviders = setOf( + FakeAnalyticsProvider( + initLambda = { }, + updateSuperPropertiesLambda = updateSuperPropertiesLambda, + ) + ), + analyticsStore = FakeAnalyticsStore(defaultUserConsent = true), + ) + sut.updateSuperProperties(aSuperProperty) + updateSuperPropertiesLambda.assertions().isCalledOnce().with(value(aSuperProperty)) + } + private suspend fun createDefaultAnalyticsService( coroutineScope: CoroutineScope, analyticsProviders: Set<@JvmSuppressWildcards AnalyticsProvider> = setOf( @@ -280,6 +298,11 @@ class DefaultAnalyticsServiceTest { private val aUserProperty = UserProperties( ftueUseCaseSelection = UserProperties.FtueUseCaseSelection.WorkMessaging, ) + private val aSuperProperty = SuperProperties( + appPlatform = SuperProperties.AppPlatform.EXA, + cryptoSDK = SuperProperties.CryptoSDK.Rust, + cryptoSDKVersion = "0.0" + ) private val anError = Exception("a reason") private const val AN_ID = "anId" } diff --git a/services/analyticsproviders/api/src/main/kotlin/io/element/android/services/analyticsproviders/api/trackers/AnalyticsTracker.kt b/services/analyticsproviders/api/src/main/kotlin/io/element/android/services/analyticsproviders/api/trackers/AnalyticsTracker.kt index aa88ba0cdd..e65e50bfb9 100644 --- a/services/analyticsproviders/api/src/main/kotlin/io/element/android/services/analyticsproviders/api/trackers/AnalyticsTracker.kt +++ b/services/analyticsproviders/api/src/main/kotlin/io/element/android/services/analyticsproviders/api/trackers/AnalyticsTracker.kt @@ -19,6 +19,7 @@ package io.element.android.services.analyticsproviders.api.trackers import im.vector.app.features.analytics.itf.VectorAnalyticsEvent import im.vector.app.features.analytics.itf.VectorAnalyticsScreen import im.vector.app.features.analytics.plan.Interaction +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties interface AnalyticsTracker { @@ -36,6 +37,12 @@ interface AnalyticsTracker { * Update user specific properties. */ fun updateUserProperties(userProperties: UserProperties) + + /** + * Update the super properties. + * Super properties are added to any tracked event automatically. + */ + fun updateSuperProperties(updatedProperties: SuperProperties) } fun AnalyticsTracker.captureInteraction(name: Interaction.Name, type: Interaction.InteractionType? = null) { diff --git a/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt b/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt index fb121aeb5e..6b81f0d758 100644 --- a/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt +++ b/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt @@ -20,7 +20,7 @@ import com.posthog.PostHogInterface import com.squareup.anvil.annotations.ContributesMultibinding import im.vector.app.features.analytics.itf.VectorAnalyticsEvent import im.vector.app.features.analytics.itf.VectorAnalyticsScreen -import im.vector.app.features.analytics.plan.Error +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.libraries.di.AppScope import io.element.android.services.analyticsproviders.api.AnalyticsProvider @@ -42,6 +42,8 @@ class PosthogAnalyticsProvider @Inject constructor( private var pendingUserProperties: MutableMap? = null + private var superProperties: SuperProperties? = null + private val userPropertiesLock = Any() override fun init() { @@ -64,7 +66,7 @@ class PosthogAnalyticsProvider @Inject constructor( synchronized(userPropertiesLock) { posthog?.capture( event = event.getName(), - properties = event.getProperties()?.keepOnlyNonNullValues().withExtraProperties(), + properties = event.getProperties()?.keepOnlyNonNullValues().withSuperProperties(), userProperties = pendingUserProperties, ) pendingUserProperties = null @@ -74,7 +76,7 @@ class PosthogAnalyticsProvider @Inject constructor( override fun screen(screen: VectorAnalyticsScreen) { posthog?.screen( screenTitle = screen.getName(), - properties = screen.getProperties().withExtraProperties(), + properties = screen.getProperties().withSuperProperties(), ) } @@ -94,6 +96,14 @@ class PosthogAnalyticsProvider @Inject constructor( } } + override fun updateSuperProperties(updatedProperties: SuperProperties) { + this.superProperties = SuperProperties( + cryptoSDK = updatedProperties.cryptoSDK ?: this.superProperties?.cryptoSDK, + appPlatform = updatedProperties.appPlatform ?: this.superProperties?.appPlatform, + cryptoSDKVersion = updatedProperties.cryptoSDKVersion ?: superProperties?.cryptoSDKVersion + ) + } + override fun trackError(throwable: Throwable) { // Not implemented } @@ -110,6 +120,17 @@ class PosthogAnalyticsProvider @Inject constructor( // posthog?.identify(id, lateInitUserPropertiesFactory.createUserProperties()?.getProperties()?.toPostHogUserProperties(), IGNORED_OPTIONS) } } + + private fun Map?.withSuperProperties(): Map? { + val withSuperProperties = this.orEmpty().toMutableMap() + val superProperties = this@PosthogAnalyticsProvider.superProperties?.getProperties() + superProperties?.forEach { + if (!withSuperProperties.containsKey(it.key)) { + withSuperProperties[it.key] = it.value + } + } + return withSuperProperties.takeIf { it.isEmpty().not() } + } } private fun Map.keepOnlyNonNullValues(): Map { @@ -122,14 +143,3 @@ private fun Map.keepOnlyNonNullValues(): Map { } return result } - -/** - * Properties which will be added to all Events and Screens. - */ -private val extraProperties: Map = mapOf( - "cryptoSDK" to Error.CryptoSDK.Rust -) - -private fun Map?.withExtraProperties(): Map { - return orEmpty() + extraProperties -} diff --git a/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt b/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt index 9032ea5718..28d3957d86 100644 --- a/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt +++ b/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt @@ -19,6 +19,8 @@ package io.element.android.services.analyticsproviders.posthog import com.google.common.truth.Truth.assertThat import com.posthog.PostHogInterface import im.vector.app.features.analytics.itf.VectorAnalyticsEvent +import im.vector.app.features.analytics.plan.MobileScreen +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.tests.testutils.WarmUpRule import io.mockk.every @@ -113,4 +115,113 @@ class PosthogAnalyticsProviderTest { assertThat(userPropertiesSlot.captured).isNotNull() assertThat(userPropertiesSlot.captured["verificationState"] as String).isEqualTo(testUserPropertiesUpdate.verificationState?.name) } + + @Test + fun `Posthog - Test super properties added to all captured events`() = runTest { + val mockPosthog = mockk().also { + every { it.optIn() } just runs + every { it.capture(any(), any(), any(), any(), any(), any()) } just runs + every { it.screen(any(), any()) } just runs + } + val mockPosthogFactory = mockk() + every { mockPosthogFactory.createPosthog() } returns mockPosthog + + val analyticsProvider = PosthogAnalyticsProvider(mockPosthogFactory) + analyticsProvider.init() + + val testSuperProperties = SuperProperties( + appPlatform = SuperProperties.AppPlatform.EXA, + ) + analyticsProvider.updateSuperProperties(testSuperProperties) + + // Test with events having different sort of properties + listOf( + mapOf("foo" to "bar"), + mapOf("a" to "aValue1", "b" to "aValue2"), + null + ).forEach { eventProperties -> + // report an event with properties + val mockEvent = mockk().also { + every { + it.getProperties() + } returns eventProperties + every { it.getName() } returns "MockEventName" + } + + analyticsProvider.capture(mockEvent) + + val expectedProperties = eventProperties.orEmpty() + testSuperProperties.getProperties().orEmpty() + verify { mockPosthog.capture(event = "MockEventName", any(), properties = expectedProperties, any()) } + } + + // / Test it is also added to screens + val screenEvent = MobileScreen(null, MobileScreen.ScreenName.Home) + analyticsProvider.screen(screenEvent) + verify { mockPosthog.screen(MobileScreen.ScreenName.Home.rawValue, testSuperProperties.getProperties()) } + } + + @Test + fun `Posthog - Test super properties can be updated`() = runTest { + val mockPosthog = mockk().also { + every { it.optIn() } just runs + every { it.capture(any(), any(), any(), any(), any(), any()) } just runs + } + val mockPosthogFactory = mockk() + every { mockPosthogFactory.createPosthog() } returns mockPosthog + + val analyticsProvider = PosthogAnalyticsProvider(mockPosthogFactory) + analyticsProvider.init() + + // Test with events having different sort of aggregation + // left is the updated properties, right is the expected aggregated state + mapOf( + SuperProperties(appPlatform = SuperProperties.AppPlatform.EXA) to SuperProperties(appPlatform = SuperProperties.AppPlatform.EXA), + SuperProperties(cryptoSDKVersion = "0.0") to SuperProperties(appPlatform = SuperProperties.AppPlatform.EXA, cryptoSDKVersion = "0.0"), + SuperProperties(cryptoSDKVersion = "1.0") to SuperProperties(appPlatform = SuperProperties.AppPlatform.EXA, cryptoSDKVersion = "1.0"), + SuperProperties(cryptoSDK = SuperProperties.CryptoSDK.Rust) to SuperProperties( + appPlatform = SuperProperties.AppPlatform.EXA, + cryptoSDKVersion = "1.0", + cryptoSDK = SuperProperties.CryptoSDK.Rust + ), + ).entries.forEach { (updated, expected) -> + // report an event with properties + val mockEvent = mockk().also { + every { + it.getProperties() + } returns null + every { it.getName() } returns "MockEventName" + } + + analyticsProvider.updateSuperProperties(updated) + analyticsProvider.capture(mockEvent) + + verify { mockPosthog.capture(event = "MockEventName", any(), properties = expected.getProperties(), any()) } + } + } + + @Test + fun `Posthog - Test super properties do not override property with same name on the event`() = runTest { + val mockPosthog = mockk().also { + every { it.optIn() } just runs + every { it.capture(any(), any(), any(), any(), any(), any()) } just runs + } + val mockPosthogFactory = mockk() + every { mockPosthogFactory.createPosthog() } returns mockPosthog + + val analyticsProvider = PosthogAnalyticsProvider(mockPosthogFactory) + analyticsProvider.init() + + // report an event with properties + val mockEvent = mockk().also { + every { + it.getProperties() + } returns mapOf("appPlatform" to SuperProperties.AppPlatform.Other) + every { it.getName() } returns "MockEventName" + } + + analyticsProvider.updateSuperProperties(SuperProperties(appPlatform = SuperProperties.AppPlatform.EXA)) + analyticsProvider.capture(mockEvent) + + verify { mockPosthog.capture(event = "MockEventName", any(), properties = mapOf("appPlatform" to SuperProperties.AppPlatform.Other), any()) } + } } diff --git a/services/analyticsproviders/test/src/main/kotlin/io/element/android/services/analyticsproviders/test/FakeAnalyticsProvider.kt b/services/analyticsproviders/test/src/main/kotlin/io/element/android/services/analyticsproviders/test/FakeAnalyticsProvider.kt index fcbfc4112f..0d4d15991c 100644 --- a/services/analyticsproviders/test/src/main/kotlin/io/element/android/services/analyticsproviders/test/FakeAnalyticsProvider.kt +++ b/services/analyticsproviders/test/src/main/kotlin/io/element/android/services/analyticsproviders/test/FakeAnalyticsProvider.kt @@ -18,6 +18,7 @@ package io.element.android.services.analyticsproviders.test import im.vector.app.features.analytics.itf.VectorAnalyticsEvent import im.vector.app.features.analytics.itf.VectorAnalyticsScreen +import im.vector.app.features.analytics.plan.SuperProperties import im.vector.app.features.analytics.plan.UserProperties import io.element.android.services.analyticsproviders.api.AnalyticsProvider import io.element.android.tests.testutils.lambda.lambdaError @@ -29,6 +30,7 @@ class FakeAnalyticsProvider( private val captureLambda: (VectorAnalyticsEvent) -> Unit = { lambdaError() }, private val screenLambda: (VectorAnalyticsScreen) -> Unit = { lambdaError() }, private val updateUserPropertiesLambda: (UserProperties) -> Unit = { lambdaError() }, + private val updateSuperPropertiesLambda: (SuperProperties) -> Unit = { lambdaError() }, private val trackErrorLambda: (Throwable) -> Unit = { lambdaError() } ) : AnalyticsProvider { override fun init() = initLambda() @@ -37,4 +39,5 @@ class FakeAnalyticsProvider( override fun screen(screen: VectorAnalyticsScreen) = screenLambda(screen) override fun updateUserProperties(userProperties: UserProperties) = updateUserPropertiesLambda(userProperties) override fun trackError(throwable: Throwable) = trackErrorLambda(throwable) + override fun updateSuperProperties(updatedProperties: SuperProperties) = updateSuperPropertiesLambda(updatedProperties) }