From 437cc9b009eff28353b23e13237e779c1ca3b4e4 Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Tue, 24 Dec 2019 13:52:52 +0800 Subject: [PATCH] [SettingsLib] Add a metric for time spending in each Settings page Add elapsed time in the target page hidden metric Bug: 146610928 Test: robotest Change-Id: I94a8f25b3bafc0979de0da05fc69878a1169d431 --- .../core/instrumentation/EventLogWriter.java | 8 ++++++-- .../core/instrumentation/LogWriter.java | 2 +- .../instrumentation/MetricsFeatureProvider.java | 10 ++++++++-- .../instrumentation/VisibilityLoggerMixin.java | 17 ++++++++++------- .../VisibilityLoggerMixinTest.java | 8 ++++---- 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/EventLogWriter.java b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/EventLogWriter.java index 5b9281cb9d2ec..d84e57a38ee40 100644 --- a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/EventLogWriter.java +++ b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/EventLogWriter.java @@ -40,8 +40,12 @@ public class EventLogWriter implements LogWriter { } @Override - public void hidden(Context context, int category) { - MetricsLogger.hidden(context, category); + public void hidden(Context context, int category, int visibleTime) { + final LogMaker logMaker = new LogMaker(category) + .setType(MetricsProto.MetricsEvent.TYPE_CLOSE) + .addTaggedData(MetricsProto.MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, + visibleTime); + MetricsLogger.action(logMaker); } @Override diff --git a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/LogWriter.java b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/LogWriter.java index 9d9c17f3b4439..d4ef3d7b24a2a 100644 --- a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/LogWriter.java +++ b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/LogWriter.java @@ -31,7 +31,7 @@ public interface LogWriter { /** * Logs a visibility event when view becomes hidden. */ - void hidden(Context context, int category); + void hidden(Context context, int category, int visibleTime); /** * Logs an user action. diff --git a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/MetricsFeatureProvider.java b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/MetricsFeatureProvider.java index a82231a597c34..c34c365c1bfa6 100644 --- a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/MetricsFeatureProvider.java +++ b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/MetricsFeatureProvider.java @@ -83,9 +83,15 @@ public class MetricsFeatureProvider { } } - public void hidden(Context context, int category) { + /** + * Logs an event when target page is hidden. + * + * @param category the target page id + * @param visibleTime the time spending on target page since being visible + */ + public void hidden(Context context, int category, int visibleTime) { for (LogWriter writer : mLoggerWriters) { - writer.hidden(context, category); + writer.hidden(context, category, visibleTime); } } diff --git a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixin.java b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixin.java index 0a1a1226d0a3a..61e47f8f8dd88 100644 --- a/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixin.java +++ b/packages/SettingsLib/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixin.java @@ -40,7 +40,8 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnAttach { private MetricsFeatureProvider mMetricsFeature; private int mSourceMetricsCategory = MetricsProto.MetricsEvent.VIEW_UNKNOWN; - private long mTimestamp; + private long mCreationTimestamp; + private long mVisibleTimestamp; public VisibilityLoggerMixin(int metricsCategory, MetricsFeatureProvider metricsFeature) { mMetricsCategory = metricsCategory; @@ -49,7 +50,7 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnAttach { @Override public void onAttach() { - mTimestamp = SystemClock.elapsedRealtime(); + mCreationTimestamp = SystemClock.elapsedRealtime(); } @OnLifecycleEvent(Event.ON_RESUME) @@ -57,8 +58,9 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnAttach { if (mMetricsFeature == null || mMetricsCategory == METRICS_CATEGORY_UNKNOWN) { return; } - if (mTimestamp != 0L) { - final int elapse = (int) (SystemClock.elapsedRealtime() - mTimestamp); + mVisibleTimestamp = SystemClock.elapsedRealtime(); + if (mCreationTimestamp != 0L) { + final int elapse = (int) (mVisibleTimestamp - mCreationTimestamp); mMetricsFeature.visible(null /* context */, mSourceMetricsCategory, mMetricsCategory, elapse); } else { @@ -69,9 +71,10 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnAttach { @OnLifecycleEvent(Event.ON_PAUSE) public void onPause() { - mTimestamp = 0; + mCreationTimestamp = 0; if (mMetricsFeature != null && mMetricsCategory != METRICS_CATEGORY_UNKNOWN) { - mMetricsFeature.hidden(null /* context */, mMetricsCategory); + final int elapse = (int) (SystemClock.elapsedRealtime() - mVisibleTimestamp); + mMetricsFeature.hidden(null /* context */, mMetricsCategory, elapse); } } @@ -84,7 +87,7 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnAttach { if (mMetricsFeature == null || mMetricsCategory == METRICS_CATEGORY_UNKNOWN) { return; } - final int elapse = (int) (SystemClock.elapsedRealtime() - mTimestamp); + final int elapse = (int) (SystemClock.elapsedRealtime() - mCreationTimestamp); mMetricsFeature.action(METRICS_CATEGORY_UNKNOWN, action, mMetricsCategory, key, elapse); } diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixinTest.java b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixinTest.java index f070a37aa6599..7de36e8b0d898 100644 --- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixinTest.java +++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/instrumentation/VisibilityLoggerMixinTest.java @@ -88,7 +88,7 @@ public class VisibilityLoggerMixinTest { mMixin.onPause(); verify(mMetricsFeature, times(1)) - .hidden(nullable(Context.class), eq(TestInstrumentable.TEST_METRIC)); + .hidden(nullable(Context.class), eq(TestInstrumentable.TEST_METRIC), anyInt()); } @Test @@ -98,7 +98,7 @@ public class VisibilityLoggerMixinTest { mMixin.onPause(); verify(mMetricsFeature, never()) - .hidden(nullable(Context.class), anyInt()); + .hidden(nullable(Context.class), anyInt(), anyInt()); } @Test @@ -109,7 +109,7 @@ public class VisibilityLoggerMixinTest { mMixin.onPause(); verify(mMetricsFeature, never()) - .hidden(nullable(Context.class), anyInt()); + .hidden(nullable(Context.class), anyInt(), anyInt()); } @Test @@ -121,7 +121,7 @@ public class VisibilityLoggerMixinTest { verify(testActivity.mMetricsFeatureProvider, times(1)).visible(any(), anyInt(), anyInt(), anyInt()); ac.pause().stop().destroy(); - verify(testActivity.mMetricsFeatureProvider, times(1)).hidden(any(), anyInt()); + verify(testActivity.mMetricsFeatureProvider, times(1)).hidden(any(), anyInt(), anyInt()); } public static class TestActivity extends FragmentActivity {