1010449 - Implement telemetry measures from bug 760356 for before/after analysis

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[:Irving Reid (No longer working on Firefox)]

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.

Comment 1

[:Irving Reid (No longer working on Firefox)]

Attachment: Telemetry for the add-on compatibility check dialog

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.

Comment 2

[:Irving Reid (No longer working on Firefox)]

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 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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

Comment 4

[:Irving Reid (No longer working on Firefox)]

Attachment: Telemetry for the add-on compatibility check dialog, nits fixed

I agree about renaming the telemetry variables; also removed the executeSoon bits that actually belonged in bug 1007906.

Comment 5

[:Irving Reid (No longer working on Firefox)]

Attachment: Telemetry for the add-on compatibility check dialog, even fixedier

Bah, it helps if I update the telemetry field names in the test case as well as the code. Carrying forward Blair's r+.

Comment 6

[:Irving Reid (No longer working on Firefox)]

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

Comment 7

[:Irving Reid (No longer working on Firefox)]

https://hg.mozilla.org/integration/fx-team/rev/87b5205bab4f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/87b5205bab4f

Comment 9

[:Irving Reid (No longer working on Firefox)]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e79674fe4a79