Add FXIOS-12509 [TA 2025] Add event for history migration and rework logic to only migrate on upgrades, not fresh installs #27267
Conversation
… old metrics ping data.
…migration for fresh installs / first launches (as opposed to upgrades).
| let isFirstRun = introScreenManager.shouldShowIntroScreen | ||
|
|
||
| // If this is a first run, there won't be history to migrate since we are far past v110 | ||
| guard !isFirstRun else { | ||
| // Mark migration as succeeded and return early | ||
| UserDefaults.standard.setValue(true, forKey: PrefsKeys.PlacesHistoryMigrationSucceeded) | ||
| return | ||
| } |
There was a problem hiding this comment.
cc @OrlaM and @lmarceau with regards to our slack conversation today
Do either of you foresee any issues with me hooking into introScreenManager.shouldShowIntroScreen to determine if this is the first time the app has been launched?
My thinking is that we should always mark migration as complete for first launch of the app. That way we can record the telemetry event just when an actual migration is potentially needed (i.e. an upgrade).
There was a problem hiding this comment.
The other thing maybe we could use is InstallType.fresh ? But that's setup in setUpPostLaunchDependencies. Otherwise I think it's a fair trade to use the introScreenManager.shouldShowIntroScreen
Client.app: Coverage: 35.38
Generated by 🚫 Danger Swift against a920dce |
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.
We want to know if we can remove our code related to the application services history migration for v110.
This will help us improve the maintainability of our code as we can remove legacy / unused migration code.
We tried to use preexisting telemetry to answer these questions, but we could only infer usage at best. Moreover, the logic for when the telemetry was recorded was insufficient for the questions we had (migration telemetry was recorded on every first launch of the app, regardless of whether a migration was in fact performed).
No.
Note that the data steward reviewing your request will characterize your data collection based on the highest (and most sensitive) category.
This collection is documented in the Glean Dictionary at https://dictionary.telemetry.mozilla.org/
Only for a few months. As long as usage is low, we can remove this event.
New users and existing users. Release channel, all countries.
User has the general option of disable telemetry or/and studies.
We will use event volume to determine whether we can retire old migration code.
Mozilla Internal (Product and Dev Team).
No. We will collect the info using Glean. We will analyze the data with the traditional Looker/SQL database that Mozilla uses for all other telemetry. |
|
FYI I omitted unit tests for this telemetry because it's expected to be short-lived. |
Co-authored-by: lmarceau <[email protected]>
📜 Tickets
Jira ticket
Github issue
💡 Description
We wanted to add an event to know how often we are firing application services history migration code from v110.
Preexisting telemetry was not serviceable and was removed in #27211.
This PR adds a new event to check how many users are using the history migration code. The logic was also reworked to only attempt migration if the app is not a fresh install.
We will be able to delete this code (and the event) if usage is none or very low.
Indentation was fixed + Sentry logs were updated for current best practices (parameters passed as extras instead of part of the event text).
📝 Checklist
@Mergifyio backport release/v120)