1386959 - [Form Autofill] Add probe for form autofill availability

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P1)

Description

[Steve Chung [:steveck]]

We should be able to know whether the feature is available or not after bug 1385196 landed. PMs would like to know the number of available users.

Comment 1

[Steve Chung [:steveck]]

Attachment: Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Review commit: https://reviewboard.mozilla.org/r/168128/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168128/

Comment 2

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review173282

::: browser/extensions/formautofill/bootstrap.js:71
(Diff revision 1)
>    if (!isAvailable()) {
>      Services.prefs.clearUserPref("dom.forms.autocomplete.formautofill");
> +    Services.telemetry.scalarSet("formautofill.availability", false);
>      return;
>    }
> +  Services.telemetry.scalarSet("formautofill.availability", true);

nit: put this line below `Services.prefs.setBoolPref("dom.forms.autocomplete.formautofill", true);`.

Comment 3

[Steve Chung [:steveck]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168128/diff/1-2/

Comment 4

[Steve Chung [:steveck]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168128/diff/2-3/

Comment 5

[François Marier [:francois]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174182

::: toolkit/components/telemetry/Scalars.yaml:793
(Diff revision 3)
>  # The following section contains the form autofill related scalars.
> +formautofill:
> +  availability:
> +    bug_numbers:
> +      - 1386959
> +    description: A boolean to represent whether the formautofill is available

Can you expand on this description a little?

- Is it sent everytime you encounter a form on a page?
- Does a form need to contain a field of a particular type (e.g. text field) for this signal to get sent?
- Is it sent more than once per top-level page load (if there's more than one form on the page)?
- Is it sent if the formautofill pref is OFF?

::: toolkit/components/telemetry/Scalars.yaml:794
(Diff revision 3)
> +formautofill:
> +  availability:
> +    bug_numbers:
> +      - 1386959
> +    description: A boolean to represent whether the formautofill is available
> +    expires: never

This may send a lot of pings back to our servers.

Would it be reasonable to start with an expiry in say 6 releases and then renew the probe if we're still using the data?

Comment 6

[Matthew N. [:MattN]]

(In reply to François Marier [:francois] from comment #5)
> Comment on attachment 8896833 [details]
> Bug 1386959 - [Form Autofill] Add probe for form autofill availability.
> 
> https://reviewboard.mozilla.org/r/168128/#review174182
> 
> ::: toolkit/components/telemetry/Scalars.yaml:793
> (Diff revision 3)
> >  # The following section contains the form autofill related scalars.
> > +formautofill:
> > +  availability:
> > +    bug_numbers:
> > +      - 1386959
> > +    description: A boolean to represent whether the formautofill is available
> 
> Can you expand on this description a little?
> 
> - Is it sent everytime you encounter a form on a page?
> - Does a form need to contain a field of a particular type (e.g. text field)
> for this signal to get sent?
> - Is it sent more than once per top-level page load (if there's more than
> one form on the page)?
> - Is it sent if the formautofill pref is OFF?

This is only recorded once per session to indicate if the form autofill feature is available in the build. We are rolling it out by locale and geolocation so we'll use this to filter out telemetry to look at users who actually have the feature.

> ::: toolkit/components/telemetry/Scalars.yaml:794
> (Diff revision 3)
> > +formautofill:
> > +  availability:
> > +    bug_numbers:
> > +      - 1386959
> > +    description: A boolean to represent whether the formautofill is available
> > +    expires: never
> 
> This may send a lot of pings back to our servers.
> 
> Would it be reasonable to start with an expiry in say 6 releases and then
> renew the probe if we're still using the data?

Since it's only once per session I guess this would no longer be a concern?

Comment 7

[François Marier [:francois]]

(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #6)
> This is only recorded once per session to indicate if the form autofill
> feature is available in the build. We are rolling it out by locale and
> geolocation so we'll use this to filter out telemetry to look at users who
> actually have the feature.

Ok great. Maybe we can rephrase the description to something like:

  A boolean sent once per session to represent whether the formautofill is available in the build

> > Would it be reasonable to start with an expiry in say 6 releases and then
> > renew the probe if we're still using the data?
> 
> Since it's only once per session I guess this would no longer be a concern?

Yes that's fine. You can ignore that comment.

Comment 8

[Steve Chung [:steveck]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168128/diff/3-4/

Comment 9

[Steve Chung [:steveck]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174182

> Can you expand on this description a little?
> 
> - Is it sent everytime you encounter a form on a page?
> - Does a form need to contain a field of a particular type (e.g. text field) for this signal to get sent?
> - Is it sent more than once per top-level page load (if there's more than one form on the page)?
> - Is it sent if the formautofill pref is OFF?

Description is rephrased per your suggestion, thanks.

Comment 10

[François Marier [:francois]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174774

datareview+

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/837485049120
[Form Autofill] Add probe for form autofill availability. r=francois,lchang

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/837485049120

Comment 13

[Matthew N. [:MattN]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Approval Request Comment
[Feature/Bug causing the regression]: Form autofill
[User impact if declined]: There will be no reliable way to know how many users have autofill available (en-US + US geo.) which will affect stats we can check
[Is this code covered by automated tests?]: No, simple telemetry probe
[Has the fix been verified in Nightly?]: No, not necessary since it's trivial
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple telemetry probe
[String changes made/needed]: None

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Add telemetry for autofill. Beta56+.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/72f3e9b7c6ea

Comment 16

[Andrei Vaida [:avaida]]

(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #13)
> [Is this code covered by automated tests?]: No, simple telemetry probe
> [Has the fix been verified in Nightly?]: No, not necessary since it's trivial
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Matthew's assessment on manual testing needs.