Implement telemetry measures from bug 760356 for before/after analysis

RESOLVED FIXED in Firefox 31

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Irving, Assigned: Irving)

Tracking

unspecified
mozilla32
Points:
---

Firefox Tracking Flags

(firefox31 fixed, firefox32 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 2 obsolete attachments)

Bug 760356 adds a few telemetry measurements to see how many add-ons have their compatibility changed or are upgraded during the "new version" add-on compatibility check.

In order to do before/after comparison, we want a minimal patch that takes the same measurements against the existing implementation, suitable for uplift so that it lands one release ahead of bug 760356.
Oddly enough, writing tests for this shook out at least one mistaken assumption; I'll need to update bug 760356 both for the bitrot caused by this, and for the logic difference.
Attachment #8423391 - Flags: review?(bmcbride)
Comment on attachment 8423391 [details] [diff] [review]
Telemetry for the add-on compatibility check dialog

I just realized that I'm not sure this always removes the AddonListener; you can review this version if you want, but there's a good chance I'll update the patch soon...
Comment on attachment 8423391 [details] [diff] [review]
Telemetry for the add-on compatibility check dialog

Review of attachment 8423391 [details] [diff] [review]:
-----------------------------------------------------------------

Thinking about before/after, would be nice to also have a measure of how often the update dialog shows after bug 760356. We can't accurately infer that with the current measures - as even the measures that are always recorded in the update dialog itself won't be recorded if you cancel the dialog early enough. Another followup bug?

::: toolkit/mozapps/extensions/content/update.js
@@ +225,5 @@
>    },
>  
>    onAllUpdatesFinished: function gVersionInfoPage_onAllUpdatesFinished() {
> +    AddonManager.removeAddonListener(listener);
> +    AddonManagerPrivate.recordSimpleMeasure("XPIDB_disabled",

Hm, thinking about this more, I think we should name these differently for the update dialog, to make them easily distinguishable. Something like "appUpdate_XXX"?
Attachment #8423391 - Flags: review?(bmcbride) → review+
I agree about renaming the telemetry variables; also removed the executeSoon bits that actually belonged in bug 1007906.
Attachment #8423391 - Attachment is obsolete: true
Attachment #8424510 - Flags: review?(bmcbride)
Attachment #8424510 - Flags: review?(bmcbride) → review+
Bah, it helps if I update the telemetry field names in the test case as well as the code. Carrying forward Blair's r+.
Attachment #8424510 - Attachment is obsolete: true
Attachment #8425135 - Flags: review+
(In reply to Blair McBride [:Unfocused] from comment #3)
> Comment on attachment 8423391 [details] [diff] [review]
> Telemetry for the add-on compatibility check dialog
> 
> Review of attachment 8423391 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thinking about before/after, would be nice to also have a measure of how
> often the update dialog shows after bug 760356. We can't accurately infer
> that with the current measures - as even the measures that are always
> recorded in the update dialog itself won't be recorded if you cancel the
> dialog early enough. Another followup bug?

SIMPLE_MEASURES_STARTUPINTERRUPTED gives us that - we get 1 if the dialog was displayed at all (it counts if we get either the compat-check dialog *or* the selection dialog, but we expect nearly none of them to be the selection dialog)
https://hg.mozilla.org/mozilla-central/rev/87b5205bab4f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla32
Comment on attachment 8425135 [details] [diff] [review]
Telemetry for the add-on compatibility check dialog, even fixedier

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 760356
User impact if declined: This patch adds telemetry to measure the effect of fixing bug 760356; we'd like to uplift it to get before/after data as Beta and Release go through the process of upgrading Firefox version.
Testing completed (on m-c, etc.): Code has been on Nightly for 10 days, output is showing up in Telemetry.
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #8425135 - Flags: approval-mozilla-aurora?
status-firefox31: --- → affected
status-firefox32: --- → fixed
Attachment #8425135 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.