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

Sync: Usage Pixels and Monitoring #4018

Merged
merged 1 commit into from
Dec 22, 2023
Merged
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
2 changes: 1 addition & 1 deletion app/src/internal/res/xml/network_security_config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@
<certificates src="user" />
</trust-anchors>
</base-config>
</network-security-config>
</network-security-config>
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.duckduckgo.sync.api.engine.SyncEngine.SyncTrigger.ACCOUNT_CREATION
import com.duckduckgo.sync.api.engine.SyncEngine.SyncTrigger.ACCOUNT_LOGIN
import com.duckduckgo.sync.crypto.*
import com.duckduckgo.sync.impl.Result.Error
import com.duckduckgo.sync.impl.Result.Success
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few unused imports in this file

import com.duckduckgo.sync.impl.pixels.*
import com.duckduckgo.sync.store.*
import com.duckduckgo.sync.store.SyncStore
Expand Down Expand Up @@ -198,6 +199,7 @@ class AppSyncAccountRepository @Inject constructor(
if (result is Error) {
syncPixels.fireSyncAccountErrorPixel(result)
}

return result
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ class RealSyncEngine @Inject constructor(
Timber.d("Sync-Engine: sync is not in progress, starting to sync")
syncStateRepository.store(SyncAttempt(state = IN_PROGRESS, meta = trigger.toString()))

syncPixels.fireDailyPixel()

Timber.i("Sync-Engine: getChanges - performSync")
val changes = getChanges()
performFirstSync(changes.filter { it.isFirstSync() })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class SyncDailyReportingWorker(
override suspend fun doWork(): Result {
return withContext(dispatchers.io()) {
if (syncAccountRepository.isSignedIn()) {
syncPixels.fireStatsPixel()
syncPixels.fireDailyPixel()
}
return@withContext Result.success()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,52 @@

package com.duckduckgo.sync.impl.pixels

import android.content.SharedPreferences
import androidx.core.content.edit
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.sync.api.engine.*
import com.duckduckgo.sync.impl.Result.Error
import com.duckduckgo.sync.impl.pixels.SyncPixelName.SYNC_DAILY_PIXEL
import com.duckduckgo.sync.impl.stats.SyncStatsRepository
import com.duckduckgo.sync.store.SharedPrefsProvider
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import javax.inject.Inject
import org.threeten.bp.Instant
import org.threeten.bp.ZoneOffset
import org.threeten.bp.format.DateTimeFormatter

interface SyncPixels {

/**
* Fired once per day, for all users with sync enabled
*/
fun fireDailyPixel()

/**
* Fired when adding new device to existing account
*/
fun fireLoginPixel()

/**
* Fired when user sets up a sync account from connect flow
*/
fun fireSignupConnectPixel()

/**
* Fired when user sets up a sync account directly.
*/
fun fireSignupDirectPixel()

fun fireStatsPixel()

fun fireOrphanPresentPixel(feature: String)

fun firePersisterErrorPixel(feature: String, mergeError: SyncMergeResult.Error)
fun firePersisterErrorPixel(
feature: String,
mergeError: SyncMergeResult.Error,
)

fun fireEncryptFailurePixel()

Expand All @@ -48,10 +80,33 @@ interface SyncPixels {
}

@ContributesBinding(AppScope::class)
@SingleInstanceIn(AppScope::class)
class RealSyncPixels @Inject constructor(
private val pixel: Pixel,
private val statsRepository: SyncStatsRepository,
private val sharedPrefsProvider: SharedPrefsProvider,
) : SyncPixels {

private val preferences: SharedPreferences by lazy {
sharedPrefsProvider.getSharedPrefs(SYNC_PIXELS_PREF_FILE)
}

override fun fireDailyPixel() {
tryToFireDailyPixel(SYNC_DAILY_PIXEL)
}

override fun fireLoginPixel() {
pixel.fire(SyncPixelName.SYNC_LOGIN)
}

override fun fireSignupConnectPixel() {
pixel.fire(SyncPixelName.SYNC_SIGNUP_CONNECT)
}

override fun fireSignupDirectPixel() {
pixel.fire(SyncPixelName.SYNC_SIGNUP_DIRECT)
}

override fun fireStatsPixel() {
val dailyStats = statsRepository.getDailyStats()
pixel.fire(
Expand All @@ -77,7 +132,10 @@ class RealSyncPixels @Inject constructor(
)
}

override fun firePersisterErrorPixel(feature: String, mergeError: SyncMergeResult.Error) {
override fun firePersisterErrorPixel(
feature: String,
mergeError: SyncMergeResult.Error,
) {
pixel.fire(
SyncPixelName.SYNC_PERSISTER_FAILURE,
mapOf(
Expand Down Expand Up @@ -128,9 +186,42 @@ class RealSyncPixels @Inject constructor(
),
)
}

private fun tryToFireDailyPixel(
pixel: SyncPixelName,
payload: Map<String, String> = emptyMap(),
) {
val now = getUtcIsoLocalDate()
val timestamp = preferences.getString(pixel.name.appendTimestampSuffix(), null)

// check if pixel was already sent in the current day
if (timestamp == null || now > timestamp) {
this.pixel.fire(pixel, payload)
.also { preferences.edit { putString(pixel.name.appendTimestampSuffix(), now) } }
}
}

private fun getUtcIsoLocalDate(): String {
// returns YYYY-MM-dd
return Instant.now().atOffset(ZoneOffset.UTC).format(DateTimeFormatter.ISO_LOCAL_DATE)
}

private fun String.appendTimestampSuffix(): String {
return "${this}_timestamp"
}

companion object {
private const val SYNC_PIXELS_PREF_FILE = "com.duckduckgo.sync.pixels.v1"
}
}

// https://app.asana.com/0/72649045549333/1205649300615861
enum class SyncPixelName(override val pixelName: String) : Pixel.PixelName {
SYNC_DAILY_PIXEL("m_sync_daily"),
SYNC_LOGIN("m_sync_login"),
SYNC_SIGNUP_DIRECT("m_sync_signup_direct"),
SYNC_SIGNUP_CONNECT("m_sync_signup_connect"),

SYNC_SUCCESS_RATE("m_sync_daily_success_rate"),
SYNC_DAILY_ATTEMPTS("m_sync_daily_attempts"),
SYNC_ORPHAN_PRESENT("m_sync_orphan_present"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.duckduckgo.sync.impl.R.string
import com.duckduckgo.sync.impl.Result.Error
import com.duckduckgo.sync.impl.Result.Success
import com.duckduckgo.sync.impl.SyncAccountRepository
import com.duckduckgo.sync.impl.pixels.SyncPixels
import com.duckduckgo.sync.impl.ui.SyncConnectViewModel.Command.LoginSuccess
import com.duckduckgo.sync.impl.ui.SyncConnectViewModel.Command.ReadTextCode
import com.duckduckgo.sync.impl.ui.SyncConnectViewModel.Command.ShowMessage
Expand All @@ -48,6 +49,7 @@ class SyncConnectViewModel @Inject constructor(
private val syncAccountRepository: SyncAccountRepository,
private val qrEncoder: QREncoder,
private val clipboard: Clipboard,
private val syncPixels: SyncPixels,
private val dispatchers: DispatcherProvider,
) : ViewModel() {
private val command = Channel<Command>(1, DROP_OLDEST)
Expand All @@ -66,6 +68,7 @@ class SyncConnectViewModel @Inject constructor(
delay(POLLING_INTERVAL)
when (syncAccountRepository.pollConnectionKeys()) {
is Success -> {
syncPixels.fireSignupConnectPixel()
command.send(LoginSuccess)
polling = false
}
Expand Down Expand Up @@ -131,13 +134,17 @@ class SyncConnectViewModel @Inject constructor(
viewModelScope.launch(dispatchers.io()) {
when (syncAccountRepository.processCode(qrCode)) {
is Error -> command.send(Command.Error)
is Success -> command.send(LoginSuccess)
is Success -> {
syncPixels.fireLoginPixel()
command.send(LoginSuccess)
}
}
}
}

fun onLoginSuccess() {
viewModelScope.launch {
syncPixels.fireLoginPixel()
command.send(LoginSuccess)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.sync.impl.Result.Error
import com.duckduckgo.sync.impl.SyncAccountRepository
import com.duckduckgo.sync.impl.pixels.SyncPixels
import com.duckduckgo.sync.impl.ui.SyncLoginViewModel.Command.LoginSucess
import com.duckduckgo.sync.impl.ui.SyncLoginViewModel.Command.ReadTextCode
import javax.inject.*
Expand All @@ -35,6 +36,7 @@ import kotlinx.coroutines.launch
@ContributesViewModel(ActivityScope::class)
class SyncLoginViewModel @Inject constructor(
private val syncAccountRepository: SyncAccountRepository,
private val syncPixels: SyncPixels,
private val dispatchers: DispatcherProvider,
) : ViewModel() {
private val command = Channel<Command>(1, DROP_OLDEST)
Expand All @@ -54,6 +56,7 @@ class SyncLoginViewModel @Inject constructor(

fun onLoginSuccess() {
viewModelScope.launch {
syncPixels.fireLoginPixel()
command.send(LoginSucess)
}
}
Expand All @@ -64,6 +67,7 @@ class SyncLoginViewModel @Inject constructor(
if (result is Error) {
command.send(Command.Error)
} else {
syncPixels.fireLoginPixel()
command.send(LoginSucess)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.sync.impl.Result.Error
import com.duckduckgo.sync.impl.Result.Success
import com.duckduckgo.sync.impl.SyncAccountRepository
import com.duckduckgo.sync.impl.pixels.SyncPixels
import com.duckduckgo.sync.impl.ui.setup.SyncCreateAccountViewModel.Command.FinishSetupFlow
import com.duckduckgo.sync.impl.ui.setup.SyncCreateAccountViewModel.ViewMode.CreatingAccount
import javax.inject.*
Expand All @@ -38,6 +39,7 @@ import kotlinx.coroutines.launch
@ContributesViewModel(ActivityScope::class)
class SyncCreateAccountViewModel @Inject constructor(
private val syncAccountRepository: SyncAccountRepository,
private val syncPixels: SyncPixels,
private val dispatchers: DispatcherProvider,
) : ViewModel() {

Expand Down Expand Up @@ -74,6 +76,7 @@ class SyncCreateAccountViewModel @Inject constructor(
}

is Success -> {
syncPixels.fireSignupDirectPixel()
command.send(FinishSetupFlow)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (c) 2023 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.sync.impl.pixels

import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.common.test.api.InMemorySharedPreferences
import com.duckduckgo.sync.impl.stats.SyncStatsRepository
import com.duckduckgo.sync.store.SharedPrefsProvider
import org.junit.Before
import org.junit.Test
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

class SyncPixelsTest {

private var pixel: Pixel = mock()
private var syncStatsRepository: SyncStatsRepository = mock()
private var sharedPrefsProv: SharedPrefsProvider = mock()

private lateinit var testee: RealSyncPixels

@Before
fun setUp() {
val prefs = InMemorySharedPreferences()
whenever(
sharedPrefsProv.getSharedPrefs(eq("com.duckduckgo.sync.pixels.v1")),
).thenReturn(prefs)

testee = RealSyncPixels(
pixel,
syncStatsRepository,
sharedPrefsProv,
)
}

@Test
fun whenDailyPixelCalledThenPixelFired() {
testee.fireDailyPixel()

verify(pixel).fire(SyncPixelName.SYNC_DAILY_PIXEL)
}

@Test
fun whenDailyPixelCalledTwiceThenPixelFiredOnce() {
testee.fireDailyPixel()
testee.fireDailyPixel()

verify(pixel).fire(SyncPixelName.SYNC_DAILY_PIXEL)
}

@Test
fun whenLoginPixelCalledThenPixelFired() {
testee.fireLoginPixel()

verify(pixel).fire(SyncPixelName.SYNC_LOGIN)
}

@Test
fun whenSignupDirectPixelCalledThenPixelFired() {
testee.fireSignupDirectPixel()

verify(pixel).fire(SyncPixelName.SYNC_SIGNUP_DIRECT)
}

@Test
fun whenSignupConnectPixelCalledThenPixelFired() {
testee.fireSignupConnectPixel()

verify(pixel).fire(SyncPixelName.SYNC_SIGNUP_CONNECT)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class SyncActivityViewModelTest {
}

@Test
fun whenSyncWithAnoterDeviceThenEmitCommandSyncWithAnotherDevice() = runTest {
fun whenSyncWithAnotherDeviceThenEmitCommandSyncWithAnotherDevice() = runTest {
testee.onSyncWithAnotherDevice()

testee.commands().test {
Expand Down
Loading
Loading