Closed Bug 1729970 Opened 3 years ago Closed 3 years ago

Add new firefox suggest preferences to clients daily and urlbar clients daily

Categories

(Data Platform and Tools :: General, enhancement, P2)

Product:
Data Platform and Tools
Component:
General
Platform:
Desktop
All
Type:
enhancement

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: rburwei, Assigned: klukas)

References

Details

Attachments

(2 files)

Link to GitHub pull-request: https://github.com/mozilla/bigquery-etl/pull/2332
49 bytes, text/x-github-pull-request
Details | Review
Link to GitHub pull-request: https://github.com/mozilla/bigquery-etl/pull/2336
49 bytes, text/x-github-pull-request
Details | Review

The 3 firefox suggest preferences are listed at the bottom of this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1728430#c2

  • I believe browser.urlbar.quicksuggest.enabled already exists, but is taking on a new functional meaning (i.e., flipping this pref does something different now). Does documentation need to be updated?

  • I believe browser.urlbar.suggest.quicksuggest and browser.urlbar.suggest.quicksuggest.sponsored are new preferences.

We would like to include these preferences in both clients daily and urlbar clients daily.

Teon dropped this PR that might help as a reference: https://github.com/mozilla/bigquery-etl/pull/2304

Assignee: nobody → jklukas
Priority: -- → P2

Teon indicates the specific prefs are:

  • browser.urlbar.suggest.quicksuggest
  • browser.urlbar.suggest.quicksuggest.sponsored

I've looked over this a bit, and I can see these settings in my own Nightly instance in about:config, but I don't think we have any guarantee of seeing these values in telemetry.

Per https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html the userPrefs block of the environment sent in main pings contains only prefs that have changed; furthermore I believe that individual preferences have to be registered to appear in the environment, and I'm not sure that's the case here.

So, if the intention of this bug is to have a field in clients_daily that records the current values of the client's browser.urlbar.suggest.quicksuggest and browser.urlbar.suggest.quicksuggest.sponsored preferences, then I don't believe the telemetry we currently have allows that.

?ni :chutten - Am I correct in understanding that userPrefs in the environment will only contain changes to prefs and that only prefs that are registered will appear? Where would I look to understand whether a given preference is registered to have changes tracked in the telemetry environment?

Flags: needinfo?(chutten)

There are further docs for userPrefs that I had missed: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html#userprefs

The two prefs we're targeting here are not registered in DEFAULT_ENVIRONMENT_PREFS so I don't expect them to be present in the environment.

(In reply to Jeff Klukas [:klukas] (UTC-4) from comment #2)

So, if the intention of this bug is to have a field in clients_daily that records the current values of the client's browser.urlbar.suggest.quicksuggest and browser.urlbar.suggest.quicksuggest.sponsored preferences, then I don't believe the telemetry we currently have allows that.

There is a field in clients_daily called user_pref_browser_search_suggest_enabled. I think that this field records the current value of the client's browser.urlbar.quicksuggest.enabled preferences. Could we do something similar as what is done there? If something needs to be communicated to the engineers implementing the telemetry, I can do that.

(In reply to Jeff Klukas [:klukas] (UTC-4) from comment #4)

The two prefs we're targeting here are not registered in DEFAULT_ENVIRONMENT_PREFS so I don't expect them to be present in the environment.

Is this something I should ask the engineers implementing telemetry to do?

There is a field in clients_daily called user_pref_browser_search_suggest_enabled. I think that this field records the current value of the client's browser.urlbar.quicksuggest.enabled preferences

That field records the value of browser.search.suggest.enabled which is distinct from browser.urlbar.suggest.quicksuggest. The former is listed in DEFAULT_ENVIRONMENT_PREFS:

https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/app/TelemetryEnvironment.jsm#254

I believe a similar entry for each of browser.urlbar.suggest.quicksuggest and browser.urlbar.suggest.quicksuggest.sponsored would unblock us here. I'm hoping that chutten can chime in here to confirm.

Is this something I should ask the engineers implementing telemetry to do?

Yes, I think we should go ahead and ask for these to both be added as prefs to record, using a "value" policy like browser.search.suggest.enabled.

The two prefs we want are:

browser.urlbar.suggest.quicksuggest
browser.urlbar.suggest.quicksuggest.sponsored

We almost certainly don't want browser.urlbar.quicksuggest.enabled because that pref is the feature gate for the whole Firefox Suggest feature. It's set by us, not by users.

Yes, my reference to browser.urlbar.quicksuggest.enabled was a mistake. I've just edited https://bugzilla.mozilla.org/show_bug.cgi?id=1729970#c7 to correct that.

Yes, I can confirm that adding prefs to DEFAULT_ENVIRONMENT_PREFS is how you get them into userPrefs in the Environment. Whether it's the changed value or just the string "(user value)" to denote the changed state, these aim to only show prefs with non-default values to minimize transport and storage costs.

Flags: needinfo?(chutten)
Depends on: 1730721

There's a little bit of complexity I want to clear up:

  • For the offline scenario, the default value of these prefs is true. When the user goes to about:preferences and unchecks them, the prefs will have a "user" value of false. If the user checks them again, they'll go back to having their default true values and they won't have a user value.
  • For the online scenario it's reversed: The default value is false, and the "user" value will be true after the user opts in or checks the checkboxes.

For the telemetry environment, we can record either the user value only or the active value -- "active" meaning the current value regardless of whether it's default or user. That means:

  • If we record the user value only, then:
    • For offline, the prefs will show up in telemetry environment only when the user has disabled suggestions
    • For online, the prefs will show up only when the user has enabled suggestions
  • If we record the active value, then:
    • For both offline and online, the prefs will always show up regardless of their enabled statuses.

I would think we want the active value so that the prefs are present regardless of the rollout or their default values, but most prefs in telemetry environment record only the user value.

Is one way better than the other from your perspective?

Flags: needinfo?(rburwei)
Flags: needinfo?(jklukas)

It sounds to me like it will be difficult to interpret what the value means if we only record the user value, so recording the active value would be idea.

But I don't see recording the active value listed as one of the policies documented in https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html#userprefs . Is there some other mechanism for recording this?

Flags: needinfo?(jklukas)

Thanks for explaining things, Drew! I think we need the active value as well.

Flags: needinfo?(rburwei)

(In reply to Jeff Klukas [:klukas] (UTC-4) from comment #13)

But I don't see recording the active value listed as one of the policies documented in https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html#userprefs . Is there some other mechanism for recording this?

I'm guessing it's termed "value" in that doc, but I'm looking at the source: https://searchfox.org/mozilla-central/rev/fb7c66cb59ccc282aecfe157b05dc12b1e38753f/toolkit/components/telemetry/app/TelemetryEnvironment.jsm#229

The definitions of those consts are here: https://searchfox.org/mozilla-central/rev/fb7c66cb59ccc282aecfe157b05dc12b1e38753f/toolkit/components/telemetry/app/TelemetryEnvironment.jsm#186

I've verified that RECORD_DEFAULTPREF_VALUE indeed records the active value (as the code comment above the definition says) and RECORD_PREF_VALUE records the user value.

I've verified that RECORD_DEFAULTPREF_VALUE indeed records the active value (as the code comment above the definition says) and RECORD_PREF_VALUE records the user value.

+1 to RECORD_DEFAULTPREF_VALUE then.

https://github.com/mozilla/bigquery-etl/pull/2332 is in review to get the fields into clients_daily. That change can get merged independent of :adw's change for how the telemetry is reported.

I'll follow up with a separate PR for urlbar_clients_daily.

The clients_daily change is merged and the new fields exist:

user_pref_browser_urlbar_suggest_quicksuggest
user_pref_browser_urlbar_suggest_quicksuggest_sponsored

https://github.com/mozilla/bigquery-etl/pull/2336 will include those fields in urlbar_clients_daily

The new fields are now present in urlbar_clients_daily as well. We can close this as soon as https://bugzilla.mozilla.org/show_bug.cgi?id=1730721 is closed, which will start populating the values in telemetry.

Marking this as fixed, since 1730721 is now merged.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
See Also: → 1734464
See Also: → 1737374
Component: Datasets: General → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: