Add FXIOS-10896 iOS Add additional fields to the usage ping

[razvanlitianu]

📜 Tickets

Jira ticket
Github issue

💡 Description

Add additional fields to the usage ping

📝 Checklist

You have to check all boxes before merging

• Filled in the above information (tickets numbers and description of your work)
• Updated the PR name to follow our PR naming guidelines
• Wrote unit tests and/or ensured the tests suite is passing
• When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
• If needed, I updated documentation / comments for complex code and public methods
• If needed, added a backport comment (example @Mergifyio backport release/v120)

[mobiletest-ci-bot]

	Messages
📖	Project coverage: 33.51%
📖	Edited 2 files
📖	Created 3 files

Client.app: Coverage: 32.38

File	Coverage
GleanUsageReporting.swift	20.27%	⚠️

Generated by 🚫 Danger Swift against d023b4b

[travis79]

A couple of suggestions, nothing to block on. Looks just as I expected and I really appreciate the consistency with the Fenix implementation. Bravo 🎉

[travis79]

You could also use the start() API of the metric, and then stopAndAccumulate() in the background handler if you didn't want to manage the timer yourself, but this is perfectly acceptable too. The benefit of letting Glean manage the timer is just that you don't have to, and glean uses a monotonic time source (I'm not sure what currentTimeProvider's specifications are for this, actually)

[travis79]

Please link to the original Glean duration for the data-review and bugs also, as that is the review that covers this collection. You can find these links in the Glean dictionary for this and other metrics which have been duplicated for the purpose of the usage-reporting ping.

[razvanlitianu]

@travis79 Just to double check, I searched Glean Dictionary and the only metrics from which we duplicated seems to be in the Glean repo here. Are these to be linked?

[travis79]

Yes, that is correct. Please include the data-review link(s) from the Glean definitions in the definitions you are adding. That way the data-review that covers the collection of these is linked in the metadata and easy to find from the Glean Dictionary and other tools. You can also copy over the linked bugs, but that isn't as necessary as the data-reviews. Thanks!

[razvanlitianu]

@travis79 I updated the PR with the requested changes. One more thing I would like to ask is how will this observer be working? Where will it be enabled, and what toggled should be tied to?

Added @nbhasin2 @dicarobinho for review.

[dicarobinho]

Is looking good to me

[travis79]

@travis79 I updated the PR with the requested changes. One more thing I would like to ask is how will this observer be working? Where will it be enabled, and what toggled should be tied to?

Added @nbhasin2 @dicarobinho for review.

Glean's internal observers are set up right after Glean is initialized, so I expect these to be set up early enough to trigger correctly. Glean will often be initialized after the first appWillEnterForeground event, so it triggers the first event as it registers the observers in order to not miss it: https://github.com/mozilla/glean/blob/main/glean-core/ios/Glean/Scheduler/GleanLifecycleObserver.swift#L28. I'm not sure if you will need to do something similar or not, but it was something necessary for Glean.

The preference should not be connected to whether or not the observers are registered, Glean will need to be able to still receive these events regardless of whether the usage-reporting preference is enabled. As long as the usage-reporting ping setEnabled is tied to the preference, then attempting to record metrics or submit the ping are no-ops aside from Glean cleaning up any data that might have been recorded.

[razvanlitianu]

@travis79 I updated the PR with the requested changes. One more thing I would like to ask is how will this observer be working? Where will it be enabled, and what toggled should be tied to?
Added @nbhasin2 @dicarobinho for review.

Glean's internal observers are set up right after Glean is initialized, so I expect these to be set up early enough to trigger correctly. Glean will often be initialized after the first appWillEnterForeground event, so it triggers the first event as it registers the observers in order to not miss it: https://github.com/mozilla/glean/blob/main/glean-core/ios/Glean/Scheduler/GleanLifecycleObserver.swift#L28. I'm not sure if you will need to do something similar or not, but it was something necessary for Glean.

The preference should not be connected to whether or not the observers are registered, Glean will need to be able to still receive these events regardless of whether the usage-reporting preference is enabled. As long as the usage-reporting ping setEnabled is tied to the preference, then attempting to record metrics or submit the ping are no-ops aside from Glean cleaning up any data that might have been recorded.

Okay, I was only asking this because on Android it seems there is some condition for the service to be registered based on a condition.