Merge pull request #8839 from element-hq/feature/bca/add_platformcode…

…_to_posthog Support reporting super properties to posthog (appPlatform)
element-hq · May 31, 2024 · 47bb23a · 47bb23a
2 parents 72575a2 + 7e41d73
commit 47bb23a
Show file tree

Hide file tree

Showing 12 changed files with 169 additions and 29 deletions.
diff --git a/changelog.d/8839.misc b/changelog.d/8839.misc
@@ -0,0 +1 @@
+Posthog | report platform code for EA
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/Matrix.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/Matrix.kt
@@ -147,5 +147,12 @@ class Matrix(context: Context, matrixConfiguration: MatrixConfiguration) {
         fun getSdkVersion(): String {
             return BuildConfig.SDK_VERSION + " (" + BuildConfig.GIT_SDK_REVISION + ")"
         }
+
+        fun getCryptoVersion(longFormat: Boolean): String {
+            val version = org.matrix.rustcomponents.sdk.crypto.version()
+            val gitHash = org.matrix.rustcomponents.sdk.crypto.versionInfo().gitSha
+            val vodozemac = org.matrix.rustcomponents.sdk.crypto.vodozemacVersion()
+            return if (longFormat) "Rust SDK $version ($gitHash), Vodozemac $vodozemac" else version
+        }
     }
 }
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt
@@ -16,7 +16,6 @@
 
 package org.matrix.android.sdk.api.session.crypto
 
-import android.content.Context
 import androidx.annotation.Size
 import androidx.lifecycle.LiveData
 import androidx.paging.PagedList
