Introduce Glean.js for collecting data #505

Dexterp37 · 2021-03-18T11:17:04Z

Fixes #117

This PR adds Glean.js as a dependency and enables documentation and metric code generation at build time, using the 'glean' command.

Important reviewer notes:

Glean.js will always submit data once initialized with upload = true. This only happens after user enrols in Rally.
This PR exclusively stores the rally id as a metric in Glean. Other pings and metrics (to be at feature parity with the current implementation) will happen in a follow-up PR.

TODO:

Add tests.
Initialize Glean.
~~Submit the onboarding ping.~~ (Will happen as a separate PR)
~~Collect study_join_selected from this spreadsheet.~~ (Will happen as a separate PR)
Request a data review. (Will happen after this is reviewed, before merging)
Collect one metric.

Checklist for reviewer:

The description should reference a bug or github issue, if relevant.
There must be a CHANGELOG.md entry for any non-test change.
Any change to the NPM commands must be carefully reviewed to make sure it won't break the Add-ons pipeline.
Any version increase must follow the release process.

Dexterp37 · 2021-03-25T07:05:07Z

This is now blocked by rollup/plugins#843

This ensures that the docs and the metrics code is regenerated when building the core-addon.

This guarantees Glean files get generated.

This fixes the warning (!) Plugin node-resolve: preferring built-in module 'util' over local alternative at '/home/dexter/rally-core-addon/node_modules/util/util.js', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning

This works around rollup/plugins#843. See the documentation: https://github.com/rollup/plugins/tree/master/packages/node-resolve#exportconditions

Glean is disabled by default when building the core-addon. The config-enable-glean option enables Glean. Note that, if Glean is not initialized, no metric recording happens, even if the Glean specific metrics APIs are called.

brizental

From a Glean integration standpoint this LGTM. Very exciting to see this reaching this status. 🎉

Unfortunately this "hack" is required because the add-ons pipeline does not enable consumers to customize the installed packages.

In the spirit of keeping this PR simple, this commit removes the pings definition file.

marniepw · 2021-04-06T22:27:53Z

@rhelmer just a reminder about this one...thanks!

rhelmer

