Closed Bug 1447302 Opened 6 years ago Closed 6 years ago

Add telemetry probes for actions related to accessibility inspector tool.

Categories

(DevTools :: Accessibility Tools, enhancement)

enhancement
Not set
normal

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: yzen, Assigned: yzen)

References

Details

Attachments

(1 file, 4 obsolete files)

      No description provided.
Accessibility Inspector tool is going to be landing on Nightly in the next several weeks and we would like to collect some telemetry related to its usage patterns, similar to how other Developer Tools panels. 

We are also planning on running a shield study for a Developer Edition population where test cohort would have the Accessibility Inspector enabled by default vs the control cohort that will only have it available in the Developer Tools options. The above mentioned telemetry would help with answering questions about whether there's a need for this tool to be more visible or not.

The following probes are of interest:

* Which tools (accessibility in particular) are checked/unchecked within the Developer tools options

* Number of times and active time of the accessibility panel being open

* Number of times and active time of the accessibility service being enabled

* Number of times and active time of the picker tool being used within the accessibility panel.

* How often the accessibility object is inspected in the accessibility panel (and from where, e.g. context menu, DOM inspector panel)

* How often the DOM node is inspected from the accessibility panel (via navigation from an accessible object to a corresponding DOM node).
Attached patch 1447302 patch (obsolete) — Splinter Review
Depends on the patch from bug 1428427 (for inspect a11y props pathway).
Attachment #8966336 - Flags: review?(jdescottes)
Attached patch 1447302 patch v2 (obsolete) — Splinter Review
Moved accessibility node inspected count from scalars to histogram.
Attachment #8966336 - Attachment is obsolete: true
Attachment #8966336 - Flags: review?(jdescottes)
Attachment #8966348 - Flags: review?(jdescottes)
Comment on attachment 8966348 [details] [diff] [review]
1447302 patch v2

Marking for data review, thanks!
Attachment #8966348 - Flags: feedback?(rweiss)
Comment on attachment 8966348 [details] [diff] [review]
1447302 patch v2

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

Code-wise this looks good, thanks Yura.
A few questions and comments regarding the actual probes we use, hopefully can be clarified during data-review.

