Closed Bug 1253125 Opened 4 years ago Closed 4 years ago

[telemetry] Toolbox panels should not call toolOpened manually

Categories

(DevTools :: General, defect, P1)

defect

Tracking

(firefox47 verified)

VERIFIED FIXED
Firefox 47
Tracking Status
firefox47 --- verified

People

(Reporter: jryans, Assigned: jryans)

Details

(Whiteboard: [btpp-fix-now])

Attachments

(1 file)

Several panels apparently call toolOpened/Closed themselves, but they don't need to do that!  The toolbox calls it for you.  Only things outside of the toolbox should need to call toolOpened manually.

This was spotted by QA in bug 1247985.

At least the following are affected:

> DEVTOOLS_SHADEREDITOR_OPENED_COUNT
> DEVTOOLS_CANVASDEBUGGER_OPENED_COUNT
> DEVTOOLS_MEMORY_OPENED_COUNT
> DEVTOOLS_STORAGE_OPENED_COUNT
> DEVTOOLS_WEBAUDIOEDITOR_OPENED_COUNT
> DEVTOOLS_SCRATCHPAD_OPENED_COUNT

Unfortunately, this means our opened counts and active times for these tools are not correct. :(
Flags: qe-verify+
QA Contact: mihai.boldan
Priority: -- → P1
Whiteboard: [btpp-fix-now]
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53940cc0b217

This run is noisy from a bad parent rev, but none of the failures seem related to the change here.
Comment on attachment 8726332 [details]
MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe

https://reviewboard.mozilla.org/r/37925/#review34543

::: devtools/client/scratchpad/scratchpad.js
(Diff revision 1)
> -telemetry.toolOpened("scratchpad");

Scratchpad can be launched from the Web Developer menu or inside the toolbox.

We need to find a way to only call telemetry.toolOpened and telemetry.toolClosed when we are *not* in the toolbox.

Another option would be to stop automatically sending pings for main toolbox panels... the advantage being that it would be less confusing.

Your choice.
Attachment #8726332 - Flags: review?(mratcliffe)
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3)
> Comment on attachment 8726332 [details]
> MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside
> tools. r=mratcliffe
> 
> https://reviewboard.mozilla.org/r/37925/#review34543
> 
> ::: devtools/client/scratchpad/scratchpad.js
> (Diff revision 1)
> > -telemetry.toolOpened("scratchpad");
> 
> Scratchpad can be launched from the Web Developer menu or inside the toolbox.

The current patch will record `toolOpened("scratchpad")` for the toolbox panel, and `toolOpened("scratchpad-window")` for the separate.  Is this sufficient, or are you saying `toolOpened("scratchpad")` should be record for both cases?
Flags: needinfo?(mratcliffe)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> `toolOpened("scratchpad-window")` for the separate.

for the separate *window*.
Comment on attachment 8726332 [details]
MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe

https://reviewboard.mozilla.org/r/37925/#review34545

In that case... ship it ;)
Attachment #8726332 - Flags: review+
Flags: needinfo?(mratcliffe)
https://hg.mozilla.org/mozilla-central/rev/ad2c21a89e6e
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
I reproduced this issue on Firefox 47.0a1 (2016-03-01) and on Windows 10 x64.
The issue is no longer reproducible on Firefox 47.0b3.
Tests were performed on Windows 10 x64 OS and by using the affected counters from Comment 0.
I am marking this issue Verified Fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.