The default bug view has changed. See this FAQ.

Consider making FX_URLBAR_SELECTED_RESULT_TYPE opt-out

RESOLVED FIXED in Firefox 53

Status

()

Firefox
Location Bar
P1
normal
RESOLVED FIXED
2 months ago
10 days ago

People

(Reporter: past, Assigned: past)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [fxsearch])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

It would be useful to make FX_URLBAR_SELECTED_RESULT_TYPE opt-out, if the data privacy review approves it. It doesn't contain any PII in my view.
This one is top priority

Updated

2 months ago
Priority: -- → P1
Assignee: nobody → past
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Benjamin, to give you some more context, I'm making two changes here:

(a) removing the expiration from FX_URLBAR_SELECTED_RESULT_INDEX
(b) making both that and FX_URLBAR_SELECTED_RESULT_TYPE opt-out

I have read your comments in bug 775825 where you had argued against (a), but we now have a dedicated data analyst tasked to create dashboards for us and it's one of the top-level goals for both the search team and BD to monitor the awesomebar results and use them to improve their value for our users.

In addition to the above, another argument for (b) is that it doesn't contain any PII, is going to be super useful for improving the awesomebar and Javaun tells me that it is covered by the data policy documentation Rebecca is putting together as something that is acceptable to have opt-out.

Comment 4

29 days ago
Javaun and I had a discussion about how the utility of the selected result index measurement could be improved if it could be associated with result type.

