Last Comment Bug 1010449 - Implement telemetry measures from bug 760356 for before/after analysis
: Implement telemetry measures from bug 760356 for before/after analysis
Status: RESOLVED FIXED
[qa-]
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla32
Assigned To: :Irving Reid (No longer working on Firefox)
:
: Andy McKay [:andym]
Mentors:
Depends on:
Blocks: 760356
  Show dependency treegraph
 
Reported: 2014-05-14 12:18 PDT by :Irving Reid (No longer working on Firefox)
Modified: 2014-06-20 16:11 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Telemetry for the add-on compatibility check dialog (22.50 KB, patch)
2014-05-15 13:43 PDT, :Irving Reid (No longer working on Firefox)
blair: review+
Details | Diff | Splinter Review
Telemetry for the add-on compatibility check dialog, nits fixed (16.24 KB, patch)
2014-05-18 15:17 PDT, :Irving Reid (No longer working on Firefox)
blair: review+
Details | Diff | Splinter Review
Telemetry for the add-on compatibility check dialog, even fixedier (16.26 KB, patch)
2014-05-19 16:31 PDT, :Irving Reid (No longer working on Firefox)
irving: review+
sledru: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image :Irving Reid (No longer working on Firefox) 2014-05-14 12:18:32 PDT
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 User image :Irving Reid (No longer working on Firefox) 2014-05-15 13:43:31 PDT
Created attachment 8423391 [details] [diff] [review]
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 User image :Irving Reid (No longer working on Firefox) 2014-05-15 13:52:38 PDT
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 User image Blair McBride [:Unfocused] (UNAVAILABLE) 2014-05-15 20:20:06 PDT
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 User image :Irving Reid (No longer working on Firefox) 2014-05-18 15:17:30 PDT
Created attachment 8424510 [details] [diff] [review]
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 User image :Irving Reid (No longer working on Firefox) 2014-05-19 16:31:13 PDT
Created attachment 8425135 [details] [diff] [review]
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 User image :Irving Reid (No longer working on Firefox) 2014-05-19 16:38:07 PDT
(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 User image :Irving Reid (No longer working on Firefox) 2014-05-20 11:19:35 PDT
https://hg.mozilla.org/integration/fx-team/rev/87b5205bab4f
Comment 8 User image Carsten Book [:Tomcat] 2014-05-21 05:02:52 PDT
https://hg.mozilla.org/mozilla-central/rev/87b5205bab4f
Comment 9 User image :Irving Reid (No longer working on Firefox) 2014-06-03 07:47:25 PDT
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 User image Ryan VanderMeulen [:RyanVM] 2014-06-04 09:17:40 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/e79674fe4a79

Note You need to log in before you can comment on or make changes to this bug.