Skip to content

Commit

Permalink
Sync: Usage Pixels and Monitoring (#4018)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/488551667048375/1206193954622385/f

### Description
This PR adds the usage pixels for Sync

### Steps to test this PR

_Signup Direct_
Fired when user sets up a sync account directly.
- [x] Create a new sync account
- [x] Verify “m.sync.signup.direct” pixel is sent

_Signup Connect_
Fired when user sets up a sync account from connect flow. You’ll need
two devices for this one.
- [x] Device A - Open Sync Setup screen
- [x] Tap on Sync with another device
- [x] Device B- Open Sync Setup screen
- [x] Tap on Sync with another device
- [x] Device A - Read QR code present in Device B
- [x] Verify “m.sync.signup.connect” pixel is sent in Device B
- [x] Verify “m.sync.login” pixel is sent in Device A

_Login - Entering Code_
Fired when adding new device to existing account. You’ll need two
devices for this one.
- [x] Device A - Create a new sync account
- [x] Copy the recovery code
- [x] Device B- Open Sync Setup screen
- [x] Tap on Sync with another device
- [x] Tap on Manually Enter Code
- [x] Paste code from Device A
- [x] Verify “m.sync.login” pixel is sent in Device B

_Login - Scan QR_
Fired when adding new device to existing account. You’ll need two
devices for this one.
- [x] Device A - Create a new sync account
- [x] Navigate to Sync Settings
- [x] Device B- Open Sync Setup screen
- [x] Tap on Sync with another device
- [x] Scan QR Code from Device A
- [x] Verify “m.sync.login” pixel is sent in Device B

_Login - Recover Account with Code_
Fired when adding new device to existing account. You’ll need two
devices for this one.
- [x] Device A - Create a new sync account
- [x] Copy the recovery code
- [x] Device B- Open Sync Setup screen
- [x] Tap on Recover Synced Data
- [x] Tap on Manually Enter Code
- [x] Paste code from Device A
- [x] Verify “m.sync.login” pixel is sent in Device B

_Login - Recover Account with QR_
Fired when adding new device to existing account. You’ll need two
devices for this one.
- [x] Device A - Create a new sync account
- [x] Navigate to Sync Settings
- [x] Device B- Open Sync Setup screen
- [x] Tap on Recover Synced Data
- [x] Scan QR Code from Device A
- [x] Verify “m.sync.login” pixel is sent in Device B

_Daily pixel_
Fired for first sync on a given day.
- [x] Create a new sync account
- [x] Verify “m.sync.daily” pixel is sent
- [x] Close app
- [x] Open app again
- [x] Verify “m.sync.daily” pixel is NOT sent
  • Loading branch information
malmstein authored Dec 22, 2023
1 parent 951d355 commit a0f0fdc
Show file tree
Hide file tree
Showing 15 changed files with 236 additions and 9 deletions.
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
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 @@ -150,7 +150,7 @@ class SyncActivityViewModelTest {
}

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

testee.commands().test {
Expand Down
Loading

0 comments on commit a0f0fdc

Please sign in to comment.