1815706 - Fix history sidebar telemetry and add total clicks count

Status: VERIFIED FIXED

(Firefox :: Firefox View, task, P1)

Description

[Sarah Clements [:sclements]]

I noticed a bug with how the cumulative histogram telemetry was implemented in bug 1801290 a few months ago. If you click anywhere on the sidebar, it triggers the onclick handler, updates the histograms and clears the counters.

This needs to be fixed so that the histograms are only updated when a history entry is clicked.

Additionally, Leif suggested we add a counter for total history clicks in a session, which we don't currently have.

Comment 1

[Sarah Clements [:sclements]]

Attachment: Bug 1815706 - Update history sidebar telemetry r=mak,mkaply

• fix sidebar click event handling
• Add scalar for total number of history entries per session
• Add test for scalar

Comment 2

[Sarah Clements [:sclements]]

Attachment: Firefox View Data Collection Request - additional Sidebar Telemetry probe

Comment 3

[Chris H-C :chutten]

Comment on attachment 9317332 [details]
Firefox View Data Collection Request - additional Sidebar Telemetry probe

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 can be controlled through Firefox's Preferences.

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

Yes, John Hall is responsible.

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

Category 2, Interaction.

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

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

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

Yes.

Does the data collection use a third-party collection tool?

No.

---

Result: datareview+

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b26a56aab365
Update history sidebar telemetry r=mak,kcochrane

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on browser_sidebar_history_telemetry.js

• backout: https://hg.mozilla.org/integration/autoland/rev/02449d7e49332c534a95afe7def303ba1fcfaff3
• push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=PnABZl-MQIOnkiVVQdR2tQ.0&revision=b26a56aab365ebd5590a8145f82d11340defe87f&group_state=expanded
• failure log: https://treeherder.mozilla.org/logviewer?job_id=406073543&repo=autoland&lineNumber=7447

[task 2023-02-17T19:43:36.371Z] 19:43:36     INFO - TEST-PASS | browser/components/places/tests/browser/browser_sidebar_history_telemetry.js | sidebar.link must contain the 'history' key. - true == true - 
[task 2023-02-17T19:43:36.372Z] 19:43:36     INFO - Buffered messages finished
[task 2023-02-17T19:43:36.374Z] 19:43:36     INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_sidebar_history_telemetry.js | sidebar.link['history'] must contain the expected value - 5 == 4 - {"filename":"resource://testing-common/TelemetryTestUtils.sys.mjs","name":"assertKeyedScalar","sourceId":1585,"lineNumber":47,"columnNumber":12,"sourceLine":"","asyncCause":null,"asyncCaller":null,"caller":{"filename":"chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_sidebar_history_telemetry.js","name":"test_click_multiple_history_entries/<","sourceId":2344,"lineNumber":84,"columnNumber":24,"sourceLine":"","asyncCause":null,"asyncCaller":null,"caller":null,"formattedStack":"test_click_multiple_history_entries/<@chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_sidebar_history_telemetry.js:84:24\n","nativeSavedFrame":{}},"formattedStack":"assertKeyedScalar@resource://testing-common/TelemetryTestUtils.sys.mjs:47:12\ntest_click_multiple_history_entries/<@chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_sidebar_history_telemetry.js:84:24\n","nativeSavedFrame":{}}
[task 2023-02-17T19:43:36.374Z] 19:43:36     INFO - Stack trace:
[task 2023-02-17T19:43:36.374Z] 19:43:36     INFO - resource://testing-common/TelemetryTestUtils.sys.mjs:assertKeyedScalar:47
[task 2023-02-17T19:43:36.374Z] 19:43:36     INFO - chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_sidebar_history_telemetry.js:test_click_multiple_history_entries/<:84
[task 2023-02-17T19:43:36.406Z] 19:43:36     INFO - Leaving test bound test_click_multiple_history_entries
[task 2023-02-17T19:43:36.408Z] 19:43:36     INFO - Entering test bound test_search_and_filter
[task 2023-02-17T19:43:36.409Z] 19:43:36     INFO - withSidebarTree: waiting sidebar load
[task 2023-02-17T19:43:36.565Z] 19:43:36     INFO - GECKO(6984) | JavaScript error: resource://devtools/client/jsonview/Sniffer.sys.mjs, line 50: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIChannel.contentType]
[task 2023-02-17T19:43:36.592Z] 19:43:36     INFO - Console message: [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIChannel.contentType]" {file: "resource://devtools/client/jsonview/Sniffer.sys.mjs" line: 50}]
[task 2023-02-17T19:43:36.592Z] 19:43:36     INFO - getMIMETypeFromContent@resource://devtools/client/jsonview/Sniffer.sys.mjs:50:13
[task 2023-02-17T19:43:36.592Z] 19:43:36     INFO - 

Marco, can you take a look at this since Sarah is away until the 27th?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99ac48311f47
Update history sidebar telemetry r=mak,mkaply

Comment 7

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/99ac48311f47

Comment 8

[Kelly Cochrane [:kcochrane]]

Comment on attachment 9317067 [details]
Bug 1815706 - Update history sidebar telemetry r=mak,mkaply

Beta/Release Uplift Approval Request

• User impact if declined: Telemetry for the History sidebar will be recorded somewhat improperly and will not be as robust.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Open history sidebar using View > Sidebar > History. Ensure telemetry is recorded properly for cumulative searches within the History sidebar before opening a history link(s) from that same sidebar. This can be viewed as a Histogram "PLACES_SEARCHBAR_CUMULATIVE_SEARCHES". Also ensure that telemetry is recorded each time one of these history links is opened from the sidebar (using a new keyed scalar for "sidebar.link" with the name "history").
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Updates existing telemetry and adds a new keyed scalar. New and existing telemetry is covered by automated tests.
• String changes made/needed: N/A
• Is Android affected?: No

Comment 9

[Donal Meehan [:dmeehan]]

Comment on attachment 9317067 [details]
Bug 1815706 - Update history sidebar telemetry r=mak,mkaply

Approved for 111.0b5

Comment 10

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cc5d4b96b0dc

Comment 11

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! Reproduced the initial issue with Firefox 111.0b4 on Windows 10x64. Clicking anywhere on the sidebar will increase the PLACES_SEARCHBAR_CUMULATIVE_SEARCHES and PLACES_SEARCHBAR_CUMULATIVE_FILTER_COUNT histogram numbers.

The above histograms are no longer increased by clicking on the sidebar with Firefox 111.0b5 from comment 10 and Firefox 112.0a1 (2023-02-23) on Windows 10x64, macOS 11, and Ubuntu 20.04. Also the new keyed scalar for sidebar.link > history number is correctly increased each time a history link is clicked inside the history sidebar.