-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #22147 - New search term groups telemetry #22368
For #22147 - New search term groups telemetry #22368
Conversation
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.
|
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.
eng r+
app/metrics.yaml
Outdated
bugs: | ||
- https://github.com/mozilla-mobile/fenix/issues/22147 | ||
data_reviews: | ||
- ???????????? |
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.
app/metrics.yaml
Outdated
search_term_group_open_tab: | ||
type: event | ||
description: | | ||
A user opens a tab from the search term group. |
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.
From reading these descriptions, I thought these were probes for the tabs tray itself. Might be good to clarify the location where these probes are being added. For example..
A user opens a tab from the search term group. | |
A user opens a tab from the search term group from the history list. |
(same for the other two probes)
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.
Agreed. Looks like these are for History. FYI, I recently add one telemetry for this requirement #22300
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.
Please move these to the history
category. Also double check the PR I linked. Thanks,
app/metrics.yaml
Outdated
search_term_group_open_tab: | ||
type: event | ||
description: | | ||
A user opens a tab from the search term group. |
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.
Agreed. Looks like these are for History. FYI, I recently add one telemetry for this requirement #22300
These new probes are already in the
so in Glean similarly to https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/history_search_term_group_tapped the 3 events will actually be:
Will add the history reference in the probe description and also add some tests. |
Data Review
Yes, through the metrics.yaml file and the Glean Dictionary
Yes, through the "Send Usage Data" preference in the application settings
N/A, collection set to end or be renewed by 2022-11-01
Category 2, Interaction data
default-on
No
Yes
No Resultdata-review+ |
Looks good. I've data reviewed. Please update the data-review link in metrics.yml. Thanks |
Thank you Roger! |
Kimmy said will also leave a comment on whether these events look right or not. |
Kimmy confirmed these events look good. |
Ohh I see @rocketsroger requested changes and the merge is blocked. |
3 new events added for when the user opens a tab, closes one or closes all from a search group.
Kimmy to confirm if the current https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/history_search_term_group_tapped is okay to be considered the 4th we needed.
Pull Request checklist
To download an APK when reviewing a PR: