Closed Bug 1403695 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: New Tab Page, defect)

Product:
Firefox
Component:
New Tab Page
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: emtwo, Assigned: emtwo, NeedInfo)

References

Details

Attachments

(1 file)

Bug 1403695: Send a generic health telemetry ping through Ping Centre for Activity Stream.
59 bytes, text/x-review-board-request
k88hudson
: review+
ritu
: approval-mozilla-beta+
Details
Depends on: 1403215
Attachment #8913216 - Flags: review?(khudson)
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 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+
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
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)
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
https://hg.mozilla.org/mozilla-central/rev/698b17a7805c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
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?
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+
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)
: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)
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
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: