Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Closes #19147: Move startup metrics to right after Glean initialization #19252

Merged
merged 1 commit into from
May 5, 2021
Merged

Closes #19147: Move startup metrics to right after Glean initialization #19252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 30 additions & 30 deletions app/metrics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,8 +1269,8 @@ metrics:
expires: "2021-08-11"

preferences:
show_search_suggestions:
type: string_list
search_suggestions_enabled:
type: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for your awareness: This will work, but land in a different column.
Old: metrics.string_list.preferences_show_search_suggestions
New: metrics.boolean.preferences_show_search_suggestions

That means comparing these across versions is a bit more involved, but not impossible.
(There should never be a client that has both set though)

(I'm still waiting for confirmation that what I said is actually true)

Copy link
Contributor Author

@rocketsroger rocketsroger Apr 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please confirm that this is OK. Since this workaround and because currently there are high number of startup metric pings that have null values, I would argue that comparing against data we've collected before is not very useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree it sounds like it's worthwhile to make the change. You still may want to add in the description field "Prior to release X, this metric was of type string_list" so that if someone wanted to trace back through the (problematic) historical data they'd at least know how...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of change is not supported by our tooling, a similar change of a string to a datetime in the past has caused delays on our schema deploys: mozilla/mozilla-schema-generator#118

Type changes is typically frowned upon at a low level because it breaks schema evolution rules that allows for low operational overhead during ingestion time. It's possible to support changing types in the schema generator based on the behavior of moving the probe to a new namespace (which avoids a schema incompatible change). However, this will be confusing from a metadata perspective since we lose context on the type change.

I suggest expiring these probes and creating a new probe with the correct type, while clearly documenting the reason for deprecation so that tools like the glean dictionary can pick this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. I will look into expiring the probes and create new ones with the correct type. Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rocketsroger Can you ping me when you think you need a final review? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabrielluong Sure, this should be the final change but I'll ping you once I have the go ahead from the data science team. Thanks,

description: >
Whether or not the user has search suggestions enabled
default: true
Expand All @@ -1286,8 +1286,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
remote_debugging:
type: string_list
remote_debugging_enabled:
type: boolean
description: >
Whether or not the user has remote debugging enabled
default: false
Expand All @@ -1303,8 +1303,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
telemetry:
type: string_list
telemetry_enabled:
type: boolean
description: >
Whether or not the user has telemetry enabled. Note we should
never receive a "false" value for this since telemetry would
Expand All @@ -1322,8 +1322,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
tracking_protection:
type: string_list
enhanced_tracking_protection:
type: string
description: >
What type of enhanced tracking protection the user has enabled.
"standard," "strict," "custom," or "" (if disabled)
Expand All @@ -1340,8 +1340,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
search_bookmarks:
type: string_list
bookmarks_suggestion:
type: boolean
description: >
Whether or not the user has enabled bookmark search suggestions
default: true
Expand All @@ -1357,8 +1357,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
search_browsing_history:
type: string_list
browsing_history_suggestion:
type: boolean
description: >
Whether or not the user has enabled browsing history suggestions.
default: true
Expand All @@ -1374,8 +1374,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
show_clipboard_suggestions:
type: string_list
clipboard_suggestions_enabled:
type: boolean
description: >
Whether or not the user has enabled clipboard search suggestions.
default: true
Expand All @@ -1391,8 +1391,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
show_search_shortcuts:
type: string_list
search_shortcuts_enabled:
type: boolean
description: >
Whether or not the user has enabled search shortcuts.
default: true
Expand All @@ -1408,8 +1408,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
open_links_in_a_private_tab:
type: string_list
open_links_in_private:
type: boolean
description: >
Whether or not the user has enabled open links in a private tab.
default: false
Expand All @@ -1425,8 +1425,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
sync:
type: string_list
signed_in_sync:
type: boolean
description: >
Whether or not the user is signed into FxA
default: false
Expand Down Expand Up @@ -1461,8 +1461,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
show_voice_search:
type: string_list
voice_search_enabled:
type: boolean
description: >
Whether or not the user has enabled the voice search button.
default: true
Expand All @@ -1478,8 +1478,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
search_suggestions_private:
type: string_list
private_search_suggestions:
type: boolean
description: >
Whether or not the user has enabled showing search suggestions
in private mode.
Expand All @@ -1496,8 +1496,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
toolbar_position:
type: string_list
toolbar_position_setting:
type: string
description: >
The position of the toolbar
default: bottom (defaults to top if the user has accessibility services)
Expand Down Expand Up @@ -1531,8 +1531,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
open_links_in_app:
type: string_list
open_links_in_app_enabled:
type: boolean
description: >
Whether or not the user has the open links in apps feature enabled.
default: false
Expand All @@ -1548,8 +1548,8 @@ preferences:
notification_emails:
- fenix-core@mozilla.com
expires: "2021-08-01"
theme:
type: string_list
user_theme:
type: string
description: >
The theme the user has enabled. "light," "dark," "system," or "battery"
default: "system" for API 28+, else "light"
Expand Down
185 changes: 178 additions & 7 deletions app/src/main/java/org/mozilla/fenix/FenixApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ import org.mozilla.fenix.session.VisibilityLifecycleCallback
import org.mozilla.fenix.telemetry.TelemetryLifecycleObserver
import org.mozilla.fenix.utils.BrowsersCache
import java.util.concurrent.TimeUnit
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.search.ext.buildSearchUrl
import mozilla.components.feature.search.ext.waitForSelectedOrDefaultSearchEngine
import mozilla.components.service.fxa.manager.SyncEnginesStorage
import org.mozilla.fenix.GleanMetrics.Addons
import org.mozilla.fenix.GleanMetrics.Preferences
import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MozillaProductDetector
import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.utils.Settings

/**
*The main application class for Fenix. Records data to measure initialization performance.
Expand Down Expand Up @@ -130,13 +141,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
buildInfo = GleanBuildInfo.buildInfo
)

// Set this early to guarantee it's in every ping from here on.
Metrics.distributionId.set(
when (Config.channel.isMozillaOnline) {
true -> "MozillaOnline"
false -> "Mozilla"
}
)
setStartupMetrics(components.core.store, settings())
rocketsroger marked this conversation as resolved.
Show resolved Hide resolved
}

@CallSuper
Expand Down Expand Up @@ -499,6 +504,172 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
}
}

/**
* This function is called right after Glean is initialized. Part of this function depends on
* shared preferences to be updated so the correct value is sent with the metrics ping.
*
* The reason we're using shared preferences to track these values is due to the limitations of
* the current metrics ping design. The values set here will be sent in every metrics ping even
* if these values have not changed since the last startup.
*/
@Suppress("ComplexMethod", "LongMethod")
@VisibleForTesting
internal fun setStartupMetrics(
browserStore: BrowserStore,
settings: Settings,
browsersCache: BrowsersCache = BrowsersCache,
mozillaProductDetector: MozillaProductDetector = MozillaProductDetector
) {
setPreferenceMetrics(settings)
with(Metrics) {
// Set this early to guarantee it's in every ping from here on.
distributionId.set(
when (Config.channel.isMozillaOnline) {
true -> "MozillaOnline"
false -> "Mozilla"
}
)

defaultBrowser.set(browsersCache.all(applicationContext).isDefaultBrowser)
mozillaProductDetector.getMozillaBrowserDefault(applicationContext)?.also {
defaultMozBrowser.set(it)
}

mozillaProducts.set(mozillaProductDetector.getInstalledMozillaProducts(applicationContext))

adjustCampaign.set(settings.adjustCampaignId)
adjustAdGroup.set(settings.adjustAdGroup)
adjustCreative.set(settings.adjustCreative)
adjustNetwork.set(settings.adjustNetwork)

searchWidgetInstalled.set(settings.searchWidgetInstalled)

val openTabsCount = settings.openTabsCount
hasOpenTabs.set(openTabsCount > 0)
if (openTabsCount > 0) {
tabsOpenCount.add(openTabsCount)
}

val topSitesSize = settings.topSitesSize
hasTopSites.set(topSitesSize > 0)
if (topSitesSize > 0) {
topSitesCount.add(topSitesSize)
}

val installedAddonSize = settings.installedAddonsCount
Addons.hasInstalledAddons.set(installedAddonSize > 0)
if (installedAddonSize > 0) {
Addons.installedAddons.set(settings.installedAddonsList.split(','))
}

val enabledAddonSize = settings.enabledAddonsCount
Addons.hasEnabledAddons.set(enabledAddonSize > 0)
if (enabledAddonSize > 0) {
Addons.enabledAddons.set(settings.enabledAddonsList.split(','))
}

val desktopBookmarksSize = settings.desktopBookmarksSize
hasDesktopBookmarks.set(desktopBookmarksSize > 0)
if (desktopBookmarksSize > 0) {
desktopBookmarksCount.add(desktopBookmarksSize)
}

val mobileBookmarksSize = settings.mobileBookmarksSize
hasMobileBookmarks.set(mobileBookmarksSize > 0)
if (mobileBookmarksSize > 0) {
mobileBookmarksCount.add(mobileBookmarksSize)
}

toolbarPosition.set(
when (settings.toolbarPosition) {
ToolbarPosition.BOTTOM -> Event.ToolbarPositionChanged.Position.BOTTOM.name
ToolbarPosition.TOP -> Event.ToolbarPositionChanged.Position.TOP.name
}
)

tabViewSetting.set(settings.getTabViewPingString())
closeTabSetting.set(settings.getTabTimeoutPingString())
}

browserStore.waitForSelectedOrDefaultSearchEngine { searchEngine ->
if (searchEngine != null) {
SearchDefaultEngine.apply {
code.set(searchEngine.id)
name.set(searchEngine.name)
submissionUrl.set(searchEngine.buildSearchUrl(""))
}
}
}
}

@Suppress("ComplexMethod")
private fun setPreferenceMetrics(
settings: Settings
) {
with(Preferences) {
searchSuggestionsEnabled.set(settings.shouldShowSearchSuggestions)
remoteDebuggingEnabled.set(settings.isRemoteDebuggingEnabled)
telemetryEnabled.set(settings.isTelemetryEnabled)
browsingHistorySuggestion.set(settings.shouldShowHistorySuggestions)
bookmarksSuggestion.set(settings.shouldShowBookmarkSuggestions)
clipboardSuggestionsEnabled.set(settings.shouldShowClipboardSuggestions)
searchShortcutsEnabled.set(settings.shouldShowSearchShortcuts)
openLinksInPrivate.set(settings.openLinksInAPrivateTab)
privateSearchSuggestions.set(settings.shouldShowSearchSuggestionsInPrivate)
voiceSearchEnabled.set(settings.shouldShowVoiceSearch)
openLinksInAppEnabled.set(settings.openLinksInExternalApp)

val isLoggedIn =
components.backgroundServices.accountManager.accountProfile() != null
signedInSync.set(isLoggedIn)

val syncedItems = SyncEnginesStorage(applicationContext).getStatus().entries.filter {
it.value
}.map { it.key.nativeName }
syncItems.set(syncedItems)

toolbarPositionSetting.set(
when {
settings.shouldUseFixedTopToolbar -> "fixed_top"
settings.shouldUseBottomToolbar -> "bottom"
else -> "top"
}
)

enhancedTrackingProtection.set(
when {
!settings.shouldUseTrackingProtection -> ""
settings.useStandardTrackingProtection -> "standard"
settings.useStrictTrackingProtection -> "strict"
settings.useCustomTrackingProtection -> "custom"
else -> ""
}
)

val accessibilitySelection = mutableListOf<String>()

if (settings.switchServiceIsEnabled) {
accessibilitySelection.add("switch")
}

if (settings.touchExplorationIsEnabled) {
accessibilitySelection.add("touch exploration")
}

accessibilityServices.set(accessibilitySelection.toList())

userTheme.set(
when {
settings.shouldUseLightTheme -> "light"
settings.shouldUseDarkTheme -> "dark"
settings.shouldFollowDeviceTheme -> "system"
settings.shouldUseAutoBatteryTheme -> "battery"
else -> ""
}
)
}
}

protected fun recordOnInit() {
// This gets called by more than one process. Ideally we'd only run this in the main process
// but the code to check which process we're in crashes because the Context isn't valid yet.
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/org/mozilla/fenix/components/Analytics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Analytics(
val metrics: MetricController by lazyMonitored {
MetricController.create(
listOf(
GleanMetricsService(context, lazy { context.components.core.store }),
GleanMetricsService(context),
AdjustMetricsService(context as Application)
),
isDataTelemetryEnabled = { context.settings().isTelemetryEnabled },
Expand Down
Loading