-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #15369: Add synced tabs metrics. #16727
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16727 +/- ##
============================================
- Coverage 30.78% 30.11% -0.68%
+ Complexity 1240 1206 -34
============================================
Files 455 454 -1
Lines 18604 18600 -4
Branches 2602 2565 -37
============================================
- Hits 5728 5601 -127
- Misses 12417 12557 +140
+ Partials 459 442 -17 Continue to review full report at Codecov.
|
Request for data collection review formAll questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection. 1) What questions will you answer with this data? 2) Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? 3) What alternative methods did you consider to answer these questions? Why were they not sufficient? 4) Can current instrumentation answer these questions? 5) List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories found 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.
6) Please provide a link to the documentation for this data collection which describes the ultimate data set in a public, complete, and accurate way. 7) How long will this data be collected? Choose one of the following: 8) What populations will you measure? 9) If this data collection is default on, what is the opt-out mechanism for users? 10) Please provide a general description of how you will analyze this data. 11) Where do you intend to share the results of your analysis? 12) Is there a third-party tool (i.e. not Telemetry) that you are proposing to use for this data collection? |
app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt
Outdated
Show resolved
Hide resolved
@@ -659,6 +659,12 @@ private val Event.wrapper: EventWrapper<*>? | |||
{ ProgressiveWebApp.backgroundKeys.valueOf(it) } | |||
) | |||
|
|||
Event.SyncedTabOpened -> EventWrapper<NoExtraKeys>( | |||
{ Metrics.syncedTabsOpenedCount.add(1) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A counter that indicates how many synced tabs a user has opened
The description for this event reads like we want to count the number of times a synced tab item is clicked and not the synced tabs UI is opened. Is that right?
If so, we probably want to add this counter to where the tab was clicked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We spoke offline and clarified how metrics are recorded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments went as individual items by mistake, sorry about the notifications!
@jonalmeida Responding here, because I am not sure the right code was referenced in the comments.
and I did not know how that would affect the code in |
Ordinarily, this would be true. Although, Synced Tabs in the tabs tray is behind a flag in the "secret settings" so this metric would not be fired (unless a power user knows about this setting and enables it). The default location for the UI exists in the browser menu, so the data we collect from this metric would probably be skewed to those power users then. It might make more sense to track the metric in the Another part I would like to understand is the Event.SyncedTabOpened -> EventWrapper<NoExtraKeys>(
{ Metrics.syncedTabsOpenedCount.add(1) }
) This seems to coupling two events together: opening the Synced Tabs UI and clicking on synced tab items. If I understand that correctly, then similar to the above situation, we may be better tracking this in the When we move Synced Tabs to a new place in the tabs tray, we should definitely fix the metrics into the re-written tabs tray. Apologises if I've caused any confusion. 🙂 |
7e3ae0b
to
ed10ae3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We spoke offline to clarify the questions. Thanks!
Talked offline with Kate who confirmed with DS and Boek:
|
4b06923
to
c486080
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data Review Form (to be filled by Data Stewards)
-
Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?
Yes, metrics.yaml and metrics.md -
Is there a control mechanism that allows the user to turn the data collection on and off?
Yes, Fenix data collection settings. -
If the request is for permanent data collection, is there someone who will monitor the data over time?
No, Fenix team will monitor -
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
Type 2 -
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)?
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?
Has expiry -
Does the data collection use a third-party collection tool?
No
This pull request has conflicts when rebasing. Could you fix it @mcarare? 🙏 |
Pull Request checklist
To download an APK when reviewing a PR: