-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Backfill metrics pt. 1 #1067
Backfill metrics pt. 1 #1067
Changes from all commits
d42a15c
836e214
75dd80f
7787507
5190e80
6cf4e72
2de49bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
# This Source Code Form is subject to the terms of the Mozilla Public | ||
# License, v. 2.0. If a copy of the MPL was not distributed with this | ||
# file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||
|
||
|
||
$schema: moz://mozilla.org/schemas/glean/metrics/1-0-0 | ||
|
||
events: | ||
app_opened: | ||
type: event | ||
description: > | ||
A user opened the app | ||
extra_keys: | ||
source: | ||
description: "The method used to open Fenix. Possible values are: `app_icon`, `custom_tab` or `link`" | ||
bugs: | ||
- 968 | ||
data_reviews: | ||
- https://github.com/mozilla-mobile/fenix/pull/1067#issuecomment-474598673 | ||
notification_emails: | ||
- telemetry-client-dev@mozilla.com | ||
expires: "2020-03-01" | ||
search_bar_tapped: | ||
type: event | ||
description: > | ||
A user tapped the search bar | ||
extra_keys: | ||
source: | ||
description: "The view the user was on when they initiated the search (For example: `Home` or `Browser`)" | ||
bugs: | ||
- 959 | ||
data_reviews: | ||
- https://github.com/mozilla-mobile/fenix/pull/1067#issuecomment-474598673 | ||
notification_emails: | ||
- telemetry-client-dev@mozilla.com | ||
expires: "2020-03-01" | ||
entered_url: | ||
type: event | ||
description: > | ||
A user entered a url | ||
extra_keys: | ||
autocomplete: | ||
description: "A boolean that tells us whether the URL was autofilled by an Autocomplete suggestion" | ||
bugs: | ||
- 959 | ||
data_reviews: | ||
- https://github.com/mozilla-mobile/fenix/pull/1067#issuecomment-474598673 | ||
notification_emails: | ||
- telemetry-client-dev@mozilla.com | ||
expires: "2020-03-01" | ||
performed_search: | ||
type: event | ||
description: > | ||
A user performed a search | ||
extra_keys: | ||
search_suggestion: | ||
description: "A boolean that tells us whether or not the search term was suggested by the Awesomebar" | ||
bugs: | ||
- 959 | ||
data_reviews: | ||
- https://github.com/mozilla-mobile/fenix/pull/1067#issuecomment-474598673 | ||
notification_emails: | ||
- telemetry-client-dev@mozilla.com | ||
expires: "2020-03-01" | ||
|
||
metrics: | ||
default_browser: | ||
type: boolean | ||
description: > | ||
Is Fenix the default browser? | ||
send_in_pings: | ||
- metrics | ||
bugs: | ||
- 960 | ||
data_reviews: | ||
- https://github.com/mozilla-mobile/fenix/pull/1067#issuecomment-474598673 | ||
notification_emails: | ||
- telemetry-client-dev@mozilla.com | ||
expires: "2020-03-01" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,19 +4,55 @@ | |
package org.mozilla.fenix.components.metrics | ||
|
||
import android.content.Context | ||
import mozilla.components.service.glean.EventMetricType | ||
import mozilla.components.service.glean.Glean | ||
import mozilla.components.support.utils.Browsers | ||
import org.mozilla.fenix.BuildConfig | ||
import org.mozilla.fenix.utils.Settings | ||
import org.mozilla.fenix.debug.GleanMetrics.Metrics | ||
import org.mozilla.fenix.debug.GleanMetrics.Events | ||
|
||
private class EventWrapper<T : Enum<T>>( | ||
private val event: EventMetricType<T>, | ||
private val keyMapper: ((String) -> T)? = null | ||
) { | ||
fun track(event: Event) { | ||
val extras = if (keyMapper != null) { | ||
event.extras?.mapKeys { keyMapper.invoke(it.key) } | ||
} else { | ||
null | ||
} | ||
|
||
this.event.record(extras) | ||
} | ||
} | ||
|
||
private val Event.wrapper | ||
get() = when (this) { | ||
is Event.OpenedApp -> EventWrapper(Events.appOpened) { Events.appOpenedKeys.valueOf(it) } | ||
is Event.SearchBarTapped -> EventWrapper(Events.searchBarTapped) { Events.searchBarTappedKeys.valueOf(it) } | ||
is Event.EnteredUrl -> EventWrapper(Events.enteredUrl) { Events.enteredUrlKeys.valueOf(it) } | ||
is Event.PerformedSearch -> EventWrapper(Events.performedSearch) { Events.performedSearchKeys.valueOf(it) } | ||
else -> null | ||
} | ||
|
||
class GleanMetricsService(private val context: Context) : MetricsService { | ||
override fun start() { | ||
Glean.initialize(context) | ||
Glean.setUploadEnabled(IsGleanEnabled) | ||
|
||
Metrics.apply { | ||
defaultBrowser.set(Browsers.all(context).isDefaultBrowser) | ||
} | ||
} | ||
|
||
override fun track(event: Event) { } | ||
override fun track(event: Event) { | ||
event.wrapper?.track(event) | ||
} | ||
|
||
override fun shouldTrack(event: Event): Boolean = Settings.getInstance(context).isTelemetryEnabled | ||
override fun shouldTrack(event: Event): Boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Glean does this internally based on a persistent internal flag that can be toggled using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @travis79 Good to know, the secondary case for this method is we need a way to allow/block some metrics from being sent to some providers to support Leanplum There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then that's a good case for doing this in the client app. Glean currently doesn't have a way to disable a single specific metric at run-time, only through the metrics.yaml file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It probably wouldn't be too hard to make that work -- but maybe it doesn't buy much... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @travis79 @mdboom If you look at the code in |
||
return Settings.getInstance(context).isTelemetryEnabled && event.wrapper != null | ||
} | ||
|
||
companion object { | ||
private const val IsGleanEnabled = BuildConfig.TELEMETRY | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,13 @@ sealed class Event { | |
object AddBookmark : Event() | ||
object RemoveBookmark : Event() | ||
object OpenedBookmark : Event() | ||
object OpenedApp : Event() | ||
|
||
data class OpenedApp(val source: Source) : Event() { | ||
enum class Source { APP_ICON, LINK, CUSTOM_TAB } | ||
override val extras: Map<String, String>? | ||
get() = hashMapOf("source" to source.name) | ||
} | ||
|
||
object OpenedAppFirstRun : Event() | ||
object InteractWithSearchURLArea : Event() | ||
object SavedLoginandPassword : Event() | ||
|
@@ -38,7 +44,24 @@ sealed class Event { | |
object OpenedPocketStory : Event() | ||
object DarkModeEnabled : Event() | ||
|
||
val extras: Map<String, Any>? | ||
// Interaction Events | ||
data class SearchBarTapped(val source: Source) : Event() { | ||
enum class Source { HOME, BROWSER } | ||
override val extras: Map<String, String>? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a heads up -- the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually really like this change 👍 |
||
get() = mapOf("source" to source.name) | ||
} | ||
|
||
data class EnteredUrl(val autoCompleted: Boolean) : Event() { | ||
override val extras: Map<String, String>? | ||
get() = mapOf("autocomplete" to autoCompleted.toString()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I don't love all these booleans-to-strings, because it's less clear what is happening. @mdboom can Glean support non-string, string maps? I thought maps could be printed out as strings without every item being a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
data class PerformedSearch(val fromSearchSuggestion: Boolean) : Event() { | ||
override val extras: Map<String, String>? | ||
get() = mapOf("search_suggestion" to fromSearchSuggestion.toString()) | ||
} | ||
|
||
open val extras: Map<String, String>? | ||
get() = null | ||
} | ||
|
||
|
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.
Will it be useful to have a Product/Team email here as well, to be aware of expiry?