Pass Sync telemetry from a-s to consumers.

components/browser/storage-sync/build.gradle

@@ -32,6 +32,7 @@ dependencies {
     implementation project(':support-utils')
 
     implementation Dependencies.kotlin_stdlib
+    implementation Dependencies.mozilla_appservices_support
 
     testImplementation project(':support-test')
 

components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Connection.kt

@@ -8,11 +8,36 @@ import android.support.annotation.GuardedBy
 import mozilla.appservices.places.PlacesApi
 import mozilla.appservices.places.PlacesReaderConnection
 import mozilla.appservices.places.PlacesWriterConnection
+import mozilla.appservices.support.SyncTelemetryPing
+import mozilla.components.concept.sync.StoreStats
 import java.io.Closeable
 import java.io.File
 
 const val DB_NAME = "places.sqlite"
 
+fun pingsToStats(ping: SyncTelemetryPing): Map<String, StoreStats> {
+    return ping.syncs.flatMap { it.engines }
+        .groupingBy { it.name }
+        .aggregate { _, stats: StoreStats?, engine, _ ->
+            val (sent, failed) = engine.outgoing.fold(Pair(0, 0)) { acc, outgoing ->
+                Pair(acc.first + outgoing.sent, acc.second + outgoing.failed)
+            }
+            StoreStats(
+                    name = engine.name,
+                    at = engine.at,
+                    took = engine.took,
+                    applied = (stats?.applied ?: 0) + (engine.incoming?.applied ?: 0),
+                    failedToApply = (stats?.failedToApply ?: 0) +
+                            (engine.incoming?.failed ?: 0) + (engine.incoming?.newFailed ?: 0),
+                    reconciled = (stats?.reconciled ?: 0) + (engine.incoming?.reconciled ?: 0),
+                    uploaded = (stats?.uploaded ?: 0) + sent,
+                    failedToUpload = (stats?.failedToUpload ?: 0) + failed,
+                    uploadBatches = (stats?.uploadBatches ?: 0) + engine.outgoing.size
+            )
+        }
+
+}
+
 /**
  * A slight abstraction over [PlacesApi].
  *
@@ -28,8 +53,8 @@ interface Connection : Closeable {
 
     // Until we get a real SyncManager in application-services libraries, we'll have to live with this
     // strange split that doesn't quite map all that well to our internal storage model.
-    fun syncHistory(syncInfo: SyncAuthInfo)
-    fun syncBookmarks(syncInfo: SyncAuthInfo)
+    fun syncHistory(syncInfo: SyncAuthInfo): StoreStats?
+    fun syncBookmarks(syncInfo: SyncAuthInfo): StoreStats?
 }
 
 /**
@@ -65,14 +90,16 @@ internal object RustPlacesConnection : Connection {
         return api!!.getWriter()
     }
 
-    override fun syncHistory(syncInfo: SyncAuthInfo) {
+    override fun syncHistory(syncInfo: SyncAuthInfo): StoreStats? {
         check(api != null) { "must call init first" }
-        api!!.syncHistory(syncInfo)
+        val ping = api!!.syncHistory(syncInfo)
+        return pingsToStats(ping).get("history")
     }
 
-    override fun syncBookmarks(syncInfo: SyncAuthInfo) {
+    override fun syncBookmarks(syncInfo: SyncAuthInfo): StoreStats? {
         check(api != null) { "must call init first" }
-        api!!.syncBookmarks(syncInfo)
+        val ping = api!!.syncBookmarks(syncInfo)
+        return pingsToStats(ping).get("bookmarks")
     }
 
     override fun close() = synchronized(this) {

components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt

@@ -18,6 +18,7 @@ import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.components.concept.storage.BookmarksStorage
 import mozilla.components.concept.sync.AuthInfo
+import mozilla.components.concept.sync.StoreSyncStatus
 import mozilla.components.concept.sync.SyncStatus
 import mozilla.components.concept.sync.SyncableStore
 
@@ -175,14 +176,14 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo
      * @param authInfo The authentication information to sync with.
      * @return Sync status of OK or Error
      */
-    override suspend fun sync(authInfo: AuthInfo): SyncStatus {
+    override suspend fun sync(authInfo: AuthInfo): StoreSyncStatus {
         return try {
             withContext(scope.coroutineContext) {
-                places.syncBookmarks(authInfo.into())
-                SyncStatus.Ok
+                val stats = places.syncBookmarks(authInfo.into())
+                StoreSyncStatus(SyncStatus.Ok, stats)
             }
         } catch (e: PlacesException) {
-            SyncStatus.Error(e)
+            StoreSyncStatus(SyncStatus.Error(e), null)
         }
     }
 }

components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt

@@ -15,6 +15,7 @@ import mozilla.components.concept.storage.SearchResult
 import mozilla.components.concept.storage.VisitInfo
 import mozilla.components.concept.storage.VisitType
 import mozilla.components.concept.sync.AuthInfo
+import mozilla.components.concept.sync.StoreSyncStatus
 import mozilla.components.concept.sync.SyncStatus
 import mozilla.components.concept.sync.SyncableStore
 import mozilla.components.support.utils.segmentAwareDomainMatch
@@ -162,14 +163,14 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist
      * @param authInfo The authentication information to sync with.
      * @return Sync status of OK or Error
      */
-    override suspend fun sync(authInfo: AuthInfo): SyncStatus {
+    override suspend fun sync(authInfo: AuthInfo): StoreSyncStatus {
         return try {
             withContext(scope.coroutineContext) {
-                places.syncHistory(authInfo.into())
-                SyncStatus.Ok
+                val stats = places.syncHistory(authInfo.into())
+                StoreSyncStatus(SyncStatus.Ok, stats)
             }
         } catch (e: PlacesException) {
-            SyncStatus.Error(e)
+            StoreSyncStatus(SyncStatus.Error(e), null)
         }
     }
 }

components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt

@@ -11,9 +11,11 @@ import mozilla.appservices.places.PlacesWriterConnection
 import mozilla.appservices.places.SearchResult
 import mozilla.appservices.places.VisitInfo
 import mozilla.appservices.places.VisitObservation
+import mozilla.appservices.support.SyncTelemetryPing
 import mozilla.components.concept.storage.PageObservation
 import mozilla.components.concept.storage.VisitType
 import mozilla.components.concept.sync.AuthInfo
+import mozilla.components.concept.sync.StoreStats
 import mozilla.components.concept.sync.SyncStatus
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
@@ -343,12 +345,13 @@ class PlacesHistoryStorageTest {
                 return mock()
             }
 
-            override fun syncHistory(syncInfo: SyncAuthInfo) {
+            override fun syncHistory(syncInfo: SyncAuthInfo): StoreStats? {
                 assertNull(passedAuthInfo)
                 passedAuthInfo = syncInfo
+                return null
             }
 
-            override fun syncBookmarks(syncInfo: SyncAuthInfo) {
+            override fun syncBookmarks(syncInfo: SyncAuthInfo): StoreStats? {
                 fail()
             }
 
@@ -379,9 +382,13 @@ class PlacesHistoryStorageTest {
                 return mock()
             }
 
-            override fun syncHistory(syncInfo: SyncAuthInfo) {}
+            override fun syncHistory(syncInfo: SyncAuthInfo): StoreStats? {
+                return null
+            }
 
-            override fun syncBookmarks(syncInfo: SyncAuthInfo) {}
+            override fun syncBookmarks(syncInfo: SyncAuthInfo): StoreStats? {
+                return null
+            }
 
             override fun close() {
                 fail()
@@ -407,12 +414,13 @@ class PlacesHistoryStorageTest {
                 return mock()
             }
 
-            override fun syncHistory(syncInfo: SyncAuthInfo) {
+            override fun syncHistory(syncInfo: SyncAuthInfo): StoreStats? {
                 throw exception
             }
 
-            override fun syncBookmarks(syncInfo: SyncAuthInfo) {
+            override fun syncBookmarks(syncInfo: SyncAuthInfo): StoreStats? {
                 fail()
+                return null
             }
 
             override fun close() {

components/concept/sync/src/main/java/mozilla/components/concept/sync/Sync.kt

@@ -7,6 +7,31 @@ package mozilla.components.concept.sync
 import mozilla.components.support.base.observer.Observable
 import java.lang.Exception
 
+/**
+ * Timestamps and counts for a synced store.
+ *
+ * @param name The name of this store.
+ * @param at When this store was synced.
+ * @param took How long it took this store to sync.
+ * @param applied The number of incoming changes that applied successfully.
+ * @param failedToApply The number of incoming changes that failed to apply.
+ * @param reconciled The number of incoming changes merged with local changes.
+ * @param uploaded The number of outgoing changes that uploaded successfully.
+ * @param failedToUpload The number of outgoing changes that failed to upload.
+ * @param uploadBatches The number of batches required to upload all changes.
+ */
+data class StoreStats(
+    val name: String,
+    val at: Int = 0,
+    val took: Int = 0,
+    val applied: Int = 0,
+    val failedToApply: Int = 0,
+    val reconciled: Int = 0,
+    val uploaded: Int = 0,
+    val failedToUpload: Int = 0,
+    val uploadBatches: Int = 0
+)
+
 /**
  * Results of running a sync via [SyncableStore.sync].
  */
@@ -32,7 +57,7 @@ interface SyncableStore {
      * @param authInfo Auth information necessary for syncing this store.
      * @return [SyncStatus] A status object describing how sync went.
      */
-    suspend fun sync(authInfo: AuthInfo): SyncStatus
+    suspend fun sync(authInfo: AuthInfo): StoreSyncStatus
 }
 
 /**
@@ -83,6 +108,11 @@ interface SyncStatusObserver {
      */
     fun onStarted()
 
+    /**
+     * Gets called when a store finishes syncing, and has telemetry to report.
+     */
+    fun onStoreSynced(stats: StoreStats)
+
     /**
      * Gets called at the end of a sync, after every configured syncable has been synchronized.
      */
@@ -99,4 +129,4 @@ interface SyncStatusObserver {
  * A set of results of running a sync operation for multiple instances of [SyncableStore].
  */
 typealias SyncResult = Map<String, StoreSyncStatus>
-data class StoreSyncStatus(val status: SyncStatus)
+data class StoreSyncStatus(val status: SyncStatus, val stats: StoreStats?)

components/feature/sync/src/main/java/mozilla/components/feature/sync/BackgroundSyncManager.kt

@@ -4,10 +4,7 @@
 
 package mozilla.components.feature.sync
 
-import mozilla.components.concept.sync.OAuthAccount
-import mozilla.components.concept.sync.SyncManager
-import mozilla.components.concept.sync.SyncStatusObserver
-import mozilla.components.concept.sync.SyncableStore
+import mozilla.components.concept.sync.*
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
@@ -168,6 +165,10 @@ abstract class GeneralSyncManager : SyncManager, Observable<SyncStatusObserver>
         notifyObservers { onStarted() }
     }
 
+    override fun onStoreSynced(stats: StoreStats) {
+        notifyObservers { onStoreSynced(stats) }
+    }
+
     override fun onIdle() {
         notifyObservers { onIdle() }
     }

components/feature/sync/src/main/java/mozilla/components/feature/sync/StorageSync.kt

@@ -7,14 +7,7 @@ package mozilla.components.feature.sync
 import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
-import mozilla.components.concept.sync.AuthException
-import mozilla.components.concept.sync.AuthInfo
-import mozilla.components.concept.sync.OAuthAccount
-import mozilla.components.concept.sync.StoreSyncStatus
-import mozilla.components.concept.sync.SyncResult
-import mozilla.components.concept.sync.SyncStatus
-import mozilla.components.concept.sync.SyncStatusObserver
-import mozilla.components.concept.sync.SyncableStore
+import mozilla.components.concept.sync.*
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
@@ -52,8 +45,9 @@ class StorageSync(
         val authInfo = try {
             account.authInfo(syncScope)
         } catch (e: AuthException) {
+            // TODO: Record telemetry for auth errors.
             syncableStores.keys.forEach { storeName ->
-                results[storeName] = StoreSyncStatus(SyncStatus.Error(e))
+                results[storeName] = StoreSyncStatus(SyncStatus.Error(e), null)
             }
             return@withListeners results
         }
@@ -70,18 +64,23 @@ class StorageSync(
         storeName: String,
         auth: AuthInfo
     ): StoreSyncStatus {
-        return StoreSyncStatus(store.sync(auth).also {
-            if (it is SyncStatus.Error) {
-                logger.error("Error synchronizing a $storeName store", it.exception)
+        return store.sync(auth).also { (status, _) ->
+            if (status is SyncStatus.Error) {
+                logger.error("Error synchronizing a $storeName store", status.exception)
             } else {
                 logger.info("Synchronized $storeName store.")
             }
-        })
+        }
     }
 
     private suspend fun withListeners(block: suspend () -> SyncResult): SyncResult {
         notifyObservers { onStarted() }
         val result = block()
+        result.values.forEach {
+            it.stats?.let {
+                notifyObservers { onStoreSynced(it) }
+            }
+        }
         notifyObservers { onIdle() }
         return result
     }

components/service/glean/src/main/java/mozilla/components/service/glean/error/ErrorRecording.kt

@@ -68,7 +68,7 @@ object ErrorRecording {
 
         logger.warn("${metricData.identifier}: $message")
 
-        // There are two reasons for using `CountersStorageEngine.record` below
+        // There are two reasons for using `CountersStorageEngine.add` below
         // and not just using the public `CounterMetricType` API.
 
         // 1) The labeled counter metrics that store the errors are defined in the
@@ -80,7 +80,7 @@ object ErrorRecording {
         // 2) We want to bybass the restriction that there are only N values in a
         // dynamically labeled metric.  Error reporting should never report errors
         // in the __other__ category.
-        CountersStorageEngine.record(
+        CountersStorageEngine.add(
             errorMetric,
             amount = 1
         )

components/service/glean/src/main/java/mozilla/components/service/glean/private/CounterMetricType.kt

@@ -30,6 +30,26 @@ data class CounterMetricType(
 
     private val logger = Logger("glean/CounterMetricType")
 
+    /**
+     * ...
+     *
+     * @param value This is the new value for the counter.
+     */
+    fun set(value: Int) {
+        if (!shouldRecord(logger)) {
+            return
+        }
+
+        @Suppress("EXPERIMENTAL_API_USAGE")
+        Dispatchers.API.launch {
+            // Delegate storing the new counter value to the storage engine.
+            CountersStorageEngine.set(
+                    this@CounterMetricType,
+                    value = value
+            )
+        }
+    }
+
     /**
      * Add to counter value.
      *
@@ -44,7 +64,7 @@ data class CounterMetricType(
         @Suppress("EXPERIMENTAL_API_USAGE")
         Dispatchers.API.launch {
             // Delegate storing the new counter value to the storage engine.
-            CountersStorageEngine.record(
+            CountersStorageEngine.add(
                     this@CounterMetricType,
                     amount = amount
             )