Bug 1817029 - Add the client-deduplication ping (#861)

* Bug 1817245 - new Nimbus feature for controlling client-deduplication ping

This feature will enable/disable the sending of the `client-deduplication`
ping that will be added in a later PR.

* Bug 1817029 - Add the client-deduplication ping

The `client-deduplication` ping will be used to determine if
client IDs are being regenerated erroneously. This ping uses
the lifecycle observers to (almost) replicate the same scheduling
as the baseline ping.

* Bug 1817029 - Suggested changes

changelog
- add `reason` for new ping
- update the unit test for the new ping
- add new metrics
- allow for custom salt when hashing the Ad ID
- move the Nimbus check to the lifecycle observer
- record the new metrics

* Bug 1817029 - Update fenix/.experimenter.yaml

Co-authored-by: Alessio Placitelli <alessio.placitelli@gmail.com>

* Bug 1817029 - remove unnecessary pass-through function

* Bug 1817029 - add the GleanTestRule for tests

* Bug 1817029 - Update fenix/app/src/test/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPingTest.kt

Co-authored-by: Travis Long <tlong@mozilla.com>

* Bug 1817029 - remove unnecessary call to main thread

* Bug 1817029 - update comment about hashing

---------

Co-authored-by: Alessio Placitelli <alessio.placitelli@gmail.com>
Co-authored-by: Travis Long <tlong@mozilla.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.experimenter.yaml

@@ -1,4 +1,12 @@
 ---
+client-deduplication:
+  description: A feature to control the sending of the client-deduplication ping.
+  hasExposure: true
+  exposureDescription: ""
+  variables:
+    enabled:
+      type: boolean
+      description: "If true, the ping will be sent."
 cookie-banners:
   description: Features for cookie banner handling.
   hasExposure: true

app/metrics.yaml

@@ -258,6 +258,7 @@ events:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/17089
     data_reviews:
@@ -1459,6 +1460,7 @@ metrics:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/1158
       - https://github.com/mozilla-mobile/fenix/issues/6556
@@ -1754,6 +1756,7 @@ metrics:
       please see `has_open_tabs`.
     send_in_pings:
       - metrics
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/11479
     data_reviews:
@@ -2551,6 +2554,7 @@ search.default_engine:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/800
     data_reviews:
@@ -2580,6 +2584,7 @@ search.default_engine:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/800
     data_reviews:
@@ -2983,6 +2988,7 @@ activation:
       This will never be sent in a ping that also contains the client_id.
     send_in_pings:
       - activation
+      - client-deduplication
     bugs:
       - https://bugzilla.mozilla.org/1538011
       - https://bugzilla.mozilla.org/1501822
@@ -3165,6 +3171,8 @@ sync_auth:
     notification_emails:
       - android-probes@mozilla.com
       - kbrosnan@mozilla.com
+    send_in_pings:
+      - client-deduplication
     expires: never
     metadata:
       tags:
@@ -6425,6 +6433,7 @@ browser.search:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/6558
       - https://github.com/mozilla-mobile/fenix/issues/28010
@@ -6459,6 +6468,7 @@ browser.search:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/6558
       - https://github.com/mozilla-mobile/fenix/issues/28010
@@ -6484,6 +6494,7 @@ browser.search:
     send_in_pings:
       - metrics
       - baseline
+      - client-deduplication
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/6557
     data_reviews:
@@ -9001,6 +9012,39 @@ review_prompt:
     data_sensitivity:
       - interaction
     expires: 121
+client_deduplication:
+  valid_advertising_id:
+    type: boolean
+    description: |
+      Whether or not we get a valid advertising ID from the device.
+    send_in_pings:
+      - client-deduplication
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11
+    data_sensitivity:
+      - interaction
+    notification_emails:
+      - android-probes@mozilla.com
+      - fbertsch@mozilla.com
+    expires: 122
+  experiment_timeframe:
+    type: string
+    description: |
+      A string we use to identify which run of the experiment this is.
+    send_in_pings:
+      - client-deduplication
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11
+    data_sensitivity:
+      - interaction
+    notification_emails:
+      - android-probes@mozilla.com
+      - fbertsch@mozilla.com
+    expires: 122
 
 private_browsing_shortcut_cfr:
   add_shortcut:

app/pings.yaml

@@ -62,3 +62,21 @@ spoc:
     - https://github.com/mozilla-mobile/fenix/pull/27550#issuecomment-1295027631
   notification_emails:
     - android-probes@mozilla.com
+
+client-deduplication:
+  description: |
+    Contains data to help identify if client IDs are being regenerated
+    erroneously.
+  include_client_id: true
+  reasons:
+    active: |
+      The ping is being sent when the app is coming to the foreground.
+    inactive: |
+      The ping is being sent when the app is going to the background.
+  bugs:
+    - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029
+  data_reviews:
+    - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11
+  notification_emails:
+    - android-probes@mozilla.com
+    - fbertsch@mozilla.com

app/src/main/java/org/mozilla/fenix/FenixApplication.kt

@@ -75,6 +75,7 @@ import org.mozilla.fenix.components.Core
 import org.mozilla.fenix.components.appstate.AppAction
 import org.mozilla.fenix.components.metrics.MetricServiceType
 import org.mozilla.fenix.components.metrics.MozillaProductDetector
+import org.mozilla.fenix.components.metrics.clientdeduplication.ClientDeduplicationLifecycleObserver
 import org.mozilla.fenix.components.toolbar.ToolbarPosition
 import org.mozilla.fenix.experiments.maybeFetchExperiments
 import org.mozilla.fenix.ext.areNotificationsEnabledSafe
@@ -186,6 +187,12 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
         GlobalScope.launch(Dispatchers.IO) {
             setStartupMetrics(store, settings())
         }
+
+        ProcessLifecycleOwner.get().lifecycle.addObserver(
+            ClientDeduplicationLifecycleObserver(
+                this.applicationContext,
+            ),
+        )
     }
 
     @CallSuper

app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt

@@ -16,13 +16,6 @@ import org.mozilla.fenix.GleanMetrics.Pings
 import org.mozilla.fenix.components.metrics.MetricsUtils.getHashedIdentifier
 
 class ActivationPing(private val context: Context) {
-    companion object {
-        // The number of iterations to compute the hash. RFC 2898 suggests
-        // a minimum of 1000 iterations.
-        const val PBKDF2_ITERATIONS = 1000
-        const val PBKDF2_KEY_LEN_BITS = 256
-    }
-
     private val prefs: SharedPreferences by lazy {
         context.getSharedPreferences(
             "${this.javaClass.canonicalName}.prefs",

app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt

@@ -23,6 +23,10 @@ import javax.crypto.SecretKeyFactory
 import javax.crypto.spec.PBEKeySpec
 
 object MetricsUtils {
+    // The number of iterations to compute the hash. RFC 2898 suggests
+    // a minimum of 1000 iterations.
+    private const val PBKDF2_ITERATIONS = 1000
+    private const val PBKDF2_KEY_LEN_BITS = 256
 
     /**
      * Possible sources for a performed search.
@@ -57,7 +61,7 @@ object MetricsUtils {
     }
 
     /**
-     * Get the salt to use for hashing. This is a convenience
+     * Get the default salt to use for hashing. This is a convenience
      * function to help with unit tests.
      */
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@@ -77,20 +81,20 @@ object MetricsUtils {
         return try {
             AdvertisingIdClient.getAdvertisingIdInfo(context).id
         } catch (e: GooglePlayServicesNotAvailableException) {
-            Logger.debug("ActivationPing - Google Play not installed on the device")
+            Logger.debug("getAdvertisingID() - Google Play not installed on the device")
             null
         } catch (e: GooglePlayServicesRepairableException) {
-            Logger.debug("ActivationPing - recoverable error connecting to Google Play Services")
+            Logger.debug("getAdvertisingID() - recoverable error connecting to Google Play Services")
             null
         } catch (e: IllegalStateException) {
             // This is unlikely to happen, as this should be running off the main thread.
-            Logger.debug("ActivationPing - AdvertisingIdClient must be called off the main thread")
+            Logger.debug("getAdvertisingID() - AdvertisingIdClient must be called off the main thread")
             null
         } catch (e: IOException) {
-            Logger.debug("ActivationPing - unable to connect to Google Play Services")
+            Logger.debug("getAdvertisingID() - unable to connect to Google Play Services")
             null
         } catch (e: NullPointerException) {
-            Logger.debug("ActivationPing - no Google Advertising ID available")
+            Logger.debug("getAdvertisingID() - no Google Advertising ID available")
             null
         }
     }
@@ -100,46 +104,49 @@ object MetricsUtils {
      * We want users using more than one of our products to report a different
      * ID in each of them. This function runs off the main thread and is CPU-bound.
      *
+     * The `customSalt` parameter allows for dynamic setting of the salt for
+     * certain experiments or any one-off applications.
+     *
      * @return an hashed and salted Google Advertising ID or null if it was not possible
      *         to get the Google Advertising ID.
      */
-    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    suspend fun getHashedIdentifier(context: Context): String? = withContext(Dispatchers.Default) {
-        getAdvertisingID(context)?.let { unhashedID ->
-            // Add some salt to the ID, before hashing. For this specific use-case, it's ok
-            // to use the same salt value for all the hashes. We want hashes to be stable
-            // within a single product, but we don't want hashes to be the same across different
-            // products (e.g. Fennec vs Fenix).
-            val salt = getHashingSalt()
+    suspend fun getHashedIdentifier(context: Context, customSalt: String? = null): String? =
+        withContext(Dispatchers.Default) {
+            getAdvertisingID(context)?.let { unhashedID ->
+                // Add some salt to the ID, before hashing. We have a default salt that is used for
+                // all the hashes unless you specifically provide something different. This is done
+                // to stabalize all hashing within a single product. The customSalt allows for tweaking
+                // in case there are specific use-cases that require something custom.
+                val salt = customSalt ?: getHashingSalt()
 
-            // Apply hashing.
-            try {
-                // Note that we intentionally want to use slow hashing functions here in order
-                // to increase the cost of potentially repeatedly guess the original unhashed
-                // identifier.
-                val keySpec = PBEKeySpec(
-                    unhashedID.toCharArray(),
-                    salt.toByteArray(),
-                    ActivationPing.PBKDF2_ITERATIONS,
-                    ActivationPing.PBKDF2_KEY_LEN_BITS,
-                )
+                // Apply hashing.
+                try {
+                    // Note that we intentionally want to use slow hashing functions here in order
+                    // to increase the cost of potentially repeatedly guess the original unhashed
+                    // identifier.
+                    val keySpec = PBEKeySpec(
+                        unhashedID.toCharArray(),
+                        salt.toByteArray(),
+                        PBKDF2_ITERATIONS,
+                        PBKDF2_KEY_LEN_BITS,
+                    )
 
-                val keyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")
-                val hashedBytes = keyFactory.generateSecret(keySpec).encoded
-                Base64.encodeToString(hashedBytes, Base64.NO_WRAP)
-            } catch (e: java.lang.NullPointerException) {
-                Logger.error("ActivationPing - missing or wrong salt parameter")
-                null
-            } catch (e: IllegalArgumentException) {
-                Logger.error("ActivationPing - wrong parameter", e)
-                null
-            } catch (e: NoSuchAlgorithmException) {
-                Logger.error("ActivationPing - algorithm not available")
-                null
-            } catch (e: InvalidKeySpecException) {
-                Logger.error("ActivationPing - invalid key spec")
-                null
+                    val keyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")
+                    val hashedBytes = keyFactory.generateSecret(keySpec).encoded
+                    Base64.encodeToString(hashedBytes, Base64.NO_WRAP)
+                } catch (e: java.lang.NullPointerException) {
+                    Logger.error("getHashedIdentifier() - missing or wrong salt parameter")
+                    null
+                } catch (e: IllegalArgumentException) {
+                    Logger.error("getHashedIdentifier() - wrong parameter", e)
+                    null
+                } catch (e: NoSuchAlgorithmException) {
+                    Logger.error("getHashedIdentifier() - algorithm not available")
+                    null
+                } catch (e: InvalidKeySpecException) {
+                    Logger.error("getHashedIdentifier() - invalid key spec")
+                    null
+                }
             }
         }
-    }
 }

app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationLifecycleObserver.kt

@@ -0,0 +1,43 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.components.metrics.clientdeduplication
+
+import android.content.Context
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleEventObserver
+import androidx.lifecycle.LifecycleOwner
+import org.mozilla.fenix.nimbus.FxNimbus
+
+/**
+ * This observer allows us to mimic roughly the same schedule as the Glean SDK baseline ping.
+ * https://github.com/mozilla/glean/blob/main/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/GleanLifecycleObserver.kt
+ */
+class ClientDeduplicationLifecycleObserver(context: Context) : LifecycleEventObserver {
+    private val clientDeduplicationPing = ClientDeduplicationPing(context)
+
+    override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) {
+        // The ping will only be sent whenever the Nimbus feature is enabled.
+        if (FxNimbus.features.clientDeduplication.value().enabled) {
+            when (event) {
+                Lifecycle.Event.ON_STOP -> {
+                    clientDeduplicationPing.triggerPing(active = false)
+                }
+                Lifecycle.Event.ON_START -> {
+                    // We use ON_START here because we don't want to incorrectly count metrics in
+                    // ON_RESUME as pause/resume can happen when interacting with things like the
+                    // navigation shade which could lead to incorrectly recording the start of a
+                    // duration, etc.
+                    //
+                    // https://developer.android.com/reference/android/app/Activity.html#onStart()
+
+                    clientDeduplicationPing.triggerPing(active = true)
+                }
+                else -> {
+                    // For other lifecycle events, do nothing
+                }
+            }
+        }
+    }
+}

app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPing.kt

@@ -0,0 +1,50 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.components.metrics.clientdeduplication
+
+import android.content.Context
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import org.mozilla.fenix.GleanMetrics.Activation
+import org.mozilla.fenix.GleanMetrics.ClientDeduplication
+import org.mozilla.fenix.GleanMetrics.Pings
+import org.mozilla.fenix.components.metrics.MetricsUtils.getHashedIdentifier
+
+/**
+ * Class to help construct and send the `clientDeduplication` ping.
+ */
+class ClientDeduplicationPing(private val context: Context) {
+    private val customHashingSalt = "bug-1813195-02-2023"
+
+    /**
+     * Fills the metrics and triggers the 'clientDeduplication' ping.
+     */
+    internal fun triggerPing(active: Boolean) {
+        CoroutineScope(Dispatchers.IO).launch {
+            val hashedId = getHashedIdentifier(context, customHashingSalt)
+
+            // Record the metrics.
+            if (hashedId != null) {
+                // We have a valid, hashed Google Advertising ID.
+                Activation.identifier.set(hashedId)
+                ClientDeduplication.validAdvertisingId.set(true)
+            } else {
+                ClientDeduplication.validAdvertisingId.set(false)
+            }
+            ClientDeduplication.experimentTimeframe.set(customHashingSalt)
+
+            // Set the reason based on if the app is foregrounded or backgrounded.
+            val reason = if (active) {
+                Pings.clientDeduplicationReasonCodes.active
+            } else {
+                Pings.clientDeduplicationReasonCodes.inactive
+            }
+
+            // Submit the ping.
+            Pings.clientDeduplication.submit(reason)
+        }
+    }
+}

app/src/test/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPingTest.kt

@@ -0,0 +1,41 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.components.metrics.clientdeduplication
+
+import mozilla.components.service.glean.testing.GleanTestRule
+import mozilla.components.support.test.robolectric.testContext
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.GleanMetrics.ClientDeduplication
+import org.mozilla.fenix.GleanMetrics.Pings
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+
+// For gleanTestRule
+@RunWith(FenixRobolectricTestRunner::class)
+internal class ClientDeduplicationPingTest {
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
+    @Test
+    fun `The clientDeduplication ping is sent`() {
+        // Record test data.
+        ClientDeduplication.validAdvertisingId.set(true)
+
+        // Instruct the ping API to validate the ping data.
+        var validatorRun = false
+        Pings.clientDeduplication.testBeforeNextSubmit { reason ->
+            assertEquals(Pings.clientDeduplicationReasonCodes.active, reason)
+            assertEquals(true, ClientDeduplication.validAdvertisingId.testGetValue())
+            validatorRun = true
+        }
+        Pings.clientDeduplication.submit(Pings.clientDeduplicationReasonCodes.active)
+
+        // Verify that the validator ran.
+        assertTrue(validatorRun)
+    }
+}

nimbus.fml.yaml

@@ -138,6 +138,21 @@ features:
       - channel: nightly
         value:
           enabled: false
+  
+  client-deduplication:
+    description: A feature to control the sending of the client-deduplication ping.
+    variables:
+      enabled:
+        description: If true, the ping will be sent.
+        type: Boolean
+        default: false
+    defaults:
+      - channel: nightly
+        value:
+          enabled: false
+      - channel: developer
+        value:
+          enabled: false
 
   growth-data:
     description: A feature measuring campaign growth data