Closed Bug 1007095 Opened 10 years ago Closed 10 years ago

Add UI telemetry for Reader actions

Categories

(Firefox for Android Graveyard :: Reader View, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox31 fixed, firefox32 fixed)

RESOLVED FIXED
Firefox 32
Tracking Status
firefox31 --- fixed
firefox32 --- fixed

People

(Reporter: mfinkle, Unassigned)

References

Details

Attachments

(1 file)

Attached patch uitelemetry-readeractions v0.1 (deleted) — Splinter Review
Add some probes for the Reader toolbar actions. I used "action.1" since this seemed very similar to the App mainmenu and Home contextmenu probes. The button IDs are passed as etras, just like the mainmenu R.id.xxx values are passed as extras.
Attachment #8418711 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8418711 [details] [diff] [review] uitelemetry-readeractions v0.1 Review of attachment 8418711 [details] [diff] [review]: ----------------------------------------------------------------- Giving f+ to get question answered before r+. ::: mobile/android/chrome/content/aboutReader.js @@ +707,5 @@ > + // Create a relative timestamp for telemetry > + let uptime = Date.now() - Services.startup.getStartupInfo().linkerInitialized; > + // Just pass the ID of the button as an extra and hope the ID doesn't change > + // unless the context changes > + UITelemetry.addEvent("action.1", "button", uptime, id); Where is the id coming from? I don't see it being defined anywhere. Maybe this uptime argument should simply default to (Date.now() - Services.startup.getStartupInfo().linkerInitialized)?
Attachment #8418711 - Flags: review?(lucasr.at.mozilla) → feedback+
(In reply to Lucas Rocha (:lucasr) from comment #1) > Comment on attachment 8418711 [details] [diff] [review] > uitelemetry-readeractions v0.1 > > Review of attachment 8418711 [details] [diff] [review]: > ----------------------------------------------------------------- > > Giving f+ to get question answered before r+. > > ::: mobile/android/chrome/content/aboutReader.js > @@ +707,5 @@ > > + // Create a relative timestamp for telemetry > > + let uptime = Date.now() - Services.startup.getStartupInfo().linkerInitialized; > > + // Just pass the ID of the button as an extra and hope the ID doesn't change > > + // unless the context changes > > + UITelemetry.addEvent("action.1", "button", uptime, id); > > Where is the id coming from? I don't see it being defined anywhere. It's passed into the function: http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/aboutReader.js#677 > Maybe this uptime argument should simply default to (Date.now() - > Services.startup.getStartupInfo().linkerInitialized)? I was going to file a bug to explore that. The code pattern is duplicated a lot and will only keep happening.
Note that we also have a "reader.1" Session active when these "action" Events are fired, so we have more context about them.
Comment on attachment 8418711 [details] [diff] [review] uitelemetry-readeractions v0.1 Review of attachment 8418711 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Mark Finkle (:mfinkle) from comment #2) > (In reply to Lucas Rocha (:lucasr) from comment #1) > It's passed into the function: > http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/ > aboutReader.js#677 Ah, ok. > > Maybe this uptime argument should simply default to (Date.now() - > > Services.startup.getStartupInfo().linkerInitialized)? > > I was going to file a bug to explore that. The code pattern is duplicated a > lot and will only keep happening. Please file a follow-up.
Attachment #8418711 - Flags: feedback+ → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Comment on attachment 8418711 [details] [diff] [review] uitelemetry-readeractions v0.1 [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: We need more before and after telemetry data Testing completed (on m-c, etc.): Working fine on m-c Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none
Attachment #8418711 - Flags: approval-mozilla-aurora?
Attachment #8418711 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: