Generate the `activation` ping and send it with Glean #1707

Dexterp37 · 2019-04-17T17:41:17Z

This fetches the Google Advertising ID, salts it and then applies hashing before sending a ping with it,
at startup. Hashing and salting are used in order to prevent ourselves to correlate advertising IDs
from the same user running different products we own off a single device. We will never send the
client_id and the Google Advertising ID in the same ping: we only send the client_id as a backup in case
the Google Advertising ID is not available.

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

colintheshots · 2019-04-18T13:05:50Z

Does the hashing algorithm require more CPU or disk I/O time? The IO dispatcher is more appropriate for disk. The CommonPool dispatcher, which is default, can be undersized on older devices with fewer cores and may deadlock. I don't know if they fixed this yet.

Dexterp37 · 2019-04-18T13:09:00Z

CommonPool

I suspect this is CPU-bound, there should be no I/O involved in this (other than the SharedPreferences, which is already taken care of by its API, I think).

colintheshots · 2019-04-18T13:10:11Z

Launch, if you never join() on the job it returns, is fire and forget. It ignores exceptions and doesn't return a value. This could be what you intend, but it's important to be aware of it.

Dexterp37 · 2019-04-18T13:14:42Z

Launch, if you never join() on the job it returns, is fire and forget. It ignores exceptions and doesn't return a value. This could be what you intend, but it's important to be aware of it.

Good point, yes, I think this is matching the behaviour I want, which is the following:

Fenix starts;
Glean inits;
Check if the ping was already generated;
If not, attempt to generate the required data and send the ping (off-the main thread, async);
4a. If something throws (and I forgot to catch and print some useful message), if the job dies for some reason, markAsTriggered is not called and a new collection is attempted at the next startup
4b. if all goes ok, markAsTriggered will prevent the ping to be sent again next time it starts.

I really intend a "fire and forget off-the-main thread" behaviour, while attempting to catch weird behaviours.

colintheshots · 2019-04-18T15:45:54Z

This is failing due to lint style checks:
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:71:16: Missing spacing after "catch"
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:113:1: Needless blank line(s)
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:138:58: Missing space after //

Dexterp37 · 2019-04-18T15:47:47Z

This is failing due to lint style checks:
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:71:16: Missing spacing after "catch"
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:113:1: Needless blank line(s)
/bitrise/src/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt:138:58: Missing space after //

Yeah, sorry about that, this is still a draft as I'm waiting for some additional implementation details to come first. I'll complete it tomorrow/next week.

liuche · 2019-04-19T23:55:02Z

Oops, this is a PR so I'm moving this comment to the Issue:
Is this something that will be restricted in who can use it? I'm concerned that including this in Glean makes it seem like this is telemetry that is "approved for inclusion" by any consumer apps, but in this case, the GAID activation ping should only be used with approval.
Maybe the thing to do here though is to add this warning/caveat in the usage docs.

mdboom

LGTM.

My only concern (which you also sort of hinted at), is that we don't have a good way of checking if the ping was actually sent. Would it be worth adding such a thing to the Glean API?

mdboom · 2019-04-23T13:58:37Z

app/metrics.yaml

@@ -539,3 +539,36 @@ custom_tab:
    notification_emails:
    - fenix-core@mozilla.com
    expires: "2020-03-01"
+
+activation.ping:


Is the .ping strictly necessary? Feels a bit redundant, but it might just be me...

You're right. Removing that.

mdboom · 2019-04-23T13:59:21Z

app/metrics.yaml

+    description: >
+      An hashed and salted version of the Google Advertising ID from the device.
+    send_in_pings:
+    - activation


über-nit: Inconsistent indentation between this and the metric below.

mdboom · 2019-04-23T14:04:37Z

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

+
+            Logger.info("ActivationPing - generating ping (has `identifier`: ${hashedId != null})")
+            // TODO: change-me
+            Glean.sendPings(listOf("activation")) //, sendClientId=false)


You know this already, but this will change once mozilla-mobile/android-components#2792 lands

Dexterp37 · 2019-04-24T08:48:43Z

My only concern (which you also sort of hinted at), is that we don't have a good way of checking if the ping was actually sent. Would it be worth adding such a thing to the Glean API?

@mdboom "On this side of the barricade", we're the application: we shouldn't really care about the transport itself, we should trust Glean to do the right thing for us. We should also be explicit, especially if we write the docs, about the expectations :) Calling sendPing doesn't imply the ping is sent "right away", but "as soon as possible" (e.g. there might be no network). But the app shouldn't really care about that.

mdboom · 2019-04-24T11:54:37Z

Yeah, thinking about this further, I think it's fine as long as Glean can guarantee that if a ping is queued it gets sent someday (and that's kind of on Glean to ensure). My concern was that this has code to check the ping is only sent once. If that first attempt fails somehow, it will never get sent, which would be a bad outcome.

Dexterp37 · 2019-04-24T18:51:29Z

@liuche I'd appreciate if you could review the documentation here (the activation.md file), the metrics.yaml and pings.yaml changes, given your knowledge of the background for this ping.

@colintheshots are you the right person to review this fully? If not, any chance you could kindly redirect the review? This is introducing a new custom ping for Fenix, which should be generated only once on new installs and never ever again. See the included docs.

@mdboom could you please review my usage of the Glean APIs (and the docs too, for clarity :) )?

Dexterp37 · 2019-04-24T18:53:12Z

app/metrics.yaml

+    bugs:
+    - 1538011
+    data_reviews:
+    - TODO


Note: this need to be updated with a link to the review.

Dexterp37 · 2019-04-24T18:57:28Z

app/metrics.yaml

+    bugs:
+    - 1538011
+    data_reviews:
+    - TODO


@liuche how can I make the data review available? Should it be copied over the related Fenix issue or in my implementation bug on bugzilla?

@Dexterp37 you can copy the data request here, and I can re-review now that the implementation is also here! I'll flag myself for review here since you mentioned you don't have perms.

mdboom

🚢

mdboom · 2019-04-24T18:56:20Z

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

+     * Checks whether or not the activation ping was already
+     * triggered by the application.
+     *
+     * Note that this only tells us that Fenix did trigger the


Suggested change

* Note that this only tells us that Fenix did trigger the

* Note that this only tells us that Fenix triggered the

mdboom · 2019-04-24T19:00:33Z

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

+
+            Logger.info("ActivationPing - generating ping (has `identifier`: ${hashedId != null})")
+            Pings.activation.send()
+            markAsTriggered()


Maybe move this to the top of the launch block to avoid the (unlikely) race condition where calling checkAndSend twice quickly could send two pings?

I'd prefer leaving this as it is: if, for some weird reason, we crash/get killed after that line gets executed, we'd never re-attempt to send the ping. We can easily detect dupes via the hash, on the other hand. So two pings shouldn't be a big deal in this specific case.

Dexterp37 · 2019-04-24T19:04:06Z

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

