Commit 6f9b7001 authored by MozLando's avatar MozLando
Browse files

Merge #6802



6802: Closes #4992: Emit provider duration facts from BrowserAwesomeBar r=csadilek a=grigoryk

Some things to consider and trade-offs:

When recording durations of various providers into a glean metric, we have a bit of a hurdle.
One approach would be to encapsulate all of the awesomebar-related perf telemetry entirely within the
awesomebar a-c component. But, set of providers isn't entirely known to us at the a-c level.
We know what providers we have defined, but we don't know what providers applications will provide themselves.
Also, glean doesn't have a metric of type (string->timespan), which would allow us to work-around this.
So, we can't define ping/metrics in the a-c component. This means that they need to be defined elsewhere, while
the measurement happens within the component. An established pattern for that in the codebase is emitting "facts",
which is what this patch does.

We delegate to the consuming application to then actually do something with these facts - e.g. map providers
to concrete glean metric definitions.

Another consideration is if we should try to group timings related to a single query. That's hard to do reliably,
and will introduce additional complexity into what's otherwise a super simple setup. Source of that complexity
is that we actively try to cancel query jobs as user is typing; our providers could take an arbitrary time to resolve,
and so grouping becomes difficult. However, we care about improving how long each individual provider takes,
since that's a good proxy for "how responsive is this UI?". Simply recording each individual timing we see
should be enough for that.
Co-authored-by: default avatarGrisha Kruglov <gkruglov@mozilla.com>
parents ba0d00e9 d63dc810
......@@ -30,6 +30,7 @@ dependencies {
api project(':concept-awesomebar')
implementation project(':support-ktx')
implementation project(':support-base')
api Dependencies.androidx_recyclerview
implementation Dependencies.androidx_constraintlayout
......
......@@ -5,6 +5,7 @@
package mozilla.components.browser.awesomebar
import android.content.Context
import android.os.SystemClock
import android.util.AttributeSet
import android.util.LruCache
import androidx.annotation.VisibleForTesting
......@@ -17,6 +18,7 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import mozilla.components.browser.awesomebar.facts.emitProviderQueryTimingFact
import mozilla.components.browser.awesomebar.layout.SuggestionLayout
import mozilla.components.browser.awesomebar.transform.SuggestionTransformer
import mozilla.components.concept.awesomebar.AwesomeBar
......@@ -30,7 +32,7 @@ internal const val INITIAL_NUMBER_OF_PROVIDERS = 5
/**
* A customizable [AwesomeBar] implementation.
*/
@Suppress("TooManyFunctions")
@Suppress("TooManyFunctions", "LargeClass")
class BrowserAwesomeBar @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null,
......@@ -154,7 +156,25 @@ class BrowserAwesomeBar @JvmOverloads constructor(
providers.forEach { provider ->
launch {
// At this point, we have a timing value for a provider.
// We have a choice here - we can try grouping different timings together for a
// single user input value, or we can just treat them as entirely independent values.
// These timings are correlated with each other in a sense that they act on the same
// inputs, and are executed at the same time. However, our goal here is to track performance
// of the providers. Each provider acts independently from another; recording their performance
// at an individual level will allow us to track that performance over time.
// Tracked value will be reflected both in perceived user experience (how quickly results from
// a provider show up), and in a purely technical interpretation of how quickly providers
// fulfill requests.
// Grouping also poses timing challenges - as user is typing, we're trying to cancel these
// provider requests. Given that each request can take an arbitrary amount of time to execute,
// grouping correctly becomes tricky and we run a risk of omitting certain values - or, of
// adding a bunch of complexity just for the sake of "correct grouping".
val start = SystemClock.elapsedRealtimeNanos()
val suggestions = withContext(jobDispatcher) { provider.block() }
val end = SystemClock.elapsedRealtimeNanos()
emitProviderQueryTimingFact(provider, timing = end - start)
val processedSuggestions = processProviderSuggestions(suggestions)
suggestionsAdapter.addSuggestions(
provider,
......
/* 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/. */
package mozilla.components.browser.awesomebar.facts
import mozilla.components.concept.awesomebar.AwesomeBar
import mozilla.components.support.base.Component
import mozilla.components.support.base.facts.Action
import mozilla.components.support.base.facts.Fact
import mozilla.components.support.base.facts.collect
/**
* Facts emitted for telemetry related to the AwesomeBar feature.
*/
class BrowserAwesomeBarFacts {
/**
* Specific types of telemetry items.
*/
object Items {
const val PROVIDER_DURATION = "provider_duration"
}
/**
* Keys used to record metadata about [Items].
*/
object MetadataKeys {
const val DURATION_PAIR = "duration_pair"
}
}
private fun emitAwesomebarFact(
action: Action,
item: String,
value: String? = null,
metadata: Map<String, Any>? = null
) {
Fact(
Component.BROWSER_AWESOMEBAR,
action,
item,
value,
metadata
).collect()
}
internal fun emitProviderQueryTimingFact(provider: AwesomeBar.SuggestionProvider, timing: Long) {
emitAwesomebarFact(
Action.INTERACTION,
BrowserAwesomeBarFacts.Items.PROVIDER_DURATION,
metadata = mapOf(
BrowserAwesomeBarFacts.MetadataKeys.DURATION_PAIR to (provider to timing)
)
)
}
......@@ -11,8 +11,14 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import mozilla.components.browser.awesomebar.facts.BrowserAwesomeBarFacts
import mozilla.components.browser.awesomebar.transform.SuggestionTransformer
import mozilla.components.concept.awesomebar.AwesomeBar
import mozilla.components.support.base.Component
import mozilla.components.support.base.facts.Action
import mozilla.components.support.base.facts.Fact
import mozilla.components.support.base.facts.FactProcessor
import mozilla.components.support.base.facts.Facts
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.test.rule.MainCoroutineRule
......@@ -28,6 +34,7 @@ import org.mockito.Mockito.inOrder
import org.mockito.Mockito.never
import org.mockito.Mockito.spy
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyZeroInteractions
import org.robolectric.Shadows.shadowOf
import java.util.UUID
import java.util.concurrent.Executor
......@@ -43,22 +50,52 @@ class BrowserAwesomeBarTest {
fun `BrowserAwesomeBar forwards input to providers`() {
runBlocking {
val provider1 = mockProvider()
val awesomeBar = BrowserAwesomeBar(testContext)
val provider2 = mockProvider()
val provider3 = mockProvider()
awesomeBar.addProviders(provider1)
val awesomeBar = BrowserAwesomeBar(testContext)
awesomeBar.addProviders(provider1, provider2)
awesomeBar.addProviders(provider3)
val facts = mutableListOf<Fact>()
Facts.registerProcessor(object : FactProcessor {
override fun process(fact: Fact) {
facts.add(fact)
}
})
awesomeBar.onInputChanged("Hello World!")
awesomeBar.awaitForAllJobsToFinish()
verify(provider1).onInputChanged("Hello World!")
verify(provider2).onInputChanged("Hello World!")
verify(provider3).onInputChanged("Hello World!")
verifyZeroInteractions(provider2)
verifyZeroInteractions(provider3)
assertEquals(1, facts.size)
assertBrowserAwesomebarFact(facts[0], provider1)
awesomeBar.addProviders(provider2, provider3)
awesomeBar.onInputChanged("Hello Back!")
awesomeBar.awaitForAllJobsToFinish()
verify(provider2).onInputChanged("Hello Back!")
verify(provider3).onInputChanged("Hello Back!")
assertEquals(4, facts.size)
facts.forEach { assertBrowserAwesomebarFact(it) }
}
}
private fun assertBrowserAwesomebarFact(f: Fact, provider: AwesomeBar.SuggestionProvider? = null) {
assertEquals(Component.BROWSER_AWESOMEBAR, f.component)
assertEquals(Action.INTERACTION, f.action)
assertEquals(BrowserAwesomeBarFacts.Items.PROVIDER_DURATION, f.item)
val pair = f.metadata!![BrowserAwesomeBarFacts.MetadataKeys.DURATION_PAIR] as Pair<*, *>
provider?.let { assertEquals(pair.first, provider) }
assertTrue(pair.second is Long)
}
@Test
fun `BrowserAwesomeBar forwards onInputStarted to providers`() {
val provider1: AwesomeBar.SuggestionProvider = mock()
......
......@@ -8,7 +8,6 @@ import mozilla.components.browser.icons.BrowserIcons
import mozilla.components.browser.icons.IconRequest
import mozilla.components.concept.awesomebar.AwesomeBar
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.concept.storage.HistoryStorage
import mozilla.components.concept.storage.SearchResult
import mozilla.components.feature.session.SessionUseCases
......@@ -20,7 +19,7 @@ internal const val HISTORY_SUGGESTION_LIMIT = 20
* A [AwesomeBar.SuggestionProvider] implementation that provides suggestions based on the browsing
* history stored in the [HistoryStorage].
*
* @property historyStorage and instance of the [BookmarksStorage] used
* @property historyStorage and instance of the [HistoryStorage] used
* to query matching bookmarks.
* @property loadUrlUseCase the use case invoked to load the url when the
* user clicks on the suggestion.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment