From 3f6b8e1ab88873281e43c506abe16808b64e3e27 Mon Sep 17 00:00:00 2001 From: Emily Kager Date: Fri, 13 Jul 2018 14:46:52 -0700 Subject: [PATCH] Send Histogram Data for Load Times Closes #2891 --- .../focus/fragment/BrowserFragment.java | 4 +- .../observer/AverageLoadTimeObserver.java | 49 ---------------- .../focus/observer/LoadTimeObserver.kt | 58 +++++++++++++++++++ .../focus/telemetry/TelemetryWrapper.kt | 38 +++++++----- 4 files changed, 83 insertions(+), 66 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/focus/observer/AverageLoadTimeObserver.java create mode 100644 app/src/main/java/org/mozilla/focus/observer/LoadTimeObserver.kt diff --git a/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.java b/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.java index 0a1d1de4650..d729856552f 100644 --- a/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.java +++ b/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.java @@ -65,7 +65,7 @@ import org.mozilla.focus.locale.LocaleAwareAppCompatActivity; import org.mozilla.focus.menu.browser.BrowserMenu; import org.mozilla.focus.menu.context.WebContextMenu; -import org.mozilla.focus.observer.AverageLoadTimeObserver; +import org.mozilla.focus.observer.LoadTimeObserver; import org.mozilla.focus.open.OpenWithFragment; import org.mozilla.focus.popup.PopupUtils; import org.mozilla.focus.session.NullSession; @@ -323,7 +323,7 @@ public boolean onEditorAction(TextView textView, int actionId, KeyEvent keyEvent setBlockingEnabled(session.isBlockingEnabled()); setShouldRequestDesktop(session.shouldRequestDesktopSite()); - session.getLoading().observe(this, new AverageLoadTimeObserver(session)); + LoadTimeObserver.addObservers(session, this); session.getLoading().observe(this, new NonNullObserver() { @Override diff --git a/app/src/main/java/org/mozilla/focus/observer/AverageLoadTimeObserver.java b/app/src/main/java/org/mozilla/focus/observer/AverageLoadTimeObserver.java deleted file mode 100644 index 2404c3d512a..00000000000 --- a/app/src/main/java/org/mozilla/focus/observer/AverageLoadTimeObserver.java +++ /dev/null @@ -1,49 +0,0 @@ -/* 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 org.mozilla.focus.observer; - -import android.os.SystemClock; -import android.support.annotation.NonNull; -import android.util.Log; - -import org.mozilla.focus.architecture.NonNullObserver; -import org.mozilla.focus.session.Session; -import org.mozilla.focus.telemetry.TelemetryWrapper; - -public class AverageLoadTimeObserver extends NonNullObserver { - - private static final String LOG_TAG = "AverageLoadTimeObserver"; - private long startLoadTime = 0; - private boolean loadStarted = false; - - private final Session session; - - public AverageLoadTimeObserver(@NonNull Session session) { - this.session = session; - } - - @Override - protected void onValueChanged(Boolean loading) { - if (loading) { - if (!loadStarted) { - startLoadTime = SystemClock.elapsedRealtime(); - Log.i(LOG_TAG, "zerdatime " + startLoadTime + - " - page load start"); - loadStarted = true; - - } - } else { - if (loadStarted) { - Log.i(LOG_TAG, "Loaded page at " + session.getUrl().getValue()); - long endTime = SystemClock.elapsedRealtime(); - Log.i(LOG_TAG, "zerdatime " + endTime + - " - page load stop"); - Log.i(LOG_TAG, (endTime - startLoadTime) + " - elapsed load"); - TelemetryWrapper.addLoadToAverage(endTime - startLoadTime); - loadStarted = false; - } - } - } -} diff --git a/app/src/main/java/org/mozilla/focus/observer/LoadTimeObserver.kt b/app/src/main/java/org/mozilla/focus/observer/LoadTimeObserver.kt new file mode 100644 index 00000000000..debed9c2235 --- /dev/null +++ b/app/src/main/java/org/mozilla/focus/observer/LoadTimeObserver.kt @@ -0,0 +1,58 @@ +package org.mozilla.focus.observer + +import android.os.SystemClock +import android.support.v4.app.Fragment +import android.util.Log +import org.mozilla.focus.architecture.NonNullObserver +import org.mozilla.focus.session.Session +import org.mozilla.focus.telemetry.TelemetryWrapper +import org.mozilla.focus.utils.UrlUtils + +object LoadTimeObserver { + private const val MIN_LOAD_TIME: Long = 40 + private const val MAX_PROGRESS = 99 + private const val LOG_TAG: String = "LoadTimeObserver" + + @JvmStatic + fun addObservers(session: Session, fragment: Fragment) { + var startLoadTime: Long = 0 + var urlLoading: String? = null + + session.loading.observe(fragment, object : NonNullObserver() { + public override fun onValueChanged(t: Boolean) { + if (t) { + if ((urlLoading != null && urlLoading != session.url.value) || urlLoading == null) { + urlLoading = session.url.value + startLoadTime = SystemClock.elapsedRealtime() + Log.i(LOG_TAG, "zerdatime $startLoadTime - page load $urlLoading start") + } + } else { + // Progress of 99 means the page completed loading and wasn't interrupted. + if (urlLoading != null && + session.url.value == urlLoading && + session.progress.value == MAX_PROGRESS) { + Log.i(LOG_TAG, "Loaded page at $session.url.value") + val endTime = SystemClock.elapsedRealtime() + Log.i(LOG_TAG, "zerdatime $endTime - page load stop") + val elapsedLoad = endTime - startLoadTime + Log.i(LOG_TAG, "$elapsedLoad - elapsed load") + // Even internal pages take longer than 40 ms to load, let's not send any loads faster than this + if (elapsedLoad > MIN_LOAD_TIME && !UrlUtils.isLocalizedContent(urlLoading)) { + Log.i(LOG_TAG, "Sent load to histogram") + TelemetryWrapper.addLoadToHistogram(elapsedLoad) + } + } + } + } + }) + session.url.observe(fragment, object : NonNullObserver() { + public override fun onValueChanged(t: String) { + if ((urlLoading != null && urlLoading != t) || urlLoading == null) { + startLoadTime = SystemClock.elapsedRealtime() + Log.i(LOG_TAG, "zerdatime $startLoadTime - url changed to $t, new page load start") + urlLoading = t + } + } + }) + } +} diff --git a/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt b/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt index 965752a8462..a72d1e0fdb3 100644 --- a/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt +++ b/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt @@ -56,6 +56,10 @@ object TelemetryWrapper { private const val MAXIMUM_CUSTOM_TAB_EXTRAS = 10 + private const val HISTOGRAM_SIZE = 200 + private const val BUCKET_SIZE_MS = 100 + private const val HISTOGRAM_MIN_INDEX = 0 + private val isEnabledByDefault: Boolean get() = !AppConstants.isKlarBuild() @@ -165,7 +169,7 @@ object TelemetryWrapper { val SOURCE = "source" val SUCCESS = "success" val ERROR_CODE = "error_code" - val AVERAGE = "average" + val HISTOGRAM = "histogram" } enum class BrowserContextMenuValue { @@ -287,19 +291,19 @@ object TelemetryWrapper { TelemetryEvent.create(Category.ACTION, Method.FOREGROUND, Object.APP).queue() } - private var numLoads: Int = 0 - private var averageTime: Double = 0.0 + private var histogram = IntArray(HISTOGRAM_SIZE) @JvmStatic - fun addLoadToAverage(newLoadTime: Long) { - numLoads++ - averageTime += (newLoadTime - averageTime) / numLoads - } + fun addLoadToHistogram(newLoadTime: Long) { + var histogramLoadIndex = (newLoadTime / BUCKET_SIZE_MS).toInt() - @JvmStatic - private fun resetAverageLoad() { - numLoads = 0 - averageTime = 0.0 + if (histogramLoadIndex > (HISTOGRAM_SIZE - 2)) { + histogramLoadIndex = HISTOGRAM_SIZE - 1 + } else if (histogramLoadIndex < HISTOGRAM_MIN_INDEX) { + histogramLoadIndex = HISTOGRAM_MIN_INDEX + } + + histogram[histogramLoadIndex]++ } @JvmStatic @@ -319,11 +323,15 @@ object TelemetryWrapper { fun stopSession() { TelemetryHolder.get().recordSessionEnd() - if (numLoads > 0) { - TelemetryEvent.create(Category.ACTION, Method.FOREGROUND, Object.BROWSER) - .extra(Extra.AVERAGE, averageTime.toString()).queue() - resetAverageLoad() + val histogramEvent = TelemetryEvent.create(Category.ACTION, Method.FOREGROUND, Object.BROWSER) + val histogramAsJSONObject = JSONObject() + for (bucketIndex in histogram.indices) { + histogramAsJSONObject.put((bucketIndex * BUCKET_SIZE_MS).toString(), histogram[bucketIndex]) } + histogramEvent.extra(Extra.HISTOGRAM, histogramAsJSONObject.toString()).queue() + + // Clear histogram array after queueing it + histogram = IntArray(HISTOGRAM_SIZE) TelemetryEvent.create(Category.ACTION, Method.BACKGROUND, Object.APP).queue() }