Metrics for devtools reload addon

RESOLVED FIXED in Firefox 47

Status

P2
normal
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: clarkbw, Assigned: ochameau)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 47
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: [btpp-2016-q2])

Attachments

(1 attachment, 4 obsolete attachments)

I'm not sure what the best way is to design a metric for this but I'd like to capture a telemetry count for the usage of the local addon.  

The questions I'd like to answer are:

Are people using this addon?

Possibly:
Are people using this on Nightly and is there Nightly the outdated?
Blocks: 1182254
Doing bug triage with Emma.
Filter on LOBSTER-THERMADOR
Priority: -- → P2
Whiteboard: [btpp-2016-q2]
Assignee: nobody → poirot.alex
Depends on: 1247985
Created attachment 8720388 [details] [diff] [review]
patch v1

Still need to test it.
It should log addon installation.
i.e. when we click on "Load temporary add-on" and register it.
And also reload action, when the developer hit the key shortcut.
Just renaming to disambiguate from James's work to support hot reloading.
Summary: Metrics for devtools hot reload addon → Metrics for devtools reload addon
(Reporter)

Comment 4

3 years ago
(In reply to Alexandre Poirot [:ochameau] from comment #2)
> Created attachment 8720388 [details] [diff] [review]
> patch v1
> 
> Still need to test it.
> It should log addon installation.
> i.e. when we click on "Load temporary add-on" and register it.
> And also reload action, when the developer hit the key shortcut.

Awesome!
Created attachment 8720743 [details] [diff] [review]
patch v2

I'm wondering if userHistogram is important?

Couldn't we identify "how many users had DEVTOOLS_RELOAD_ADDON_INSTALLED_COUNT>0" on the backend side?
Attachment #8720743 - Flags: review?(jryans)
Attachment #8720388 - Attachment is obsolete: true
Comment on attachment 8720743 [details] [diff] [review]
patch v2

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

(In reply to Alexandre Poirot [:ochameau] from comment #5)
> I'm wondering if userHistogram is important?
> 
> Couldn't we identify "how many users had
> DEVTOOLS_RELOAD_ADDON_INSTALLED_COUNT>0" on the backend side?

I think it is possible to calculate the number of users from the basic probe if you use a more advanced query through the custom Spark pipeline, but that's a lot of work for something we'll likely want to reference regularly.

I think it's fine to "pre-compute" the user count with the flag probe as well, which also makes this similar to our other probes.

::: devtools/bootstrap.js
@@ +10,5 @@
>  
> +function actionOccurred(id) {
> +  let {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
> +  let Telemetry = require("devtools/client/shared/telemetry");;
> +  let telemetry = new Telemetry();

Could we cache the `telemetry` instance after the first use?  This would make it more like other usages of the modules.  It's currently okay for now, since we don't use the timer based ones (so there is no state to store), but it could be less confusing for the future that way.

::: toolkit/components/telemetry/Histograms.json
@@ +6982,5 @@
>      "description": "How many times has a custom developer tool been opened via the toolbox button?"
>    },
> +  "DEVTOOLS_RELOAD_ADDON_INSTALLED_COUNT": {
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],
> +    "expires_in_version": "50",

Maybe let them live a bit longer, like 55?
Attachment #8720743 - Flags: review?(jryans) → review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #6)
> ::: devtools/bootstrap.js
> @@ +10,5 @@
> >  
> > +function actionOccurred(id) {
> > +  let {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
> > +  let Telemetry = require("devtools/client/shared/telemetry");;
> > +  let telemetry = new Telemetry();
> 
> Could we cache the `telemetry` instance after the first use?  This would
> make it more like other usages of the modules.  It's currently okay for now,
> since we don't use the timer based ones (so there is no state to store), but
> it could be less confusing for the future that way.

I didn't do that as I'm reloading the loader and modules before/after that and wanted to avoid using a module from a old loader. I was wondering if I should even avoid using modules at all and instead use core telemetry API directly.
Created attachment 8721986 [details] [diff] [review]
patch v3

Bumped to 55.
Attachment #8721986 - Flags: review?(benjamin)
Attachment #8720743 - Attachment is obsolete: true
Comment on attachment 8721986 [details] [diff] [review]
patch v3

I believe this is looking for feedback, not review, on these probes.  These are all for pre-release and we've set an expire version.

We need these to understand the funnel for activation and usage as we promote this feature over the next few releases.
Attachment #8721986 - Flags: review?(benjamin) → feedback?(benjamin)

Comment 11

3 years ago
Comment on attachment 8721986 [details] [diff] [review]
patch v3

I couldn't reach you on IRC, so I guess I'll comment here. I promise faster reaction time next time!

>diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json

>+  "DEVTOOLS_RELOAD_ADDON_INSTALLED_COUNT": {
>+    "description": "How many times the reload addon has been installed?",

>+  "DEVTOOLS_RELOAD_ADDON_INSTALLED_PER_USER_FLAG": {
>+    "description": "How many users have installed the reload addon?",

These descriptions really shouldn't be phrased as questions, and I can't tell by reading them what the difference is or even what they are measuring. Please rephrase these as a statement of the form "This flag is recorded once per session if the reload addon is active." or something like that.

Is the reload addon a normal extension? If so, its presence should already be recorded in the telemetry environment block and you shouldn't need to record it separately here.

I have similar questions about the RELOAD_COUNT and RELOAD_PER_USER_FLAG metrics.
Attachment #8721986 - Flags: feedback?(benjamin) → feedback-
Created attachment 8724761 [details] [diff] [review]
patch v4

With better description, similar to existing ones.

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #11)
> Is the reload addon a normal extension? If so, its presence should already
> be recorded in the telemetry environment block and you shouldn't need to
> record it separately here.

This is a temporary add-on. I'm not sure it is being covered by telemetry.
Developers install it temporarily via about:debugging. It is being automatically
removed on firefox restart.
Attachment #8724761 - Flags: feedback?(benjamin)
Created attachment 8724970 [details] [diff] [review]
patch v5

New descriptions for PER_USER histograms.
Attachment #8724970 - Flags: feedback?(benjamin)
Attachment #8721986 - Attachment is obsolete: true
Attachment #8724761 - Attachment is obsolete: true
Attachment #8724761 - Flags: feedback?(benjamin)

Comment 14

3 years ago
Comment on attachment 8724970 [details] [diff] [review]
patch v5

data-review=me
Attachment #8724970 - Flags: feedback?(benjamin) → feedback+
https://hg.mozilla.org/integration/fx-team/rev/69b8d739ddae4447e7bac8545b90fe69a6c8ff66
Bug 1248435 - Add telemetry probes for reload addon installation and reload action. r=jryans data-review=bsmedberg

Comment 17

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/69b8d739ddae
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.