lgtm! I assume this is so simple because glean.js is doing most of the heavy lifting (well, and those bits aren't implemented that you mentioned in the top comment).

rhelmer · 2021-04-06T22:44:00Z

scripts/setupTaskcluster.js

+import { promisify } from "util";
+
+// Define an async/await version of "exec".
+const execAsync = promisify(exec.exec);


Huh, why do you want this to be async? Doesn't seem like it matters here, and it's not being await'd or anything.

No real reason, it was copy 🍝 !

Dexterp37 · 2021-04-08T13:01:00Z

Request for data collection review form

All questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection.

What questions will you answer with this data?

This PR does not add any new data collection to the Core Add-on. Instead, it introduces Glean.js and stores the "rally id" in Glean.js.

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:

Establish baselines or measure changes in product or platform quality or performance.

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

Data collection currently happens using legacy telemetry in Firefox. We're migrating to Glean.

Can current instrumentation answer these questions?

Yes, but we need to migrate to Glean.js to further evolve Rally.

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

This PR does not add any new data point. The Rally id/Pioneer id was already data-reviewed for Pioneer.

Please provide a link to the documentation for this data collection which describes the ultimate data set in a public, complete, and accurate way.

Documentation is generated by Glean and included in this PR.

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

I want to permanently monitor this data. Ted Han (than@mozilla.com) will own the data (and the whole rally team).

What populations will you measure?

All the populations currently covered by the Rally policy.

If this data collection is default on, what is the opt-out mechanism for users?

Collection is only enabled after user enrolls in Rally after onboarding.

Please provide a general description of how you will analyze this data.

Data will be analysed via notebooks to understand how the core addon behaves in the wild.

Where do you intend to share the results of your analysis?

With key partners through their relative secure environments.

Is there a third-party tool (i.e. not Telemetry) that you are proposing to use for this data collection? If so:

No. Data will be stored and analysed using internal tools.

chutten · 2021-04-08T13:30:46Z

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is part of Rally which can be turned on and off.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Ted Han is responsible.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Mixed categories. This is the addition of a data collection system (Glean.js) not a specific data collection.

Is the data collection request for default-on or default-off?

Default off. Rally requires opt-in.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

Result: datareview+

Dexterp37 force-pushed the gleanjs branch 2 times, most recently from 5fcf66e to 5d84744 Compare March 22, 2021 14:26

Dexterp37 self-assigned this Mar 25, 2021

Dexterp37 added 9 commits March 26, 2021 16:05

Make sure Python is available on CI

097fa4e

Update the .gitignore to exclude Glean stuff

f589103

Add Glean.js to Rally

0651a20

Generate Glean artifacts when building Rally

0965aba

This ensures that the docs and the metrics code is regenerated when building the core-addon.

Initialize Glean and enable ping encryption

645494a

Make sure to build the addon before running tests

913bce5

This guarantees Glean files get generated.

Resolve imports exported using "browser"

f9ffdef

This works around rollup/plugins#843. See the documentation: https://github.com/rollup/plugins/tree/master/packages/node-resolve#exportconditions

Add the 'config-enable-glean' Rollup option

1c0c112

Glean is disabled by default when building the core-addon. The config-enable-glean option enables Glean. Note that, if Glean is not initialized, no metric recording happens, even if the Glean specific metrics APIs are called.

Dexterp37 force-pushed the gleanjs branch from e46d5ea to 0d913ed Compare March 26, 2021 15:09

Dexterp37 requested a review from brizental March 26, 2021 15:19

Dexterp37 marked this pull request as ready for review March 26, 2021 15:19

brizental approved these changes Mar 26, 2021

View reviewed changes

Dexterp37 added 2 commits March 26, 2021 17:08

Add a simple test for the Rally id in Glean

99ea753

Add a CHANGELOG entry

ab3c0f3

Dexterp37 force-pushed the gleanjs branch from 0d913ed to ab3c0f3 Compare March 26, 2021 16:09

Ensure the right dependencies are available on TaskCluster

d37be7c

Unfortunately this "hack" is required because the add-ons pipeline does not enable consumers to customize the installed packages.

Dexterp37 force-pushed the gleanjs branch from 3c015d3 to d37be7c Compare March 29, 2021 07:44

Dexterp37 closed this Apr 1, 2021

Dexterp37 reopened this Apr 1, 2021

Remove pings.yaml

8955cb3

In the spirit of keeping this PR simple, this commit removes the pings definition file.

Dexterp37 requested a review from rhelmer April 4, 2021 14:03

rhelmer approved these changes Apr 6, 2021

View reviewed changes

Do not use async exec in setupTaskcluster.js

0a2c9b7

Dexterp37 merged commit a397fd8 into mozilla-rally:master Apr 8, 2021

Dexterp37 deleted the gleanjs branch April 8, 2021 12:53

Dexterp37 mentioned this pull request Apr 8, 2021

Add the missing data review request info #533

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Glean.js for collecting data #505

Introduce Glean.js for collecting data #505

Dexterp37 commented Mar 18, 2021 •

edited by rhelmer

Dexterp37 commented Mar 25, 2021

brizental left a comment

marniepw commented Apr 6, 2021

rhelmer left a comment

rhelmer Apr 6, 2021

Dexterp37 Apr 8, 2021

Dexterp37 commented Apr 8, 2021

chutten commented Apr 8, 2021

Introduce Glean.js for collecting data #505

Introduce Glean.js for collecting data #505

Conversation

Dexterp37 commented Mar 18, 2021 • edited by rhelmer

Dexterp37 commented Mar 25, 2021

brizental left a comment

Choose a reason for hiding this comment

marniepw commented Apr 6, 2021

rhelmer left a comment

Choose a reason for hiding this comment

rhelmer Apr 6, 2021

Choose a reason for hiding this comment

Dexterp37 Apr 8, 2021

Choose a reason for hiding this comment

Dexterp37 commented Apr 8, 2021

Request for data collection review form

chutten commented Apr 8, 2021

Dexterp37 commented Mar 18, 2021 •

edited by rhelmer