Send Histogram Data for Load Times Closes #2891

mozilla-mobile · Jul 21, 2018 · 3f6b8e1 · 3f6b8e1
1 parent 7090e81
commit 3f6b8e1
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 66 deletions.
diff --git 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<Boolean>() {
             @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
diff --git a/app/src/main/java/org/mozilla/focus/observer/LoadTimeObserver.kt 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<Boolean>() {
+            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<String>() {
+            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
@@ -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()
     }