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.

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)?

(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.

https://hg.mozilla.org/integration/fx-team/rev/e0e28f569c29 Filed bug 1007647 for the timestamp issue.

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