@@ -61,8 +60,6 @@ interface CryptoService {
 
     suspend fun deleteDevices(@Size(min = 1) deviceIds: List<String>, userInteractiveAuthInterceptor: UserInteractiveAuthInterceptor)
 
-    fun getCryptoVersion(context: Context, longFormat: Boolean): String
-
     fun isCryptoEnabled(): Boolean
 
     fun isRoomBlacklistUnverifiedDevices(roomId: String?): Boolean

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt
@@ -16,7 +16,6 @@
 
 package org.matrix.android.sdk.internal.crypto
 
-import android.content.Context
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.map
 import androidx.paging.PagedList
@@ -184,13 +183,6 @@ internal class RustCryptoService @Inject constructor(
         deleteDevices(listOf(deviceId), userInteractiveAuthInterceptor)
     }
 
-    override fun getCryptoVersion(context: Context, longFormat: Boolean): String {
-        val version = org.matrix.rustcomponents.sdk.crypto.version()
-        val gitHash = org.matrix.rustcomponents.sdk.crypto.versionInfo().gitSha
-        val vodozemac = org.matrix.rustcomponents.sdk.crypto.vodozemacVersion()
-        return if (longFormat) "Rust SDK $version ($gitHash), Vodozemac $vodozemac" else version
-    }
-
     override suspend fun getMyCryptoDevice(): CryptoDeviceInfo = withContext(coroutineDispatchers.io) {
         olmMachine.ownDevice()
     }

diff --git a/vector-app/src/main/java/im/vector/app/VectorApplication.kt b/vector-app/src/main/java/im/vector/app/VectorApplication.kt
@@ -53,6 +53,7 @@ import im.vector.app.core.pushers.FcmHelper
 import im.vector.app.core.resources.BuildMeta
 import im.vector.app.features.analytics.DecryptionFailureTracker
 import im.vector.app.features.analytics.VectorAnalytics
+import im.vector.app.features.analytics.plan.SuperProperties
 import im.vector.app.features.call.webrtc.WebRtcCallManager
 import im.vector.app.features.configuration.VectorConfiguration
 import im.vector.app.features.invite.InvitesAcceptor
@@ -130,6 +131,13 @@ class VectorApplication :
         appContext = this
         flipperProxy.init(matrix)
         vectorAnalytics.init()
+        vectorAnalytics.updateSuperProperties(
+                SuperProperties(
+                        appPlatform = SuperProperties.AppPlatform.EA,
+                        cryptoSDK = SuperProperties.CryptoSDK.Rust,
+                        cryptoSDKVersion = Matrix.getCryptoVersion(longFormat = false)
+                )
+        )
         invitesAcceptor.initialize()
         autoRageShaker.initialize()
         decryptionFailureTracker.start()

diff --git a/vector/build.gradle b/vector/build.gradle
@@ -160,7 +160,7 @@ dependencies {
     api 'com.facebook.stetho:stetho:1.6.0'
 
     // Analytics
-    api 'com.github.matrix-org:matrix-analytics-events:0.15.0'
+    api 'com.github.matrix-org:matrix-analytics-events:0.23.0'
 
     api libs.google.phonenumber
 

diff --git a/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt b/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt
@@ -22,7 +22,6 @@ import im.vector.app.core.dispatchers.CoroutineDispatchers
 import im.vector.app.core.pushers.UnregisterUnifiedPushUseCase
 import im.vector.app.core.services.GuardServiceStarter
 import im.vector.app.core.session.ConfigureAndStartSessionUseCase
-import im.vector.app.features.analytics.DecryptionFailureTracker
 import im.vector.app.features.call.webrtc.WebRtcCallManager
 import im.vector.app.features.crypto.keysrequest.KeyRequestHandler
 import im.vector.app.features.crypto.verification.IncomingVerificationRequestHandler
@@ -57,7 +56,6 @@ class ActiveSessionHolder @Inject constructor(
         private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase,
         private val applicationCoroutineScope: CoroutineScope,
         private val coroutineDispatchers: CoroutineDispatchers,
-        private val decryptionFailureTracker: DecryptionFailureTracker,
 ) {
 
     private var activeSessionReference: AtomicReference<Session?> = AtomicReference()

diff --git a/vector/src/main/java/im/vector/app/features/analytics/AnalyticsTracker.kt b/vector/src/main/java/im/vector/app/features/analytics/AnalyticsTracker.kt
@@ -18,6 +18,7 @@ package im.vector.app.features.analytics
 
 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
 
 interface AnalyticsTracker {
@@ -35,4 +36,10 @@ 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)
 }
diff --git a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt
@@ -23,6 +23,7 @@ import im.vector.app.features.analytics.VectorAnalytics
 import im.vector.app.features.analytics.itf.VectorAnalyticsEvent
 import im.vector.app.features.analytics.itf.VectorAnalyticsScreen
 import im.vector.app.features.analytics.log.analyticsTag
+import im.vector.app.features.analytics.plan.SuperProperties
 import im.vector.app.features.analytics.plan.UserProperties
 import im.vector.app.features.analytics.store.AnalyticsStore
 import kotlinx.coroutines.CoroutineScope
@@ -63,6 +64,8 @@ class DefaultVectorAnalytics @Inject constructor(
     // Cache for the properties to send
     private var pendingUserProperties: UserProperties? = null
 
+    private var superProperties: SuperProperties? = null
+
     override fun init() {
         observeUserConsent()
         observeAnalyticsId()
@@ -168,20 +171,14 @@ class DefaultVectorAnalytics @Inject constructor(
 
     override fun capture(event: VectorAnalyticsEvent) {
         Timber.tag(analyticsTag.value).d("capture($event)")
-        posthog
-                ?.takeIf { userConsent == true }
-                ?.capture(
-                        event.getName(),
-                        analyticsId,
-                        event.getProperties()?.toPostHogProperties()
+        posthog?.takeIf { userConsent == true }?.capture(
+                        event.getName(), analyticsId, event.getProperties()?.toPostHogProperties().orEmpty().withSuperProperties()
                 )
     }
 
     override fun screen(screen: VectorAnalyticsScreen) {
         Timber.tag(analyticsTag.value).d("screen($screen)")
-        posthog
-                ?.takeIf { userConsent == true }
-                ?.screen(screen.getName(), screen.getProperties()?.toPostHogProperties())
+        posthog?.takeIf { userConsent == true }?.screen(screen.getName(), screen.getProperties()?.toPostHogProperties().orEmpty().withSuperProperties())
     }
 
     override fun updateUserProperties(userProperties: UserProperties) {
@@ -195,9 +192,7 @@ class DefaultVectorAnalytics @Inject constructor(
     private fun doUpdateUserProperties(userProperties: UserProperties) {
         // we need a distinct id to set user properties
         val distinctId = analyticsId ?: return
-        posthog
-                ?.takeIf { userConsent == true }
-                ?.identify(distinctId, userProperties.getProperties())
+        posthog?.takeIf { userConsent == true }?.identify(distinctId, userProperties.getProperties())
     }
 
     private fun Map<String, Any?>?.toPostHogProperties(): Map<String, Any>? {
@@ -226,9 +221,32 @@ class DefaultVectorAnalytics @Inject constructor(
         return nonNulls
     }
 
+    /**
+     * Adds super properties to the actual property set.
+     * If a property of the same name is already on the reported event it will not be overwritten.
+     */
+    private fun Map<String, Any>.withSuperProperties(): Map<String, Any>? {
+        val withSuperProperties = this.toMutableMap()
+        val superProperties = this@DefaultVectorAnalytics.superProperties?.getProperties()
+        superProperties?.forEach {
+            if (!withSuperProperties.containsKey(it.key)) {
+                withSuperProperties[it.key] = it.value
+            }
+        }
+        return withSuperProperties.takeIf { it.isEmpty().not() }
+    }
+
     override fun trackError(throwable: Throwable) {
         sentryAnalytics
                 .takeIf { userConsent == true }
                 ?.trackError(throwable)
     }
+
+    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
+        )
+    }
 }
diff --git a/vector/src/main/java/im/vector/app/features/rageshake/BugReporter.kt b/vector/src/main/java/im/vector/app/features/rageshake/BugReporter.kt
@@ -265,7 +265,7 @@ class BugReporter @Inject constructor(
                 activeSessionHolder.getSafeActiveSession()?.let { session ->
                     userId = session.myUserId
                     deviceId = session.sessionParams.deviceId
-                    olmVersion = session.cryptoService().getCryptoVersion(context, true)
+                    olmVersion = Matrix.getCryptoVersion(true)
                 }
 
                 if (!mIsCancelled) {

diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsHelpAboutFragment.kt b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsHelpAboutFragment.kt
@@ -96,7 +96,7 @@ class VectorSettingsHelpAboutFragment :
 
         // olm version
         findPreference<VectorPreference>(VectorPreferences.SETTINGS_CRYPTO_VERSION_PREFERENCE_KEY)!!
-                .summary = session.cryptoService().getCryptoVersion(requireContext(), true)
+                .summary = Matrix.getCryptoVersion(true)
     }
 
     companion object {

diff --git a/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt b/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt
@@ -16,6 +16,7 @@
 
 package im.vector.app.features.analytics.impl
 
+import im.vector.app.features.analytics.plan.SuperProperties
 import im.vector.app.test.fakes.FakeAnalyticsStore
 import im.vector.app.test.fakes.FakeLateInitUserPropertiesFactory
 import im.vector.app.test.fakes.FakePostHog
@@ -51,7 +52,7 @@ class DefaultVectorAnalyticsTest {
             analyticsStore = fakeAnalyticsStore.instance,
             globalScope = CoroutineScope(Dispatchers.Unconfined),
             analyticsConfig = anAnalyticsConfig(isEnabled = true),
-            lateInitUserPropertiesFactory = fakeLateInitUserPropertiesFactory.instance
+            lateInitUserPropertiesFactory = fakeLateInitUserPropertiesFactory.instance,
     )
 
     @Before
@@ -174,6 +175,117 @@ class DefaultVectorAnalyticsTest {
         fakeSentryAnalytics.verifyNoErrorTracking()
     }
 
+    @Test
+    fun `Super properties should be added to all captured events`() = runTest {
+        fakeAnalyticsStore.givenUserContent(consent = true)
+
+        val updatedProperties = SuperProperties(
+                appPlatform = SuperProperties.AppPlatform.EA,
+                cryptoSDKVersion = "0.0",
+                cryptoSDK = SuperProperties.CryptoSDK.Rust
+        )
+
+        defaultVectorAnalytics.updateSuperProperties(updatedProperties)
+
+        val fakeEvent = aVectorAnalyticsEvent("THE_NAME", mutableMapOf("foo" to "bar"))
+        defaultVectorAnalytics.capture(fakeEvent)
+
+        fakePostHog.verifyEventTracked(
+                "THE_NAME",
+                fakeEvent.getProperties().clearNulls()?.toMutableMap()?.apply {
+                    updatedProperties.getProperties()?.let { putAll(it) }
+                }
+        )
+
+        // Check with a screen event
+        val fakeScreen = aVectorAnalyticsScreen("Screen", mutableMapOf("foo" to "bar"))
+        defaultVectorAnalytics.screen(fakeScreen)
+
+        fakePostHog.verifyScreenTracked(
+                "Screen",
+                fakeScreen.getProperties().clearNulls()?.toMutableMap()?.apply {
+                    updatedProperties.getProperties()?.let { putAll(it) }
+                }
+        )
+    }
+
+    @Test
+    fun `Super properties can be updated`() = runTest {
+        fakeAnalyticsStore.givenUserContent(consent = true)
+
+        val superProperties = SuperProperties(
+                appPlatform = SuperProperties.AppPlatform.EA,
+                cryptoSDKVersion = "0.0",
+                cryptoSDK = SuperProperties.CryptoSDK.Rust
+        )
+
+        defaultVectorAnalytics.updateSuperProperties(superProperties)
+
+        val fakeEvent = aVectorAnalyticsEvent("THE_NAME", mutableMapOf("foo" to "bar"))
+        defaultVectorAnalytics.capture(fakeEvent)
+
+        fakePostHog.verifyEventTracked(
+                "THE_NAME",
+                fakeEvent.getProperties().clearNulls()?.toMutableMap()?.apply {
+                    superProperties.getProperties()?.let { putAll(it) }
+                }
+        )
+
+        val superPropertiesUpdate = superProperties.copy(cryptoSDKVersion = "1.0")
+        defaultVectorAnalytics.updateSuperProperties(superPropertiesUpdate)
+
+        defaultVectorAnalytics.capture(fakeEvent)
+
+        fakePostHog.verifyEventTracked(
+                "THE_NAME",
+                fakeEvent.getProperties().clearNulls()?.toMutableMap()?.apply {
+                    superPropertiesUpdate.getProperties()?.let { putAll(it) }
+                }
+        )
+    }
+
+    @Test
+    fun `Super properties should not override event property`() = runTest {
+        fakeAnalyticsStore.givenUserContent(consent = true)
+
+        val superProperties = SuperProperties(
+                cryptoSDKVersion = "0.0",
+        )
+
+        defaultVectorAnalytics.updateSuperProperties(superProperties)
+
+        val fakeEvent = aVectorAnalyticsEvent("THE_NAME", mutableMapOf("cryptoSDKVersion" to "XXX"))
+        defaultVectorAnalytics.capture(fakeEvent)
+
+        fakePostHog.verifyEventTracked(
+                "THE_NAME",
+                mapOf(
+                        "cryptoSDKVersion" to "XXX"
+                )
+        )
+    }
+
+    @Test
+    fun `Super properties should be added to event with no properties`() = runTest {
+        fakeAnalyticsStore.givenUserContent(consent = true)
+
+        val superProperties = SuperProperties(
+                cryptoSDKVersion = "0.0",
+        )
+
+        defaultVectorAnalytics.updateSuperProperties(superProperties)
+
+        val fakeEvent = aVectorAnalyticsEvent("THE_NAME", null)
+        defaultVectorAnalytics.capture(fakeEvent)
+
+        fakePostHog.verifyEventTracked(
+                "THE_NAME",
+                mapOf(
+                        "cryptoSDKVersion" to "0.0"
+                )
+        )
+    }
+
     private fun Map<String, Any?>?.clearNulls(): Map<String, Any>? {
         if (this == null) return null