-
Notifications
You must be signed in to change notification settings - Fork 1.3k
MR2 Telemetry: Homescreen view count #22377
MR2 Telemetry: Homescreen view count #22377
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.
|
This new probe does have a bit of overlap with two existing probes: We may want to revisit the original probes come renewal time. |
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.
This new probe does have a bit of overlap with two existing probes: https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/start_on_home_enter_home_screen https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/home_screen_home_screen_displayed
We may want to revisit the original probes come renewal time.
Please put this under "What alternative methods did you consider to answer these questions? Why were they not sufficient?"
I was looking at this from an eng perspective and I can't see how this probe would give any new data that differs from home_screen_displayed
.
We should ask product/data why this new probe is needed probably.
@@ -230,7 +230,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { | |||
} | |||
|
|||
if (!shouldStartOnHome() && | |||
shouldNavigateBrowserFragmentOnCouldStart(savedInstanceState) |
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.
Good catch.
Yeah, I was thinking the same thing but wanted to get the code out first since it was a pretty minor addition. I'll reach out to Kimmy and comment back when I have an update. Thanks! |
Reached out to Kimmy:
Confirmed we need this new probe for running a particular report. |
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+ |
@@ -480,7 +480,10 @@ class HomeFragment : Fragment() { | |||
val profilerStartTime = requireComponents.core.engine.profiler?.getProfilerTime() | |||
|
|||
super.onViewCreated(view, savedInstanceState) | |||
context?.metrics?.track(Event.HomeScreenDisplayed) | |||
context?.metrics?.apply { | |||
track(Event.HomeScreenDisplayed) |
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.
Should we remove this then? since we will now have a counter?
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.
I wouldn't think so since the event should be, in theory, used to follow a sequence of user interactions, whereas the counter will be used for some quantitative calculations and comparisons.
* For mozilla-mobile#22146 - Added counter for home screen views * For mozilla-mobile#22146 - Added PR number to metrics Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
#22146
Pull Request checklist
To download an APK when reviewing a PR: