Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick improvement for analytics reporting #8773

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ import im.vector.app.features.analytics.plan.UserProperties
interface AnalyticsTracker {
/**
* Capture an Event.
*
* @param event The event to capture.
* @param extraProperties Some extra properties to attach to the event, that are not part of the events definition
* (https://github.com/matrix-org/matrix-analytics-events/) and specific to this platform.
*/
fun capture(event: VectorAnalyticsEvent)
fun capture(event: VectorAnalyticsEvent, extraProperties: Map<String, String>? = null)

/**
* Track a displayed screen.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.crypto.MXCryptoError
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import javax.inject.Inject
Expand All @@ -36,7 +37,9 @@ private data class DecryptionFailure(
val timeStamp: Long,
val roomId: String,
val failedEventId: String,
val error: MXCryptoError.ErrorType
val error: MXCryptoError.ErrorType,
// Was the current session cross signed verified at the time of the error
val isCrossSignedVerified: Boolean = false,
)
private typealias DetailedErrorName = Pair<String, Error.Name>

Expand Down Expand Up @@ -75,11 +78,14 @@ class DecryptionFailureTracker @Inject constructor(
scope.cancel()
}

fun e2eEventDisplayedInTimeline(event: TimelineEvent) {
fun e2eEventDisplayedInTimeline(event: TimelineEvent, session: Session) {
scope.launch(Dispatchers.Default) {
val mCryptoError = event.root.mCryptoError
if (mCryptoError != null) {
addDecryptionFailure(DecryptionFailure(clock.epochMillis(), event.roomId, event.eventId, mCryptoError))
val isVerified = session.cryptoService().crossSigningService().isCrossSigningVerified()
addDecryptionFailure(
DecryptionFailure(clock.epochMillis(), event.roomId, event.eventId, mCryptoError, isVerified)
)
} else {
removeFailureForEventId(event.eventId)
}
Expand Down Expand Up @@ -115,7 +121,7 @@ class DecryptionFailureTracker @Inject constructor(

private fun checkFailures() {
val now = clock.epochMillis()
val aggregatedErrors: Map<DetailedErrorName, List<String>>
val aggregatedErrors: Map<DetailedErrorName, List<DecryptionFailure>>
synchronized(failures) {
val toReport = mutableListOf<DecryptionFailure>()
failures.removeAll { failure ->
Expand All @@ -129,23 +135,26 @@ class DecryptionFailureTracker @Inject constructor(
aggregatedErrors = toReport
.groupBy { it.error.toAnalyticsErrorName() }
.mapValues {
it.value.map { it.failedEventId }
it.value
}
}

aggregatedErrors.forEach { aggregation ->
// there is now way to send the total/sum in posthog, so iterating
aggregation.value
// for now we ignore events already reported even if displayed again?
.filter { alreadyReported.contains(it).not() }
.forEach { failedEventId ->
analyticsTracker.capture(Error(
context = aggregation.key.first,
domain = Error.Domain.E2EE,
name = aggregation.key.second,
cryptoModule = currentModule
))
alreadyReported.add(failedEventId)
.filter { alreadyReported.contains(it.failedEventId).not() }
.forEach { failure ->
analyticsTracker.capture(
event = Error(
context = aggregation.key.first,
domain = Error.Domain.E2EE,
name = aggregation.key.second,
cryptoModule = currentModule,
),
extraProperties = mapOf("is_cross_signed_verified" to failure.isCrossSignedVerified.toString())
)
alreadyReported.add(failure.failedEventId)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.posthog.android.Options
import com.posthog.android.PostHog
import com.posthog.android.Properties
import im.vector.app.core.di.NamedGlobalScope
import im.vector.app.core.resources.BuildMeta
import im.vector.app.features.analytics.AnalyticsConfig
import im.vector.app.features.analytics.VectorAnalytics
import im.vector.app.features.analytics.itf.VectorAnalyticsEvent
Expand All @@ -46,7 +47,8 @@ class DefaultVectorAnalytics @Inject constructor(
private val analyticsConfig: AnalyticsConfig,
private val analyticsStore: AnalyticsStore,
private val lateInitUserPropertiesFactory: LateInitUserPropertiesFactory,
@NamedGlobalScope private val globalScope: CoroutineScope
@NamedGlobalScope private val globalScope: CoroutineScope,
buildMeta: BuildMeta,
) : VectorAnalytics {

private var posthog: PostHog? = null
Expand All @@ -68,6 +70,20 @@ class DefaultVectorAnalytics @Inject constructor(
// Cache for the properties to send
private var pendingUserProperties: UserProperties? = null

/**
* Super Properties are properties associated with events that are set once and then sent with every capture call.
*/
private val superProperties: Map<String, String> = mapOf(
// Put the appVersion (e.g 1.6.12).
"appVersion" to buildMeta.versionName,
// The appId (im.vector.app)
"applicationId" to buildMeta.applicationId,
// The app flavor (GooglePlay, FDroid)
"appFlavor" to buildMeta.flavorDescription,
// Parity with other platforms
"cryptoSDK" to "Rust",
)

override fun init() {
observeUserConsent()
observeAnalyticsId()
Expand Down Expand Up @@ -171,18 +187,24 @@ class DefaultVectorAnalytics @Inject constructor(
}
}

override fun capture(event: VectorAnalyticsEvent) {
override fun capture(event: VectorAnalyticsEvent, extraProperties: Map<String, String>?) {
Timber.tag(analyticsTag.value).d("capture($event)")
posthog
?.takeIf { userConsent == true }
?.capture(event.getName(), event.getProperties()?.toPostHogProperties())
?.capture(
event.getName(),
(superProperties + event.getProperties().orEmpty() + extraProperties.orEmpty()).toPostHogProperties()
)
}

override fun screen(screen: VectorAnalyticsScreen) {
Timber.tag(analyticsTag.value).d("screen($screen)")
posthog
?.takeIf { userConsent == true }
?.screen(screen.getName(), screen.getProperties()?.toPostHogProperties())
?.screen(
screen.getName(),
(superProperties + screen.getProperties().orEmpty()).toPostHogProperties()
)
}

override fun updateUserProperties(userProperties: UserProperties) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class TimelineItemFactory @Inject constructor(
}
}.also {
if (it != null && event.isEncrypted()) {
decryptionFailureTracker.e2eEventDisplayedInTimeline(event)
decryptionFailureTracker.e2eEventDisplayedInTimeline(event, session)
}
}
}
Expand Down
Loading