Could we possibly turn FX_URLBAR_SELECTED_RESULT_INDEX into a keyed histogram, keyed by the values for result type, ie. URLBAR_SELECTED_RESULT_TYPES (https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUsageTelemetry.jsm#56)? We weren't sure what additional engineering effort or considerations that would entail.
Flags: needinfo?(past)
I'm happy to do this, but I wonder if repurposing an existing histogram will break our pipeline. Georg, should we create a new keyed histogram in this case or reuse the existing one? If a new one is necessary, should we keep the old one around (perhaps for a short period of time) or delete it?
Flags: needinfo?(past) → needinfo?(gfritzsche)
Changing histograms while keeping the same name is not supported, see [1].
Whether to keep the old definition around depends on the users. Removing it will make it disappear from e.g. TMO dashboards.

1: http://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/collection/histograms.html#changing-a-histogram
Flags: needinfo?(gfritzsche)
Attachment #8842469 - Flags: review?(benjamin)
Javaun, could you state for the record that you will be the long-term owner of looking at this data?
Flags: needinfo?(jmoradi)
note that this caused a perma failure with bug 1345134 - so i guess this should be fixed soon here

Updated

23 days ago
Blocks: 1345134
Duplicate of this bug: 1345134
I'll push a temporary patch to comment out the parts of the test that refer to the expired probe.
I just need time for a build and running the test locally to ensure it works.

Comment 11

23 days ago
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/mozilla-central/rev/8718138ada82
Temporarily disable tests for FX_URLBAR_SELECTED_RESULT_INDEX since it's expired. rs=Dexter, landing on central with a=tomcat

Comment 12

23 days ago
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/mozilla-central/rev/3d341b9ba535
followup - Eslint loves commented code. a=tomcat
Note: All the temporary disabling I'm doing should be undone by the patch that will land here.
Keywords: leave-open
BSmedberg: I'll be the long-term owner of this bug. I'll talk w/ DZeber about doing the data review paperwork. I'm eager to do my first so I can learn.
Flags: needinfo?(jmoradi)
You don't need the complex new-project paperwork. When the patch is ready here I can mark the data review.
It's surprisingly hard for me to find time to do the work suggested in comment 4, and since Javaun agreed that we need both the existing histograms and the new one, I will file a followup for comment 4 and use this bug for its original purpose.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8845384 - Attachment is obsolete: true
Attachment #8845384 - Flags: review?(benjamin)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
It was also surprisingly hard for me to figure out the proper syntax for 'git mozreview push', but YOLO!
Blocks: 1345834
Is it possible to have this uplifted to 53? I realize we're in Beta now. A funnelcake is being discussed and this would be a critical probe to understand some behavior they're testing in that test.

Comment 24

20 days ago
mozreview-review
Comment on attachment 8842469 [details]
Make the URL bar selected result telemetry probes opt-out (bug 1334617).

https://reviewboard.mozilla.org/r/116298/#review121134

data-r+
Attachment #8842469 - Flags: review?(benjamin) → review+

Comment 25

20 days ago
mozreview-review
Comment on attachment 8845385 [details]
Revert "Bug 1334617 followup - Eslint loves commented code. a=tomcat"

https://reviewboard.mozilla.org/r/118590/#review121136

This doesn't look like a data review. This is not my code so I don't want to do code review on this.
Attachment #8845385 - Flags: review?(benjamin)

Comment 26

20 days ago
mozreview-review
Comment on attachment 8845386 [details]
Revert "Bug 1334617 - Temporarily disable tests for FX_URLBAR_SELECTED_RESULT_INDEX since it's expired. rs=Dexter, landing on central with a=tomcat"

https://reviewboard.mozilla.org/r/118592/#review121138
Attachment #8845386 - Flags: review?(benjamin)
Attachment #8845385 - Flags: review?(mak77)
Attachment #8845386 - Flags: review?(mak77)

Comment 27

17 days ago
mozreview-review
Comment on attachment 8845385 [details]
Revert "Bug 1334617 followup - Eslint loves commented code. a=tomcat"

https://reviewboard.mozilla.org/r/118590/#review121590
Attachment #8845385 - Flags: review?(mak77) → review+

Comment 28

17 days ago
mozreview-review
Comment on attachment 8845386 [details]
Revert "Bug 1334617 - Temporarily disable tests for FX_URLBAR_SELECTED_RESULT_INDEX since it's expired. rs=Dexter, landing on central with a=tomcat"

https://reviewboard.mozilla.org/r/118592/#review121592
Attachment #8845386 - Flags: review?(mak77) → review+

Comment 29

17 days ago
Pushed by pastithas@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/536676c30456
Make the URL bar selected result telemetry probes opt-out . r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/59d830905c61
Revert "Bug 1334617 followup - Eslint loves commented code. a=tomcat" r=mak
https://hg.mozilla.org/integration/autoland/rev/3862c0c5350c
Revert "Bug 1334617 - Temporarily disable tests for FX_URLBAR_SELECTED_RESULT_INDEX since it's expired. rs=Dexter, landing on central with a=tomcat" r=mak
Keywords: leave-open
These changes landed, but somehow the bug wasn't updated:
https://hg.mozilla.org/mozilla-central/rev/536676c30456
https://hg.mozilla.org/mozilla-central/rev/59d830905c61
https://hg.mozilla.org/mozilla-central/rev/3862c0c5350c
Status: ASSIGNED → RESOLVED
Last Resolved: 14 days ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
status-firefox55: --- → fixed
Comment on attachment 8842469 [details]
Make the URL bar selected result telemetry probes opt-out (bug 1334617).

Approval Request Comment
[Feature/Bug causing the regression]: none
[User impact if declined]: no user impact, but we count on these telemetry probes to inform planned implementation work
[Is this code covered by automated tests?]: yes and it was briefly broken in nightly, before being properly fixed by this patch
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none, the other two patches aren't necessary in other branches
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a small configuration change in our data collection
[String changes made/needed]: none
Attachment #8842469 - Flags: approval-mozilla-beta?
Attachment #8842469 - Flags: approval-mozilla-aurora?
Comment on attachment 8842469 [details]
Make the URL bar selected result telemetry probes opt-out (bug 1334617).

Has data review+, sounds like it will be useful for the product team.  Let's bring it to 53.
Attachment #8842469 - Flags: approval-mozilla-beta?
Attachment #8842469 - Flags: approval-mozilla-beta+
Attachment #8842469 - Flags: approval-mozilla-aurora?
Attachment #8842469 - Flags: approval-mozilla-aurora+

Comment 33

13 days ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/471cc7315f1e
status-firefox54: affected → fixed

Comment 34

13 days ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/6694dac889dd
status-firefox53: --- → fixed
Setting qe-verify- based on Panos' assessment on manual testing needs (see Comment 31) and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.