Closed Bug 1412029 Opened 7 years ago Closed 7 years ago

Instrument interactions on the onboarding page

Categories

(DevTools :: General, enhancement, P2)

enhancement

Tracking

(firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: Harald, Assigned: jdescottes)

References

Details

Attachments

(3 files)

To properly run the experiment we need to instrument:

- Count how many users see the page and how they triggered it and which panel they wanted to open
- Count how many close the dialog without installing
- Count how many click Install
Georg, this is for an experiment. Could we use evented telemetry for this to dissect the user journey through the flow?
Flags: needinfo?(gfritzsche)
Quick question:
What is the timeline for this?
Will this experiment run as a SHIELD study?
Do you have an analyst available for this?

In general, you can use Event Telemetry to instrument this. However, the analysis tooling might be lacking, depending on what exactly you need.
The Activity Stream teams Ping Centre might be another option if their tooling fits.
Flags: needinfo?(gfritzsche) → needinfo?(hkirschner)
> What is the timeline for this?

58 Beta, so very soon. We would not block on event telemetry or setting up a new ping mechanism that requires laying new pipes.
Flags: needinfo?(hkirschner)
(In reply to :Harald Kirschner :digitarald from comment #0)
> To properly run the experiment we need to instrument:
> 
> - Count how many users see the page and how they triggered it and which
> panel they wanted to open
> - Count how many close the dialog without installing
> - Count how many click Install

I'm implementing the probes at the moment.

So far I have 3 scalars:
- devtools.aboutdevtools.opened -> logged every time the page is loaded
- devtools.aboutdevtools.installed -> logged when the user clicks on the install button
- devtools.aboutdevtools.noinstall_exits -> logged when leaving the page if devtools is not enabled

I also have 2 keyed histograms to check "how they triggered it and which panel they wanted to open"
- DEVTOOLS_ABOUT_DEVTOOLS_OPENED_REASON (is it a shortcut, is it a menu item etc...)
- DEVTOOLS_ABOUT_DEVTOOLS_OPENED_KEY (in case of a keyshortcut, what is the keyshortcut id)

As you can see it's not exactly panel-based but key-based, because I based this on my patches in Bug 1412028. I can map them before logging to telemetry if you feel that it will make it easier.

Regarding additional data points, would it be interesting to collect the time spent on the page (and maybe separating users that enable devtools from those who don't?).
Flags: needinfo?(hkirschner)
All outgoing links should have a UTM campaign: https://ga-dev-tools.appspot.com/campaign-url-builder/

Campaign Source: devtools
Campaign Medium: onboarding

Arcadio: Does that work for marketing?
Flags: needinfo?(hkirschner) → needinfo?(alainez)
Telemetry LGTM
(In reply to :Harald Kirschner :digitarald from comment #5)
> All outgoing links should have a UTM campaign:
> https://ga-dev-tools.appspot.com/campaign-url-builder/
> 
> Campaign Source: devtools
> Campaign Medium: onboarding
> 
> Arcadio: Does that work for marketing?

This works! Thank you
Flags: needinfo?(alainez)
Priority: -- → P2
Comment on attachment 8924669 [details]
Bug 1412029 - pass key shortcut id to aboutdevtools page;

https://reviewboard.mozilla.org/r/195900/#review201418

::: devtools/shim/devtools-startup.js:462
(Diff revision 1)
>      let doc = window.document;
>      let keyset = doc.createElement("keyset");
>      keyset.setAttribute("id", "devtoolsKeyset");
>  
>      for (let key of KeyShortcuts) {
> -      let xulKey = this.createKey(doc, key, () => this.onKey(window, key));
> +      let keyId = key.toolId || key.id;

Rather than converting this into a string ID and then re-fetching the key from KeyShortcuts later on in onKey, I'd rather that we keep passing it as `key` and then pass `key.toolId || key.id` as the parameter into `openInstallPage`

::: devtools/shim/devtools-startup.js:485
(Diff revision 1)
> +   * @param {Window} window
> +   *        the window where the command should be executed (command will most likely
> +   *        apply to the selected tab).
> +   */
> +  onKey(keyId, window) {
> +    if (!Services.prefs.getBoolPref(DEVTOOLS_ENABLED_PREF)) {

Is this `if` always equivalent to the case where `require` was null before?
Attachment #8924669 - Flags: review?(bgrinstead)
Comment on attachment 8924670 [details]
Bug 1412029 - add probes to about:devtools;,datareview=francois

https://reviewboard.mozilla.org/r/195902/#review201428

::: devtools/shim/aboutdevtools/aboutdevtools.js:95
(Diff revision 1)
>  
>    // Update the current page based on the current value of DEVTOOLS_ENABLED_PREF.
>    updatePage();
> +
> +  try {
> +    Services.telemetry.getHistogramById(TELEMETRY_OPENED_REASON).add(reason);

What's our intended behavior here if someone just loads `about:devtools`?  Should we null check reason and keyid and not log, log a different string, something else?
Attachment #8924670 - Flags: review?(bgrinstead) → review+
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8924670 [details]
Bug 1412029 - add probes to about:devtools;,datareview=francois

adding :francois for data-review
Attachment #8924670 - Flags: review?(francois)
Francois: see comment 4 for a description of our probes. Our goal is to understand how users land on about:devtools and how many decide to enable devtools vs those who decide to leave the page and keep DevTools disabled.
There is now a new process to follow for all new telemetry probes: https://wiki.mozilla.org/Firefox/Data_Collection#Step_1:_Submit_Request

Essentially, you need to copy and fill out the questionnaire and then attach it to the bug. I can review that file afterwards and I'll take care of completing Step 2.
Attachment #8924670 - Flags: review?(francois)
Comment on attachment 8924669 [details]
Bug 1412029 - pass key shortcut id to aboutdevtools page;

https://reviewboard.mozilla.org/r/195900/#review201418

> Rather than converting this into a string ID and then re-fetching the key from KeyShortcuts later on in onKey, I'd rather that we keep passing it as `key` and then pass `key.toolId || key.id` as the parameter into `openInstallPage`

Done!

> Is this `if` always equivalent to the case where `require` was null before?

Yes the only reason we added the if null was because of the onboarding flow. 

Ultimately I would like to stop starting the onboarding flow from initDevTools. With the current situation, initDevTools can return require or null, which is a pain for the call sites. I'd rather have initDevTools throw if DevTools are not enabled and let each call site decide how they want to handle the case where DevTools is disabled.
Comment on attachment 8924670 [details]
Bug 1412029 - add probes to about:devtools;,datareview=francois

https://reviewboard.mozilla.org/r/195902/#review201428

> What's our intended behavior here if someone just loads `about:devtools`?  Should we null check reason and keyid and not log, log a different string, something else?

I went for the first approach: guard the log with `if (key/reason)`. I don't think telemetry would log anything if the category passed was not registered but now it's more explicit.
Comment on attachment 8924669 [details]
Bug 1412029 - pass key shortcut id to aboutdevtools page;

https://reviewboard.mozilla.org/r/195900/#review201638
Attachment #8924669 - Flags: review?(bgrinstead) → review+
Attached file request-bug1412029.md
Attachment #8925177 - Flags: review?(francois)
Comment on attachment 8924670 [details]
Bug 1412029 - add probes to about:devtools;,datareview=francois

https://reviewboard.mozilla.org/r/195902/#review202064

::: toolkit/components/telemetry/Scalars.yaml:933
(Diff revision 2)
> +    description: >
> +      Number of times about:devtools was opened.
> +    expires: "60"
> +    kind: uint
> +    notification_emails:
> +      - dev-developer-tools@lists.mozilla.org

Please add your own email address here. Having a team alias is fine, but we now also ask for the email of a person to be included with all new probes.

::: toolkit/components/telemetry/Scalars.yaml:944
(Diff revision 2)
> +    description: >
> +      Number of times devtools were enabled/installed in about:devtools.
> +    expires: "60"
> +    kind: uint
> +    notification_emails:
> +      - dev-developer-tools@lists.mozilla.org

ditto

::: toolkit/components/telemetry/Scalars.yaml:955
(Diff revision 2)
> +    description: >
> +      Number of times the user left about:devtools without enabling devtools.
> +    expires: "60"
> +    kind: uint
> +    notification_emails:
> +      - dev-developer-tools@lists.mozilla.org

ditto
Comment on attachment 8925177 [details]
request-bug1412029.md

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate? (see [here](https://github.com/mozilla/activity-stream/blob/master/docs/v2-system-addon/data_dictionary.md), [here](https://github.com/mozilla-mobile/focus/wiki/Install-and-event-tracking-with-the-Adjust-SDK), and [here](https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/index.html) for examples).  Refer to the appendix for "documentation" if more detail about documentation standards is needed.

Yes, in telemetry.

2) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Yes, telemetry control.

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

Not permanent.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1.

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

Default-on in pre-release channels.

6) 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)?

No.

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

Yes.

8) 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)**

No, telemetry alerts are enough.
Attachment #8925177 - Flags: review?(francois) → review+
Comment on attachment 8924670 [details]
Bug 1412029 - add probes to about:devtools;,datareview=francois

https://reviewboard.mozilla.org/r/195902/#review202064

Thanks for the review! Added my email as requested.
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3a84e128c3c
pass key shortcut id to aboutdevtools page;r=bgrins
https://hg.mozilla.org/integration/autoland/rev/ab41b0060a87
add probes to about:devtools;r=bgrins,datareview=francois
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06538a745211
Backed out 2 changesets for ESLint failure "/devtools/shim/aboutdevtools/aboutdevtools.js" r=backout on a CLOSED TREE
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8fc199eb8a4
pass key shortcut id to aboutdevtools page;r=bgrins
https://hg.mozilla.org/integration/autoland/rev/6ddd17e8d5c4
add probes to about:devtools;r=bgrins,datareview=francois
https://hg.mozilla.org/mozilla-central/rev/b8fc199eb8a4
https://hg.mozilla.org/mozilla-central/rev/6ddd17e8d5c4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: