Closed Bug 1053746 Opened 5 years ago Closed 5 years ago
Add telemetry probes for Open
We should add telemetry probes for: * the installed plugin version * whether the plugin is enabled * (whether autoupdate is enabled for the plugin?)
Should we plan to uplift this to Fx33?
Gavin, can we get this in the next iteration?
Whiteboard: [openh264-uplift] → [openh264-uplift] [lang=js]
The probes for version and enabled state can be added in the OpenH264Provider: http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/OpenH264Provider.jsm We could submit the version using TelemetryLog (see e.g. bug 981851). See here to get started for the other telemetry probe: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe
Hi Georg, this bug is currently part of the IT 35.1 priority sheet. (In reply to Georg Fritzsche [:gfritzsche] from comment #3) > Gavin, can we get this in the next iteration?
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Iteration: --- → 35.1
I don't think this needs automated tests at this point, and mocking that would be pretty hard.
Comment on attachment 8488841 [details] [diff] [review] 1053746-openh264-telemetry Review of attachment 8488841 [details] [diff] [review]: ----------------------------------------------------------------- I don't think its too hard to set up a test for this, see here for a template: http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_openh264.js But given that we want the measurements soon, we could do a follow-up. ::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm @@ +271,5 @@ > + userDisabled: OpenH264Wrapper.userDisabled, > + version: OpenH264Wrapper.version, > + applyBackgroundUpdates: OpenH264Wrapper.applyBackgroundUpdates, > + }; > + AddonManagerPrivate.setTelemetryDetails("GMP", telemetry); Do we only want to update this once on startup? If we always want the latest data in the ping, we would need to trigger submissions from onPrefEnabledChanged(), onPrefVersionChanged() and when the auto-update setting changes.
I don't think we should update this after startup, at least until we've figured out how to deal with telemetry environments.
Substantially the same, except I'm now honoring the provider.enabled pref so that this doesn't show up for non-Firefox apps. I think we should punt automated testing to the bug which will deal with telemetry environment changes.
Attachment #8490274 - Flags: review?(georg.fritzsche)
Attachment #8488841 - Attachment is obsolete: true
(In reply to Benjamin Smedberg [:bsmedberg] from comment #8) > I don't think we should update this after startup, at least until we've > figured out how to deal with telemetry environments. Hm, but given long-running sessions, we will never see them as having OpenH264 installed (nor them updating etc.). Is that really acceptable?
Yes, I think it is. For now telemetry seems to record the state of Firefox at the beginning of the session. If I'm reading the XPIProvider code, that's roughly what happens there also (although it doesn't record the enabled state, and upgrades might change some of the metadata).
Comment on attachment 8490274 [details] [diff] [review] 1053746-openh264-telemetry Review of attachment 8490274 [details] [diff] [review]: ----------------------------------------------------------------- Ok then. I'm not happy about the missing testing though and think we should do a follow-up so we don't accidentally break this.
Attachment #8490274 - Flags: review?(georg.fritzsche) → review+
Looks like this was not landed yet?
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
I'd like to uplift this to 34, but waiting for QA verification first.
Verified with 35.0a1 (2014-10-10) on Mac OSX that I see a new entry in the add-ons section of telemetry for the openH264 add-on, which shows the expected three values and only the values from start-up time of FF. Not setting status verified as I understand there is an uplift coming.
Comment on attachment 8490274 [details] [diff] [review] 1053746-openh264-telemetry Approval Request Comment [Feature/regressing bug #]: OpenH264 is a new feature, this measures OpenH264 install rates [User impact if declined]: we won't know whether OpenH264 is installing propertly in the field [Describe test coverage new/current, TBPL]: manual verification [Risks and why]: I believe this is very low risk [String/UUID change made/needed]: none
Attachment #8490274 - Flags: approval-mozilla-aurora?
Comment on attachment 8490274 [details] [diff] [review] 1053746-openh264-telemetry Aurora+
Attachment #8490274 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified with latest Aurora on Mac OS X 10.9.5, Windows 7 64bit and Ubuntu 14.04 32bit. 'gmp-gmpopenh264' appears in Firefox Health Report together with the three expected values: version, userDisabled, applyBackgroundUpdates. Setting the status of the bug to Verified since it has been verified on 34 and 35 as well.
You need to log in before you can comment on or make changes to this bug.