+            // Apply hashing.
+            try {
+                val saltedID = unhashedID + salt
+                val digest = MessageDigest.getInstance("SHA-256")


Is there any chance we could use something else other than this? Is there anything else available for us to use (we really want bcrypt here) in the Fenix repo or Android API?

badboy

As promised in chat, here's a quick review of the documentation only.
(I did not look at any code).

badboy · 2019-04-24T20:38:08Z

docs/activation.md

+This ping is intended to provide a measure of the activation of mobile products.
+
+## Scheduling
+The `activation` ping is automatically sent at startup, after Glean is initialized, and contains


Suggested change

The `activation` ping is automatically sent at startup, after Glean is initialized, and contains

The `activation` ping is automatically sent at first startup, after Glean is initialized, and contains

We should make this clear this ping is sent only once, only during the very first startup (or during subsequent startups if it hasn't been sent yet?).

badboy · 2019-04-24T20:38:44Z

docs/activation.md

+# The `activation` ping
+
+## Description
+This ping is intended to provide a measure of the activation of mobile products.


Suggested change

This ping is intended to provide a measure of the activation of mobile products.

This ping provides a measure of the activation of mobile products.

"intended to" sounds so passive. It is sent and it is as reliable as other pings.

badboy · 2019-04-24T20:40:14Z

docs/activation.md

+
+## Scheduling
+The `activation` ping is automatically sent at startup, after Glean is initialized, and contains
+the following fields:


Don't mention this in the scheduling section? It is its own section anyway.

badboy · 2019-04-24T20:42:10Z

docs/activation.md

+| `identifier` | String | An hashed and salted version of the Google Advertising ID from the device. |
+| `activation_id` | UUID | An alternate identifier, not correlated with the client_id, generated once and only sent with the activation ping. |
+
+The `activation` ping also includes the common [ping sections]https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/docs/pings/pings.md#ping-sections)


Suggested change

The `activation` ping also includes the common [ping sections]https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/docs/pings/pings.md#ping-sections)

The `activation` ping also includes the common [ping sections](https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/docs/pings/pings.md#ping-sections)

badboy · 2019-04-24T20:44:16Z

docs/telemetry.md

@@ -14,6 +14,10 @@ Fenix creates and tries to send a "baseline" ping. It is defined inside the [`me

 Fenix sends event pings that allows us to measure feature performance. These are defined inside the [`metrics.yaml`](https://github.com/mozilla-mobile/fenix/blob/master/app/metrics.yaml) file.

+## Activation
+
+Fenix sends an activation ping once, at startup. The ping is documented [`here`](activation.md) file.


Suggested change

Fenix sends an activation ping once, at startup. The ping is documented [`here`](activation.md) file.

Fenix sends an activation ping once, at startup. Further documentation can be found in [Activation Ping](activation.md).

liuche

Documentation looks good to me with one nit.
Can you post the data request here and I'll fill it out again here so it's public and associated with the code.

liuche · 2019-04-25T00:04:28Z

app/metrics.yaml

+    type: string
+    lifetime: ping
+    description: >
+      An hashed and salted version of the Google Advertising ID from the device.


Please also include here that this is never sent with the client id. Also document the case where this is the client id.

I've addressed the first request. We are no longer sending the client_id, not even when the GAID is not available. In that case, we're generating a random UUID (the activation_id), which will be stable throughout the life of the application, and exclusively sent with this ping.

Dexterp37 · 2019-04-26T08:22:30Z

data-review? @liuche

1) What questions will you answer with this data?

We plan to count the exact number of activations, per distribution, per manufacturer.

2) Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? Some example responses:

The BD team needs an exact method for counting these activations across manufacturers.

The activation data will also help us evaluate the difference in activations from the other data sources which are currently being used for activation counts (e.g. adjust)

3) What alternative methods did you consider to answer these questions? Why were they not sufficient?

We tried many alternatives. For example, client-ids gave more clients than device shipments (see bug 1481215). There can be a variety of reasons for this, outlined in the bug. We don’t include distribution information in other data sources, like adjust, so we couldn’t count using those.

4) Can current instrumentation answer these questions?
For the reasons listed above, no.

5) List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories on the Mozilla wiki.

Measurement Description	Data Collection Category	Tracking Information
activation.identifier	Category 4	Bug 1501822
activation.activation_id	Category 4

Glean pings also include metadata which is being sent with this ping as well, with the exclusion of the client_id.

Additional documentation for this ping is provided as part of this PR.

6) How long will this data be collected? Choose one of the following:

I want this data to be collected for 6 months initially. We will probably renew when BD needs it, possible on additional mobile products.

7) What populations will you measure?

Which release channels?
All channels.
Which countries?
All countries.
Which locales?
All locales.
Any other filters? Please describe in detail below.
Initially only Fenix.

8) If this data collection is default on, what is the opt-out mechanism for users?
There is none.

9) Please provide a general description of how you will analyze this data.
Per-distribution and per-manufacturer, we will count the number of unique (hashed) Google Ad ID. Some metadata (e.g. model) can be used to stratify the results.

10) Where do you intend to share the results of your analysis?
This will be used internally by BD. They can do the analysis in re:dash, locked down to the search datasets if necessary.

liuche

Data Review Form (to be filled by Data Stewards)

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?
Documentation in metrics.yaml, and an additional activation.md file what is being collected in this activation ping, and under what circumstances.
Is there a control mechanism that allows the user to turn the data collection on and off?
No, this ping is sent at activation. This is separate from any client_id, and is used to measure activations only for partnership distribution versions in order to count activations through a distribution, which is necessary for BD.

If the answer to either of the first two questions is no, reviewers give an r-. Incremental changes to measurements or systems that have previously gone through analysis review may not require additional review.

This is only for partner distributions, and we've gone through an additional approval process for this specific use case - this ping is totally isolated from client ID and is necessary for counting Fenix activations in distributions.

If the request is for permanent data collection, is there someone who will monitor the data over time?**
6 months, arana will monitor this from the BD side.
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? **
Type 4 - Google Ad ID
Is the data collection request for default-on or default-off?
Default on
Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?
Yes, the hashed + salted GAID, and a randomly generated id sent only once for disambiguation
Is the data collection covered by the existing Firefox privacy notice? If unsure: escalate to legal if:

The data includes new identifiers; OR
The data falls within the Web activity category AND is default-on.

Yes, we have discussed this with Marshall and this data is approved to be collected in this specific use case of partner distributions of Fenix.

Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**
Yes, this probe will expire
Does the data collection use a third-party collection tool? If yes, escalate to legal.
No

travis79

I really like how the pings.yaml has turned out for documentation purposes.

colintheshots · 2019-04-30T19:11:02Z

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

+        getAdvertisingID()?.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


FYI: The packageName is not stable between Fenix Beta and Fenix Nightly, etc. They're treated as different products.

Good point. @fbertsch, is this ok? Or should we use a Fenix-specific salt (regardless of the channel), e.g. org.mozilla.fenix?

We probably want the same salt across all the Fenix products, to reduce double counting.

Thanks, fixed this by providing a static salt.

colintheshots · 2019-04-30T19:14:25Z

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

+        // Generate the activation_id.
+        Activation.activationId.generateAndSet()
+
+        CoroutineScope(Dispatchers.Default).launch {


Note that calling launch without joining on its job will fire-and-forget. Exceptions will be swallowed and there's no guarantee it ever completes. This might be your intent, but you should be aware.

Thanks, yes, this is intended behaviour: we should be fine to swallow exceptions here and re-attempt, if something went wrong, next time we start.

Dexterp37 · 2019-05-02T09:18:59Z

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

@@ -195,9 +198,9 @@ class GleanMetricsService(private val context: Context) : MetricsService {
                code.set(defaultEngine.identifier)
                name.set(defaultEngine.name)
                submissionUrl.set(defaultEngine.buildSearchUrl(""))
-
-                Glean.setUploadEnabled(true)


@hawkinsw I noticed that Glean.setUploadEnabled was moved here instead of being called before Glean.initialize in #2088 . I'm moving it back in this PR, since this is the right usage of the API. Was there a specific reason to move it here? Maybe some bug on the Glean end we need to fix?

Nope! Thanks for chatting about this offline and thank you for fixing it!

This fetches the Google Advertising ID, salts it and then applies hashing before sending a ping with it, at startup. Hashing and salting are used in order to prevent ourselves to correlate advertising IDs from the same user running different products we own off a single device. We will never send the client_id and the Google Advertising ID in the same ping.

ghost assigned Dexterp37 Apr 17, 2019

ghost added the review label Apr 17, 2019

Dexterp37 force-pushed the activation_ping branch from 4294bab to 1c33297 Compare April 18, 2019 10:39

Dexterp37 mentioned this pull request Apr 18, 2019

Send Mobile Activation Ping telemetry #1028

Closed

colintheshots approved these changes Apr 18, 2019

View reviewed changes

liuche mentioned this pull request Apr 20, 2019

Mobile Activation Ping telemetry sender mozilla-mobile/android-components#2397

Closed

Dexterp37 force-pushed the activation_ping branch from 1c33297 to 5c13a65 Compare April 23, 2019 13:55

mdboom reviewed Apr 23, 2019

View reviewed changes

Dexterp37 force-pushed the activation_ping branch from 5c13a65 to 9d6343e Compare April 24, 2019 14:48

Dexterp37 marked this pull request as ready for review April 24, 2019 18:37

Dexterp37 requested a review from a team as a code owner April 24, 2019 18:37

Dexterp37 force-pushed the activation_ping branch from 9d6343e to f9c124c Compare April 24, 2019 18:44

Dexterp37 commented Apr 24, 2019

View reviewed changes

mdboom approved these changes Apr 24, 2019

View reviewed changes

Dexterp37 commented Apr 24, 2019

View reviewed changes

badboy suggested changes Apr 24, 2019

View reviewed changes

liuche self-requested a review April 25, 2019 00:02

liuche approved these changes Apr 25, 2019

View reviewed changes

Dexterp37 force-pushed the activation_ping branch 2 times, most recently from e12ce0b to 44558d1 Compare April 29, 2019 08:56

Dexterp37 requested a review from colintheshots April 29, 2019 13:41

liuche approved these changes Apr 29, 2019

View reviewed changes

travis79 approved these changes Apr 30, 2019

View reviewed changes

colintheshots approved these changes Apr 30, 2019

View reviewed changes

Dexterp37 force-pushed the activation_ping branch from 79c6f85 to 4564edb Compare May 2, 2019 09:13

Dexterp37 commented May 2, 2019

View reviewed changes

Dexterp37 force-pushed the activation_ping branch from 4564edb to 0a17a41 Compare May 2, 2019 13:53

badboy approved these changes May 3, 2019

View reviewed changes

colintheshots merged commit 0d82431 into mozilla-mobile:master May 3, 2019

liuche mentioned this pull request Oct 28, 2019

Closes #6097 - Bump activation ping expiry date. #6309

Merged

5 tasks

This was referenced Aug 20, 2020

Review telemetry probes and formally renew them #12046

Closed

For #12046 - Renew telemetry probes before migration completion #13958

Merged

mcarare mentioned this pull request Feb 24, 2021

For #18076: Renew telemetry expiring in April. #18143

Merged

3 tasks

jonalmeida mentioned this pull request Aug 6, 2021

Issue #4901: Update metrics for data review on initial metrics mozilla-mobile/focus-android#5065

Merged

jonalmeida mentioned this pull request Aug 16, 2021

Issue #4901: Add activation ping metrics for data review mozilla-mobile/focus-android#5148

Merged

eliserichards mentioned this pull request Oct 22, 2021

For #21313: Renew metrics for December - never expire updates #21788

Merged

6 tasks

	* Note that this only tells us that Fenix did trigger the
	* Note that this only tells us that Fenix triggered the

	The `activation` ping is automatically sent at startup, after Glean is initialized, and contains
	The `activation` ping is automatically sent at first startup, after Glean is initialized, and contains

	This ping is intended to provide a measure of the activation of mobile products.
	This ping provides a measure of the activation of mobile products.

	The `activation` ping also includes the common [ping sections]https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/docs/pings/pings.md#ping-sections)
	The `activation` ping also includes the common [ping sections](https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/docs/pings/pings.md#ping-sections)

	Fenix sends an activation ping once, at startup. The ping is documented [`here`](activation.md) file.
	Fenix sends an activation ping once, at startup. Further documentation can be found in [Activation Ping](activation.md).

Generate the activation ping and send it with Glean #1707

Generate the activation ping and send it with Glean #1707

Conversation

Dexterp37 commented Apr 17, 2019

Pull Request checklist

colintheshots commented Apr 18, 2019

Dexterp37 commented Apr 18, 2019

colintheshots commented Apr 18, 2019

Dexterp37 commented Apr 18, 2019

colintheshots commented Apr 18, 2019

Dexterp37 commented Apr 18, 2019

liuche commented Apr 19, 2019 • edited Loading

mdboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dexterp37 commented Apr 24, 2019

mdboom commented Apr 24, 2019

Dexterp37 commented Apr 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

badboy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dexterp37 commented Apr 26, 2019

liuche left a comment

Choose a reason for hiding this comment

Data Review Form (to be filled by Data Stewards)

travis79 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Generate the `activation` ping and send it with Glean #1707

Generate the `activation` ping and send it with Glean #1707

liuche commented Apr 19, 2019 •

edited

Loading