Send a generic health telemetry ping through Ping Centre for Activity Stream

VERIFIED FIXED in Firefox 57

Status

()

VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: emtwo, Assigned: emtwo, NeedInfo)

Tracking

unspecified
Firefox 58
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox57 verified, firefox58 verified)

Details

Attachments

(1 attachment)

Updated

2 years ago
Depends on: 1403215
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8913216 - Flags: review?(khudson)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8913216 [details]
Bug 1403695: Send a generic health telemetry ping through Ping Centre for Activity Stream.

https://reviewboard.mozilla.org/r/184612/#review190126

::: browser/components/nsBrowserGlue.js:308
(Diff revision 2)
> +  /**
> +   * Lazily initialize PingCentre
> +   */
> +  get pingCentre() {
> +    const MAIN_TOPIC_ID = "main";
> +    Object.defineProperty(this, "_pingCentre", {

You should either cache this value (_pingCentre) or just redefine pingCentre (it's fine to do the lalter)
Attachment #8913216 - Flags: review?(khudson) → review-
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8913216 [details]
Bug 1403695: Send a generic health telemetry ping through Ping Centre for Activity Stream.

https://reviewboard.mozilla.org/r/184612/#review190140

Great, thanks!
Attachment #8913216 - Flags: review?(khudson) → review+

Comment 8

2 years ago
Pushed by msamuel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b56649e9680
Send a generic health telemetry ping through Ping Centre for Activity Stream. r=k88hudson
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years ago
Reftests and talos tests needed adjusted endpoints for the tests. Updated and rerunning try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa15dd89be484db13fc90be3402d454e4e10525e
Flags: needinfo?(msamuel)

Comment 13

2 years ago
Pushed by msamuel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/698b17a7805c
Send a generic health telemetry ping through Ping Centre for Activity Stream. r=k88hudson

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/698b17a7805c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
(Assignee)

Comment 15

2 years ago
Comment on attachment 8913216 [details]
Bug 1403695: Send a generic health telemetry ping through Ping Centre for Activity Stream.

Approval Request Comment:

[Feature/Bug causing the regression]: This is a ping to verify that Activity Stream initializes healthily. We realized this would be a good idea initially while debugging https://bugzilla.mozilla.org/show_bug.cgi?id=1399936. It helps us to get a base count for the number of initializations we expect to see and that lets us understand if activity stream is operating healthily both inside and out of experiments.
 
[User impact if declined]: We may have difficulty detecting and responding to bugs in Activity Stream quickly.

[Is this code covered by automated tests?]: Yes (Included in this changeset)

[Has the fix been verified in Nightly?]: Yes I have verified this change myself in Nightly.

[Needs manual test from QE? If yes, steps to reproduce]:
    1) Make sure `browser.ping-centre.log` is set to true.
    2) Look for a log in browser console similar to:

TELEMETRY PING: {"locale":"en-US","topic":"main","client_id":"aa0f3e12-530f-bf45-8403-83ce7ddc6258","version":"58.0a1","release_channel":"nightly","event":"AS_ENABLED","value”:true}

Note: This should only occur after shield has been initialized. Usually it is after startup is complete.

[List of other uplifts needed for the feature/fix]: n/a

[Is the change risky?]: No

[Why is the change risky/not risky?]: It's adding a bit of telemetry using an existing system that's being used by Activity Stream already.

[String changes made/needed]: n/a
Attachment #8913216 - Flags: approval-mozilla-beta?
status-firefox57: --- → affected
Comment on attachment 8913216 [details]
Bug 1403695: Send a generic health telemetry ping through Ping Centre for Activity Stream.

Since this is still early part of Beta57 cycle, I'll take it. This commit (non-test changes) seems big and normally carries a decent bit of risk. A similar patch a week from now will unfortunately get rejected. Beta57+
Flags: needinfo?(pdehaan)
Flags: needinfo?(edilee)
Attachment #8913216 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
FYI Ed, Pete. Thanks!

Comment 18

2 years ago
I wonder if there should be a separate place for Ping Centre bugs now. Although the volume is currently small and could maybe just be as part of some existing component. Doesn't quite fit under "Firefox Health Report / *" or "Mozilla Metrics / *" or "Toolkit / Telemetry". I suppose "Firefox / Activity Streams: Newtab" for now works until it gets migrated to just "Firefox / New Tab Page" and at that point, maybe Ping Centre stuff defaults to "Firefox / General" ?
Flags: needinfo?(edilee)

Comment 19

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/3d3fb2366110
status-firefox57: affected → fixed
Flags: in-testsuite+
(Assignee)

Comment 20

2 years ago
:Mardak I agree with you that it would be nice to separate it somewhere else and that when Activity Stream becomes "Firefox / New Tab Page" it might be a good time to separate this out as well.
Investigated this on 58.0a1 (2017-10-09) and 57.0b7 build 1 (20171009192146), using Windows 10 x64, macOS 10.12.1, Ubuntu 16.04 x64 and the steps provided in comment 15. The browser.ping-centre.log pref is set on false by default (Marina, is this expected?). But after setting its value on true, the above mentioned telemetry log is successfully displayed in Browser Console.
Flags: needinfo?(msamuel)
(Assignee)

Comment 22

a year ago
Yes. browser.ping-centre.log pref is false by default. If you saw the logging after turning that on then it's working as expected.
Flags: needinfo?(msamuel)
Based on comment 21 and comment 22, I will mark this as verified fixed for the above mentioned builds.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
status-firefox58: fixed → verified
You need to log in before you can comment on or make changes to this bug.