1531395 - Add telemetry to track persist option usage in the Console panel

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

We should have some stats about how many users are using the "Persist Log" option.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Bug 1463095 might be a source of inspiration.

@Nicolas: any other bugs we could refer to to make the implementation easier?

Honza

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Lucklily, we already have a mechanism to handle telemetry calls.

So, in the console, in order to change the "Persist Logs" option, we use a Redux action, persistToggle, defined in devtools/client/webconsole/actions/ui.js#24-32.
We can see that we dispatch an object with a PERSIST_TOGGLE type.

Then, we have a telemetry middleware, i.e. a piece of code that will be triggered on each state change (so, it will be called when the PERSIST_TOGGLE action is dispatched as well).

The middleware is in devtools/client/webconsole/middleware/event-telemetry.js

This is the main function:

/**
 * Event telemetry middleware is responsible for logging specific events to telemetry.
 */
function eventTelemetryMiddleware(telemetry, sessionId, store) {
  return next => action => {
    const oldState = store.getState();
    const res = next(action);
    if (!sessionId) {
      return res;
    }

    const state = store.getState();

    const filterChangeActions = [
      FILTER_TEXT_SET,
      FILTER_TOGGLE,
      DEFAULT_FILTERS_RESET,
    ];

    if (filterChangeActions.includes(action.type)) {
      filterChange({
        action,
        state,
        oldState,
        telemetry,
        sessionId,
      });
    } else if (action.type === MESSAGES_ADD) {
      messagesAdd({ action, telemetry });
    }

    return res;
  };
}

You can see that we already have some function calls that are done when a given action is dispatched.
So for this bug, we will have another else if where we check the PERSIST_TOGGLE action type.

For this case, we will use the telemetry object to record the event, like:

  telemetry.recordEvent("persist_changed", "webconsole", null, {
    "active": /** herer we need to retrieve the value of the persist checkbox **/,
    "session_id": sessionId,
  });

Comment 3

[Paarmita Bhargava]

Hey,
I nobody is working on it, I would like to work on this.
Can you please assign it to me?

Comment 4

[lloan alas:[lloanalas]]

Hi, Outreachy applicant here. I'm also interested in working on this issue, just in case previous requester already has bugs assigned.

Thanks a ton!

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

hello Paarmita, lloan,
Since Paarmita was first to ask, I'm going to assign her to bug.
lloan, it would be nice if you could also look at the bug, and assist Paarmita here. I'm thinking this might need a test to make sure we do listen for those events, so you could do some research on how to add such tests, and share your findings here :) is that okay for you?

Comment 6

[lloan alas:[lloanalas]]

(In reply to Nicolas Chevobbe from comment #5)

hello Paarmita, lloan,
Since Paarmita was first to ask, I'm going to assign her to bug.
lloan, it would be nice if you could also look at the bug, and assist Paarmita here. I'm thinking this might need a test to make sure we do listen for those events, so you could do some research on how to add such tests, and share your findings here :) is that okay for you?

Sure thing boss, sounds good!

Comment 7

[lloan alas:[lloanalas]]

(In reply to Paarmita Bhargava from comment #3)

Hey,
I nobody is working on it, I would like to work on this.
Can you please assign it to me?

In devtools/client/webconsole/middleware/event-telemetry.js line #43 I added this:

    else if (action.type === PERSIST_TOGGLE) {
      telemetry.recordEvent("persist_changed", "webconsole", null, {
        "active": action.value.toString(),
        "session_id": sessionId,
      }); 

Also, in line 7, I added PERSIST_TOGGLE as well.

Then in event-telemetry.js I updated the persistToggle function to this:

function persistToggle() {
  return ({dispatch, getState, prefsService}) => {
    const uiState = getAllUi(getState());
    dispatch({
      type: PERSIST_TOGGLE,
      value: uiState.persistLogs
    });    
    prefsService.setBoolPref(PREFS.UI.PERSIST, uiState.persistLogs);
  };
};  

Only issue I'm facing at the moment is that when I toggle the persist log state, its giving me this error:

Error: Unknown event: ["devtools.main", "persist_changed", "webconsole"] in telemetry.js:571:24

I reached out to Honza to see if I had to register the event prior to using it. Have you had any luck with your approach - what have you tried?

:nchevobbe I tried searching some more about this online, but came up pretty short in terms of documentation. My question is, do I need to register the event 'persist_changed' - I'm thinking that might be my issue. Trying to figure out if the code base already has a list of allowed events to add to it, but can't seem to find something yet, figured I'd reach out to someone just in case they knew off the top of their head.

Comment 8

[lloan alas:[lloanalas]]

Paarmina,

I was able to add the event in /toolkit/components/telemetry/Events.yaml under devtools.main:

persist_changed:
    objects: ["netmonitor", "webconsole"]
    bug_numbers: [1531395]
    notification_emails: ["[e-mail]", "[e-mail]"]
    record_in_processes: ["main"]
    description: User has changed log persist status.
    release_channel_collection: opt-out
    expiry_version: never
    extra_keys:
      persist_state: "The log persist status."
      session_id: The start time of the session in milliseconds since epoch (Unix Timestamp) e.g. 1396381378123.

I think :nchevobbe could step in on this part and tell us if that's correctly formatted.

Also updated the else if section to reflect the use for the data being passed in as extra values to the event

devtools/client/webconsole/middleware/event-telemetry.js

} else if (action.type === PERSIST_TOGGLE) {
      telemetry.recordEvent("persist_changed", "webconsole", null, {
        "persist_state": action.status.toString(),
        "session_id": sessionId,
      }); 
    }

Also updated the persistToggle function for the same reason:

function persistToggle() {
  return ({dispatch, getState, prefsService}) => {
    const uiState = getAllUi(getState());
    dispatch({
      type: PERSIST_TOGGLE,
      status: uiState.persistLogs
    });    
    prefsService.setBoolPref(PREFS.UI.PERSIST, uiState.persistLogs);
  };
};  

Don't forget to add PERSIST_TOGGLE here as well: devtools/client/webconsole/middleware/event-telemetry.js

const {
  FILTER_TEXT_SET,
  FILTER_TOGGLE,
  DEFAULT_FILTERS_RESET,
  MESSAGES_ADD,
  PERSIST_TOGGLE,
} = require("devtools/client/webconsole/constants");

:nchevobbe - do we need to write a test for this?

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Yes, a test would be nice.
We already have a test for the "filters_changed" event here: devtools/client/webconsole/test/mochitest/browser_webconsole_telemetry_filters_changed.js.

Comment 10

[lloan alas:[lloanalas]]

(In reply to Nicolas Chevobbe from comment #9)

Yes, a test would be nice.
We already have a test for the "filters_changed" event here: devtools/client/webconsole/test/mochitest/browser_webconsole_telemetry_filters_changed.js.

Awesome! I'll let :Paarmita handle the test and see if she needs any help there.

Comment 11

[lloan alas:[lloanalas]]

:paarmita How is it going with the test? Anything else you want to bounce off me?

Comment 12

[Paarmita Bhargava]

Attachment: Bug 1531395 - Add telemetry to track persist option usage in the Console panel

Bug 1531395 - Add telemetry to track persist option usage in the Console panel

Comment 13

[lloan alas:[lloanalas]]

Paarmita(In reply to Paarmita Bhargava from comment #12)

Created attachment 9052184 [details]
Bug 1531395 - Add telemetry to track persist option usage in the Console panel

Bug 1531395 - Add telemetry to track persist option usage in the Console panel

Paarmita, I haven't been able to reach you on Slack for a few days now, but take a look here -> https://phabricator.services.mozilla.com/D26046 - test this out, let me know if you have any questions or if you have any improvements.

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Paarmita, I'm assigning the bug to lloan since his patch is almost ready to land, and yours still suffers from eslint failures (also I think you have other assigned bug).

Comment 15

[lloan alas:[lloanalas]]

Attachment: Bug 1531395 - Add telemetry to track persist option usage in the Console panel. r=nchevobbe

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Attachment: data-review.txt

Comment 17

[François Marier [:francois]]

Comment on attachment 9057444 [details]
data-review.txt

Hi Nicolas, I left Mozilla last year and so I'm not a data steward anymore. You'll have to pick someone from the list at the top of https://wiki.mozilla.org/Firefox/Data_Collection.

Comment 18

[Nicolas Chevobbe [:nchevobbe]]

Thanks for letting me know François!

Comment 19

[Chris H-C :chutten]

Comment on attachment 9057444 [details]
data-review.txt

Preliminary note:

In your answer to Q3 you say you already have some collections in this area that aren't getting the job done. Are these scheduled for removal since this collection will take their place?

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. This collection is Telemetry so is documented in its definitions file [Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

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

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Harald Kirschner is responsible.

    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 for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    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?

No. This collection is permanent.

---
Result: datareview+

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

In your answer to Q3 you say you already have some collections in this area that aren't getting the job done. Are these scheduled for removal since this collection will take their place?

Sorry Chris, this is erroneous (copied from another data review 😕), we don't have anything right now for this behavior.
Should I update the document?

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c503fc359408
Add telemetry to track persist option usage in the Console panel. r=nchevobbe

Comment 22

[Chris H-C :chutten]

(In reply to Nicolas Chevobbe [:nchevobbe] [on PTO 04/17 -> 04/23 ] from comment #20)

In your answer to Q3 you say you already have some collections in this area that aren't getting the job done. Are these scheduled for removal since this collection will take their place?

Sorry Chris, this is erroneous (copied from another data review 😕), we don't have anything right now for this behavior.
Should I update the document?

This note here is documentation enough, no worries. I'm just trying to keep our data collections clean : )

Comment 23

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/c503fc359408