::: devtools/client/framework/toolbox-options.js
@@ +228,5 @@
>  
>        if (!tool.isWebExtension) {
>          gDevTools.emit(this.checked ? "tool-registered" : "tool-unregistered", tool.id);
> +        // Record which tools were registered and unregistered.
> +        Services.telemetry.keyedScalarAdd("devtools.tool.registered",

keyedScalarAdd only works with integers, this should be keyedScalarSet

::: toolkit/components/telemetry/Histograms.json
@@ +9500,5 @@
> +    "kind": "exponential",
> +    "high": 10000000,
> +    "n_buckets": 100,
> +    "bug_numbers": [1447302],
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org", "jryans@mozilla.com"],

Should that be your email instead? (same comment for two other probes below)

::: toolkit/components/telemetry/Scalars.yaml
@@ +971,5 @@
> +  registered:
> +    bug_numbers:
> +      - 1447302
> +    description: >
> +      Recorded on enable tool checkbox check/uncheck in Developer Tools options

I have never used keyed boolean scalars so I am not sure if it will really answer this question. Hopefully the data review can clarify this. My biggest question is: will we be able to now how many times the scalar was set with "false"? Otherwise we would only get information about "checking" tools.

I looked at other keyed boolean scalars :
- no_job: https://mzl.la/2JzGMgK
- sync_login_state_transitions: https://mzl.la/2JwYX6E

From there it doesn't seem obvious if anything is logged for "false" ?

@@ +974,5 @@
> +    description: >
> +      Recorded on enable tool checkbox check/uncheck in Developer Tools options
> +      panel. Boolean stating if the tool was enabled or disabled by the user.
> +    expires: never
> +    kind: boolean

should this be keyed: true? (and description should mention this is keyed by tool id)

@@ +987,5 @@
> +    bug_numbers:
> +      - 1447302
> +    description: >
> +      Number of times an accessible object was inspected from outside the
> +      Accessibility tool (navgiation to Accessibility panel).

nit: navgiation -> navigation

@@ +989,5 @@
> +    description: >
> +      Number of times an accessible object was inspected from outside the
> +      Accessibility tool (navgiation to Accessibility panel).
> +    expires: never
> +    kind: uint

should be keyed true, and description should mention this is keyed by reason
Attachment #8966348 - Flags: review?(jdescottes) → review+
Comment on attachment 8966348 [details] [diff] [review]
1447302 patch v2

Moving the data review to Chris as Rebecca is probably too busy these days.
Attachment #8966348 - Flags: feedback?(rweiss) → feedback?(chutten)
Comment on attachment 8966348 [details] [diff] [review]
1447302 patch v2

Fixing the f? to r? as per wiki.
Attachment #8966348 - Flags: review?(chutten)
Attachment #8966348 - Flags: review+
Attachment #8966348 - Flags: feedback?(chutten)
Comment on attachment 8966348 [details] [diff] [review]
1447302 patch v2

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

First things first, let me put on my Firefox Telemetry hat and take a look at what measurements you're proposing in the terms of "is this the best way to answer your questions?"

A quick note over here:

(In reply to Julian Descottes [:jdescottes][:julian] from comment #6)
> I have never used keyed boolean scalars so I am not sure if it will really
> answer this question. Hopefully the data review can clarify this. My biggest
> question is: will we be able to now how many times the scalar was set with
> "false"? Otherwise we would only get information about "checking" tools.
> 
> I looked at other keyed boolean scalars :
> - no_job: https://mzl.la/2JzGMgK
> - sync_login_state_transitions: https://mzl.la/2JwYX6E
> 
> From there it doesn't seem obvious if anything is logged for "false" ?

This is because neither of those scalars are ever set to anything but true.[1][2]

If you set it to false, we report (and aggregate) a false.

Keep in mind, however, that if we have a user that is toggling that checkbox periodically, the values we report will be whatever the value was last set to during that subsession (and Telemetry makes no guarantees when subsessions end)

As for whether it'll answer the questions or not, well, I apologize but I'm not 100% sure. devtools wraps the Telemetry APIs and renames them, so I'm not always quite sure what's going on (For instance, you use your own timers instead of TelemetryStopwatch). It seems as though through counts and durations you are intending to collect the data you'll need to answer the questions you pose. I'm pretty sure. Though I'm assuming all of :jdescottes notes about keys and descriptions are corrected.

Now to put on my Data Collection Reviewer hat:

I'm not sure that permanent, ongoing measurement of these features is necessary to answer the questions you pose. Evaluating whether the panel should be included/excluded is the type of question better (in terms of lean data practices) answered by a scoped study, not by permanent in-tree measures.

Also, for new measurements we prefer to set these running for, say, five versions instead of starting them off permanent. Five versions in the future we will have the context to decide whether they should be made permanent, whether we should adjust them and try the new versions for another five, or drop them as no longer necessary.

Please evaluate your need for these on an ongoing basis instead of as part of a study. For measurements that remain, please have them expire in five versions or so to encourage period re-evaluation of suitability.

Then re-submit for Data Collection Review.

[1]: https://searchfox.org/mozilla-central/rev/4114ad2cfcbc511705c7865a4a34741812f9a2a9/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp#333
[2]: https://searchfox.org/mozilla-central/rev/4114ad2cfcbc511705c7865a4a34741812f9a2a9/services/sync/modules/browserid_identity.js#96,116

::: toolkit/components/telemetry/Scalars.yaml
@@ +991,5 @@
> +      Accessibility tool (navgiation to Accessibility panel).
> +    expires: never
> +    kind: uint
> +    notification_emails:
> +      - dev-developer-tools@lists.mozilla.org, yzenevich@mozilla.com

This isn't how multiple entries are done in a yaml list, see browser.session.restore.worker_restart_count for an example.

::: toolkit/components/telemetry/histogram-whitelists.json
@@ +1571,5 @@
>      "DEVTOOLS_ABOUTDEBUGGING_OPENED_COUNT",
> +    "DEVTOOLS_ACCESSIBILITY_NODE_INSPECTED_COUNT",
> +    "DEVTOOLS_ACCESSIBILITY_OPENED_COUNT",
> +    "DEVTOOLS_ACCESSIBILITY_PICKER_USED_COUNT",
> +    "DEVTOOLS_ACCESSIBILITY_SERVICE_ENABLED_COUNT",

Additions to the whitelist are an instant r-, I'm afraid. The whitelist is there to tolerate ancient probes until the die out and are replaced properly.

As the error message states, if you want a count, you don't want a histogram. You want a uint scalar.
Attachment #8966348 - Flags: review?(chutten) → review-
Attached patch 1447302 patch v3 (obsolete) — Splinter Review
Updated the following:

* added logCountScalar method to telemetry for counts
* moved count histograms to count scalars
* changed all expire versions to FF65
Attachment #8966348 - Attachment is obsolete: true
Attachment #8967483 - Flags: review?(jdescottes)
Attachment #8967483 - Flags: review?(chutten)
* removed things from whitelist json
Comment on attachment 8967483 [details] [diff] [review]
1447302 patch v3

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

::: toolkit/components/telemetry/Histograms.json
@@ +9472,5 @@
> +  "DEVTOOLS_ACCESSIBILITY_PICKER_TIME_ACTIVE_SECONDS": {
> +    "record_in_processes": ["main", "content"],
> +    "expires_in_version": "65",
> +    "kind": "exponential",
> +    "high": 10000000,

That's a lot of seconds!

The way that histograms work is we use the "high" value as the highest value you want a sensible resolution at. If we receive an accumulation above the value of "high" we just stuff it into the last bucket. We won't skip accumulations, we cap them. So you don't need to set a "high" value to be this high.

That covers why you don't -have- to have it really high. To explain why you don't -want- it to be high might be best done over here: https://telemetry.mozilla.org/histogram-simulator/#low=1&high=10000000&n_buckets=100&kind=exponential&generate=normal

If you look for the value of 5min (300s) you'll see the bucket width is quite wide: 45s. If you drop three zeros off of your "high" value, it drops in width to be about half that.

So ideally what you want is to choose a value of "high" that captures the upper range of values you want to distinguish. If you don't care about any numbers above a day, cap it at 86400. We'll still know how long people spend in there (the histogram's "sum" is full-precision, even if its buckets have a cap), but we'll also have resolution where we need it (and save the users' bandwidth when we send the data)

Does this explanation make sense? What are typical values you expect in these _SECONDS probes?

@@ +9476,5 @@
> +    "high": 10000000,
> +    "n_buckets": 100,
> +    "bug_numbers": [1447302],
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org", "yzenevich@mozilla.com"],
> +    "releaseChannelCollection": "opt-out",

Do these need to be collected on the release channel, or are we only interested in the use of this on pre-release channels? (It is fine to say we need it on release channel, it's just important to acknowledge there's a cost to it)

::: toolkit/components/telemetry/Scalars.yaml
@@ +973,5 @@
> +      - 1447302
> +    description: >
> +      Recorded on enable tool checkbox check/uncheck in Developer Tools options
> +      panel. Boolean stating if the tool was enabled or disabled by the user.
> +      Keyed by tool id.

From the description, the name "registered" for this scalar seems odd. Is that what the process is called when you check the tool checkbox?

If there's a list of tool ids, it might be helpful to include in the description the place in the source code where that list is maintained.
Attachment #8967483 - Flags: review?(chutten)
(In reply to Chris H-C :chutten from comment #13)
> Comment on attachment 8967483 [details] [diff] [review]
> 1447302 patch v3
> 
> Review of attachment 8967483 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +9472,5 @@
> > +  "DEVTOOLS_ACCESSIBILITY_PICKER_TIME_ACTIVE_SECONDS": {
> > +    "record_in_processes": ["main", "content"],
> > +    "expires_in_version": "65",
> > +    "kind": "exponential",
> > +    "high": 10000000,
> 
> That's a lot of seconds!
> 
> The way that histograms work is we use the "high" value as the highest value
> you want a sensible resolution at. If we receive an accumulation above the
> value of "high" we just stuff it into the last bucket. We won't skip
> accumulations, we cap them. So you don't need to set a "high" value to be
> this high.
> 
> That covers why you don't -have- to have it really high. To explain why you
> don't -want- it to be high might be best done over here:
> https://telemetry.mozilla.org/histogram-simulator/
> #low=1&high=10000000&n_buckets=100&kind=exponential&generate=normal
> 
> If you look for the value of 5min (300s) you'll see the bucket width is
> quite wide: 45s. If you drop three zeros off of your "high" value, it drops
> in width to be about half that.
> 
> So ideally what you want is to choose a value of "high" that captures the
> upper range of values you want to distinguish. If you don't care about any
> numbers above a day, cap it at 86400. We'll still know how long people spend
> in there (the histogram's "sum" is full-precision, even if its buckets have
> a cap), but we'll also have resolution where we need it (and save the users'
> bandwidth when we send the data)
> 
> Does this explanation make sense? What are typical values you expect in
> these _SECONDS probes?
> 
> @@ +9476,5 @@
> > +    "high": 10000000,
> > +    "n_buckets": 100,
> > +    "bug_numbers": [1447302],
> > +    "alert_emails": ["dev-developer-tools@lists.mozilla.org", "yzenevich@mozilla.com"],
> > +    "releaseChannelCollection": "opt-out",
> 
> Do these need to be collected on the release channel, or are we only
> interested in the use of this on pre-release channels? (It is fine to say we
> need it on release channel, it's just important to acknowledge there's a
> cost to it)

The tool can be enabled in DevTools settings (not hidden by other prefs) so, indeed, we would like to gather data about its use on all channels, at least until the expiration date.

> 
> ::: toolkit/components/telemetry/Scalars.yaml
> @@ +973,5 @@
> > +      - 1447302
> > +    description: >
> > +      Recorded on enable tool checkbox check/uncheck in Developer Tools options
> > +      panel. Boolean stating if the tool was enabled or disabled by the user.
> > +      Keyed by tool id.
> 
> From the description, the name "registered" for this scalar seems odd. Is
> that what the process is called when you check the tool checkbox?

Yes, tool definition gets "registered"/"unregistered" from DevTools toolbox when users click on the appropriate checkbox.

> 
> If there's a list of tool ids, it might be helpful to include in the
> description the place in the source code where that list is maintained.
Attached patch 1447302 patch v4 (obsolete) — Splinter Review
Hopefully this version takes care of all you comments.
Attachment #8967483 - Attachment is obsolete: true
Attachment #8967483 - Flags: review?(jdescottes)
Attachment #8967518 - Flags: review?(jdescottes)
Attachment #8967518 - Flags: review?(chutten)
Comment on attachment 8967518 [details] [diff] [review]
1447302 patch v4

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

r+ on the telemetry-facing portions of the code.
Attachment #8967518 - Flags: review?(chutten) → review+
DATA COLLECTION REVIEW REQUEST (taken from the gist, in case it becomes inaccessible in future, and amended):

    What questions will you answer with this data?

This data will help understanding the extent of the use of the accessibility tool in Firefox Developer Tools. This is a brand new panel that recently landed on Nightly and has no panel specific telemetry associated with it. It is landed as an optional tool (can be enabled in DevTools settings), we currently do not measure which tools are enabled/disabled in the settings.

    Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? Some example responses:

    Understand how engaged our developer audience is with accessibility tools in context of using developer tools overall.

    Determine how discoverable accessibility tools are within DevTools and Firefox itself.

    Determine what functionality is used the most or least to consequently improve developer audience's user expereince.

    What alternative methods did you consider to answer these questions? Why were they not sufficient?

This is a brand new functionality (accessibility panel) that is now part of the Developer Tools and we have no telemetry associated with it at the moment. The only information that is collected is relateted to the general use of DevTools (e.g. opened/closed/active time).

    Can current instrumentation answer these questions?

Yes

    List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories on the Mozilla wiki.

Note that the data steward reviewing your request will characterize your data collection based on the highest (and most sensitive) category.
Which tools are checked/unchecked (in other words registered/unregistered) by the user within the Developer tools options. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Number of times the accessibility panel was opened. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Active use time of the accessibility panel. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Number of times the accessibility service was enabled within the accessibility tool. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Active duration time the accessibility service was enabled within the accessibility tool. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Number of times the picker tool was used within the accessibility tool. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
Active duration time the picker tool was used within the accessibility tool. 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
How often the accessibility object is inspected in the accessibility panel (and from where, e.g. context menu, DOM inspector panel). 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)
How often the DOM node is inspected from the accessibility panel (via navigation from an accessible object to a corresponding DOM node). 	Category 2 	[Bug 1447302](https://bugzilla.mozilla.org/show_bug.cgi?id=1447302)

    How long will this data be collected? Choose one of the following:

Until Firefox 64

    What populations will you measure?

    Which release channels?

All channels

    Which countries?

All countries

    Which locales?

All locales

    If this data collection is default on, what is the opt-out mechanism for users?

Same as the rest of browser telemetry gathered.

    Please provide a general description of how you will analyze this data.

We are planning on running a shield study for the Dev Edition channel where the panel would be enabled by default for a test population of users. These measurements will help answer questions about discoverability, use and overall satisfaction with the tool.

Dashboard with data visualized.

    Where do you intend to share the results of your analysis?

This is for internal use mostly. Dashboard will be accessible to all employees. The shield study will help determine if this tool is a good candidate to be a default tab in the Developer Tools on release.
DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, per usual Telemetry standards this data comes with descriptive descriptions and will be transformed and made available per the rules in place on datasets like main_summary.

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, the checkbox in the Firefox Privacy Preferences

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction

    Is the data collection request for default-on or default-off?

Default-on

    Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

Yes: the string names of tools and panels will be used as keys in the measures.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

Yes.
----
Result: datareview+

:yzen, could you file the follow-up bug for re-evaluating the suitability of extending or making-permanent of these probes?
Flags: needinfo?(yzenevich)
See Also: → 1453864
(In reply to Chris H-C :chutten from comment #18)
> 
> :yzen, could you file the follow-up bug for re-evaluating the suitability of
> extending or making-permanent of these probes?

Created bug 1453864 to evaluate the probes around FF65
Flags: needinfo?(yzenevich)
See Also: → 1454314
Comment on attachment 8967518 [details] [diff] [review]
1447302 patch v4

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

Thanks for the updated patch, I have second thoughts about reusing toolOpened/toolClosed for "accessibilityServiceEnabled". It's not really a tool and doesn't follow the same lifecycle as the tools (can be started/stopped from other places etc...). 

See my comments, let me know what you think (clearing the r? to discuss about that)

::: devtools/client/accessibility/accessibility-panel.js
@@ +231,5 @@
>      if (this._front) {
>        await this._front.destroy();
>      }
>  
>      this._front = null;

Should we set this._telemetry to null here as well ?

::: devtools/client/accessibility/components/Toolbar.js
@@ +53,5 @@
>      let { accessibility, dispatch } = this.props;
>      this.setState({ disabling: true });
> +
> +    if (gTelemetry) {
> +      gTelemetry.toolClosed("accessibilityServiceEnabled");

I didn't think about this earlier, but should we log this from the UI actions or should we listen to the accessibility events "init" and "shutdown"? (and have this code in accessibility-panel or accessibility-view?)

(see my comment in Scalars.yaml)

::: devtools/client/accessibility/picker.js
@@ +140,5 @@
>      this.pickerButton.isChecked = false;
>  
>      await this.walker.cancelPick();
> +
> +    this._telemetry.toolOpened("accessibilityPickerUsed");

This the stop() method, should it be toolClosed?

@@ +170,5 @@
>      this.walker.on("picker-accessible-previewed", this.onPickerAccessiblePreviewed);
>      this.walker.on("picker-accessible-canceled", this.onPickerAccessibleCanceled);
>  
>      await this.walker.pick(doFocus);
> +    this._telemetry.toolClosed("accessibilityPickerUsed");

Same comment, start() method -> toolOpened ?

::: devtools/client/shared/telemetry.js
@@ +338,5 @@
> +   *         Scalar in which the data is to be stored.
> +   * @param  value
> +   *         Value to store.
> +   */
> +  logCountScalar(scalarId, value) {

not an issue: I guess you introduced a different method here in order to use scalarAdd rather than scalarSet. I logged Bug 1413551 about switching scalarSet to scalarAdd but didn't go through with it. I think in the long run we might want to migrate our other "count" scalars to scalarAdd. But that should definitely be done in a separate bug.

That's another case where having our own wrapper with method names that differ from telemetry ones make things harder to assess.

::: toolkit/components/telemetry/Scalars.yaml
@@ +1052,5 @@
> +    bug_numbers:
> +      - 1447302
> +    description: >
> +      Number of times platform accessibility has been enabled in DevTools
> +      Accessibility panel.

toolOpened("accessibilityServiceEnabled") everytime DevTools accessibility panel is started if the accessibility front is already started. Consequently this scalar doesn't really measure the number of times the service was started from DevTools. If we want to measure that we would need to separate the timer histogram from the enabled count and directly use telemetry APIs rather than reusing toolOpened/toolClosed.
Attachment #8967518 - Flags: review?(jdescottes)
Attached patch 1447302 patch v5Splinter Review
Only time DEVTOOLS_ACCESSIBILITY_SERVICE_TIME_ACTIVE_SECONDS based on a11y service and its actor lifecycle (init/shutdown).

devtools.accessibility.service_enabled_count is now purely a UI interaction scalar, independent of duration and toolOpened/Closed flow.
Attachment #8967518 - Attachment is obsolete: true
Attachment #8968255 - Flags: review?(jdescottes)
Comment on attachment 8968255 [details] [diff] [review]
1447302 patch v5

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

Thanks for the updated patch! Some comments, should be ready to go after addressing them.

::: devtools/client/accessibility/accessibility-panel.js
@@ +148,5 @@
>      this.postContentMessage("initialize", this._front, this._walker, this._isOldVersion);
>    },
>  
> +  updateA11YServiceDurationTimer() {
> +    this._telemetry[this._front.enabled ? "startTimer" : "stopTimer"](

nit: I would prefer to stick with verbose forms here:

  if (this._front.enabled) { 
    this._telemetry.startTimer(A11Y_SERVICE_DURATION);
  } else {
    this._telemetry.stopTimer(A11Y_SERVICE_DURATION);
  }

I feel it makes searching and maintenance easier in the long run.

@@ +224,5 @@
>  
> +    this._telemetry.destroy();
> +
> +    this._front.off("init", this.updateA11YServiceDurationTimer);
> +    this._front.off("shutdown", this.updateA11YServiceDurationTimer);

Maybe move these two lines in the `if (this._front) {}` block below?

::: devtools/client/accessibility/components/Description.js
@@ +70,5 @@
>      let { accessibility, dispatch } = this.props;
>      this.setState({ enabling: true });
> +
> +    if (gTelemetry) {
> +      gTelemetry.logCountScalar(A11Y_SERVICE_ENABLED_COUNT);

Should be 
  gTelemetry.logCountScalar(A11Y_SERVICE_ENABLED_COUNT, 1);

::: devtools/client/accessibility/components/Toolbar.js
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  "use strict";
>  
> +/* global gTelemetry */

nit: no longer needed

::: devtools/client/shared/telemetry.js
@@ +343,5 @@
> +    try {
> +      if (isNaN(value)) {
> +        dump(`Warning: An attempt was made to write a non-numeric value ` +
> +             `${value} to the ${scalarId} scalar. Only numeric values are ` +
> +             `allowed.`);

nit: should end with "\n"
Feel free to add it to the similar dump in logKeyedScalar() at the same time :)
Attachment #8968255 - Flags: review?(jdescottes) → review+
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/529b9c25a0c2
add telemetry probes for accessibility inspector Developer Tool panel. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/529b9c25a0c2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
See Also: → 1454737
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.