Closes mozilla-mobile#5371 - Introduce 'sync' ping, add 'syncUuid' to…

… all sync-related pings This patch introduce an 'overarching' sync ping, which is meant to contain information describing a sync overall, vs individual engine runs. Currently it contains global sync errors. syncUuid was also added to all sync-related pings, allowing us to tie together in an analysis all pings that were emitted as part of a single "sync".
grigoryk · Dec 24, 2019 · f465730 · f465730
1 parent 081e685
commit f465730
Show file tree

Hide file tree

Showing 7 changed files with 283 additions and 115 deletions.
diff --git a/...efox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt b/...efox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt
@@ -435,22 +435,7 @@ class WorkManagerSyncWorker(
         }
 
         // Process telemetry.
-        syncResult.telemetry?.let {
-            // Yes, this is non-ideal...
-            // But, what this does: individual 'process' function will report global sync errors
-            // as part of its corresponding ping. We don't want to report a global sync error multiple times,
-            // so we check for the boolean flag that indicates if this happened or not.
-            // There's a complete mismatch between what Glean supports and what we need it to do here.
-            // Glean doesn't support "nested metrics" and so we resort to these hacks.
-            // It shouldn't matter in which order these 'process' functions are called.
-            var noGlobalErrorsReported = SyncTelemetry.processBookmarksPing(it)
-            if (noGlobalErrorsReported) {
-                noGlobalErrorsReported = SyncTelemetry.processHistoryPing(it)
-            }
-            if (noGlobalErrorsReported) {
-                SyncTelemetry.processPasswordsPing(it)
-            }
-        }
+        syncResult.telemetry?.let { SyncTelemetry.processSyncTelemetry(it) }
 
         // Finally, declare success, failure or request a retry based on 'sync status'.
         return when (syncResult.status) {

diff --git a/...ce/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt b/...ce/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt
@@ -333,7 +333,7 @@ open class AsyncLoginsStorageAdapter<T : LoginsStorage>(private val wrapped: T)
     override fun sync(syncInfo: SyncUnlockInfo): Deferred<SyncTelemetryPing> {
         return scope.async {
             val ping = wrapped.sync(syncInfo)
-            SyncTelemetry.processPasswordsPing(ping)
+            SyncTelemetry.processLoginsPing(ping)
             ping
         }
     }

diff --git a/components/support/sync-telemetry/docs/metrics.md b/components/support/sync-telemetry/docs/metrics.md
@@ -10,6 +10,7 @@ This means you might have to go searching through the dependency tree to get a f
  - [bookmarks-sync](#bookmarks-sync)
  - [history-sync](#history-sync)
  - [logins-sync](#logins-sync)
+ - [sync](#sync)
 
 
 ## bookmarks-sync
@@ -50,13 +51,26 @@ The following metrics are added to the ping:
 
 | Name | Type | Description | Data reviews | Extras | Expiration |
 | --- | --- | --- | --- | --- | --- |
+| bookmarks_sync.sync_uuid |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (history, logins) and the global sync ping.  |[1](TODO)||never |
+| history_sync.sync_uuid |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (bookmarks, logins) and the global sync ping.  |[1](TODO)||never |
 | logins_sync.failure_reason |[labeled_string](https://mozilla.github.io/glean/book/user/metrics/labeled_strings.html) |Records why the passwords sync failed: either due to an authentication error, unexpected exception, or other error. The error strings are truncated and sanitized to omit PII, like usernames and passwords.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|<ul><li>other</li><li>unexpected</li><li>auth</li></ul>|never |
 | logins_sync.finished_at |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |Records when the passwords sync finished. This includes the time to download, apply, and upload all records.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never |
 | logins_sync.incoming |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Records incoming passwords record counts. `applied` is the number of incoming passwords entries that were successfully stored or updated in the local database. `failed_to_apply` is the number of entries that were ignored due to errors. `reconciled` is the number of entries with changes both locally and remotely that were merged.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|<ul><li>applied</li><li>failed_to_apply</li><li>reconciled</li></ul>|never |
 | logins_sync.outgoing |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Records outgoing passwords record counts. `uploaded` is the number of records that were successfully sent to the server. `failed_to_upload` is the number of records that weren't uploaded, and will be retried on the next sync.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|<ul><li>uploaded</li><li>failed_to_upload</li></ul>|never |
 | logins_sync.outgoing_batches |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |Records the number of batches needed to upload all outgoing records. The Sync server has a hard limit on the number of records (and request body bytes) on the number of records that can fit into a single batch, and large syncs may require multiple batches.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never |
 | logins_sync.started_at |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |Records when the passwords sync started.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never |
+| logins_sync.sync_uuid |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (bookmarks, history) and the global sync ping.  |[1](TODO)||never |
 | logins_sync.uid |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The user's hashed Firefox Account ID.  |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never |
+| sync.sync_uuid |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |Unique identifier for this sync, used to correlate together individual sync pings (history, bookmarks, logins).  |[1](TODO)||never |
+
+## sync
+A summary sync ping, sent for every sync. It doesn't include the `client_id` because it reports a hashed version of the user's Firefox Account ID.
+
+The following metrics are added to the ping:
+
+| Name | Type | Description | Data reviews | Extras | Expiration |
+| --- | --- | --- | --- | --- | --- |
+| sync.failure_reason |[labeled_string](https://mozilla.github.io/glean/book/user/metrics/labeled_strings.html) |Records a global sync failure: either due to an authentication error, unexpected exception, or other error. The error strings are truncated and sanitized to omit PII, like URLs and file system paths.  |[1](https://github.com/mozilla-mobile/android-components/pull/3092)|<ul><li>other</li><li>unexpected</li><li>auth</li></ul>|never |
 
 
 <!-- AUTOGENERATED BY glean_parser.  DO NOT EDIT. -->

diff --git a/components/support/sync-telemetry/metrics.yaml b/components/support/sync-telemetry/metrics.yaml
@@ -4,11 +4,61 @@
 
 $schema: moz://mozilla.org/schemas/glean/metrics/1-0-0
 
+sync:
+  sync_uuid:
+    type: uuid
+    description: >
+      Unique identifier for this sync, used to correlate together individual sync pings (history, bookmarks, logins).
+    send_in_pings:
+      - logins-sync
+    bugs:
+      - https://github.com/mozilla-mobile/android-components/issues/5371
+    data_reviews:
+      - TODO
+    notification_emails:
+      - sync-core@mozilla.com
+    expires: never
+    lifetime: ping
+  failure_reason:
+    type: labeled_string
+    labels:
+      - other
+      - unexpected
+      - auth
+    description: >
+      Records a global sync failure: either due to an authentication error, unexpected exception,
+      or other error that caused the sync to fail. Error strings are truncated and sanitized to omit
+      PII, like URLs and file system paths.
+    send_in_pings:
+      - sync
+    bugs:
+      - https://github.com/mozilla-mobile/android-components/pull/3092
+    data_reviews:
+      - https://github.com/mozilla-mobile/android-components/pull/3092
+    notification_emails:
+      - sync-core@mozilla.com
+    expires: never
+    lifetime: ping
+
 # `history-sync`, `logins-sync` and `bookmarks-sync` metrics all use the same structure,
 # but must be specified individually. We can't define them once and use
 # `send_in_pings` because the stores might be synced in parallel, and we can't
 # guarantee that a ping for one store would be sent before the others.
 history_sync:
+  sync_uuid:
+    type: uuid
+    description: >
+      Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (bookmarks, logins) and the global sync ping.
+    send_in_pings:
+      - logins-sync
+    bugs:
+      - https://github.com/mozilla-mobile/android-components/issues/5371
+    data_reviews:
+      - TODO
+    notification_emails:
+      - sync-core@mozilla.com
+    expires: never
+    lifetime: ping
   uid:
     type: string
     description: >
@@ -135,6 +185,20 @@ history_sync:
     lifetime: ping
 
 logins_sync:
+  sync_uuid:
+    type: uuid
+    description: >
+      Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (bookmarks, history) and the global sync ping.
+    send_in_pings:
+      - logins-sync
+    bugs:
+      - https://github.com/mozilla-mobile/android-components/issues/5371
+    data_reviews:
+      - TODO
+    notification_emails:
+      - sync-core@mozilla.com
+    expires: never
+    lifetime: ping
   uid:
     type: string
     description: >
@@ -261,6 +325,20 @@ logins_sync:
     lifetime: ping
 
 bookmarks_sync:
+  sync_uuid:
+    type: uuid
+    description: >
+      Unique identifier for the overarching sync, used to correlate this ping together with other individual sync pings (history, logins) and the global sync ping.
+    send_in_pings:
+      - logins-sync
+    bugs:
+      - https://github.com/mozilla-mobile/android-components/issues/5371
+    data_reviews:
+      - TODO
+    notification_emails:
+      - sync-core@mozilla.com
+    expires: never
+    lifetime: ping
   uid:
     type: string
     description: >

diff --git a/components/support/sync-telemetry/pings.yaml b/components/support/sync-telemetry/pings.yaml
@@ -4,6 +4,17 @@
 
 $schema: moz://mozilla.org/schemas/glean/pings/1-0-0
 
+sync:
+  description: >
+    A summary sync ping, sent for every sync.
+    It doesn't include the `client_id` because it reports a hashed version of the user's Firefox Account ID.
+  include_client_id: false
+  bugs:
+    - https://github.com/mozilla-mobile/android-components/issues/5371
+  notification_emails:
+    - sync-core@mozilla.com
+  data_reviews:
+    - TODO
 history-sync:
   description: >
     A ping sent for every history sync. It doesn't include the `client_id`