diff --git a/src/com/android/settings/core/InstrumentedPreferenceFragment.java b/src/com/android/settings/core/InstrumentedPreferenceFragment.java index bfb69e78a3b..a5d07156190 100644 --- a/src/com/android/settings/core/InstrumentedPreferenceFragment.java +++ b/src/com/android/settings/core/InstrumentedPreferenceFragment.java @@ -65,4 +65,8 @@ public abstract class InstrumentedPreferenceFragment extends ObservablePreferenc protected final Context getPrefContext() { return getPreferenceManager().getContext(); } + + protected final VisibilityLoggerMixin getVisibilityLogger() { + return mVisibilityLoggerMixin; + } } diff --git a/src/com/android/settings/core/instrumentation/EventLogWriter.java b/src/com/android/settings/core/instrumentation/EventLogWriter.java index e7628e8b6b7..3196f76b323 100644 --- a/src/com/android/settings/core/instrumentation/EventLogWriter.java +++ b/src/com/android/settings/core/instrumentation/EventLogWriter.java @@ -18,6 +18,7 @@ package com.android.settings.core.instrumentation; import android.content.Context; import android.metrics.LogMaker; +import android.util.Log; import android.util.Pair; import com.android.internal.logging.MetricsLogger; @@ -28,6 +29,8 @@ import com.android.internal.logging.nano.MetricsProto; */ public class EventLogWriter implements LogWriter { + private final MetricsLogger mMetricsLogger = new MetricsLogger(); + public void visible(Context context, int source, int category) { final LogMaker logMaker = new LogMaker(category) .setType(MetricsProto.MetricsEvent.TYPE_OPEN) @@ -39,6 +42,24 @@ public class EventLogWriter implements LogWriter { MetricsLogger.hidden(context, category); } + public void action(int category, int value, Pair... taggedData) { + if (taggedData == null || taggedData.length == 0) { + mMetricsLogger.action(category, value); + } else { + final LogMaker logMaker = new LogMaker(category) + .setType(MetricsProto.MetricsEvent.TYPE_ACTION) + .setSubtype(value); + for (Pair pair : taggedData) { + logMaker.addTaggedData(pair.first, pair.second); + } + mMetricsLogger.write(logMaker); + } + } + + public void action(int category, boolean value, Pair... taggedData) { + action(category, value ? 1 : 0, taggedData); + } + public void action(Context context, int category, Pair... taggedData) { action(context, category, "", taggedData); } @@ -52,12 +73,16 @@ public class EventLogWriter implements LogWriter { MetricsLogger.action(logMaker); } + /** @deprecated use {@link #action(int, int, Pair[])} */ + @Deprecated public void action(Context context, int category, int value) { - MetricsLogger.action(context, category, Integer.toString(value)); + MetricsLogger.action(context, category, value); } + /** @deprecated use {@link #action(int, boolean, Pair[])} */ + @Deprecated public void action(Context context, int category, boolean value) { - MetricsLogger.action(context, category, Boolean.toString(value)); + MetricsLogger.action(context, category, value); } public void action(Context context, int category, String pkg, diff --git a/src/com/android/settings/core/instrumentation/LogWriter.java b/src/com/android/settings/core/instrumentation/LogWriter.java index 584217d85cb..062d46f759f 100644 --- a/src/com/android/settings/core/instrumentation/LogWriter.java +++ b/src/com/android/settings/core/instrumentation/LogWriter.java @@ -33,6 +33,16 @@ public interface LogWriter { */ void hidden(Context context, int category); + /** + * Logs a user action. + */ + void action(int category, int value, Pair... taggedData); + + /** + * Logs a user action. + */ + void action(int category, boolean value, Pair... taggedData); + /** * Logs an user action. */ @@ -45,12 +55,16 @@ public interface LogWriter { /** * Logs an user action. + * @deprecated use {@link #action(int, int, Pair[])} */ + @Deprecated void action(Context context, int category, int value); /** * Logs an user action. + * @deprecated use {@link #action(int, boolean, Pair[])} */ + @Deprecated void action(Context context, int category, boolean value); /** diff --git a/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java index afdec558556..532ec66316f 100644 --- a/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java +++ b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java @@ -21,7 +21,7 @@ import android.content.Intent; import android.text.TextUtils; import android.util.Pair; -import com.android.internal.logging.nano.MetricsProto; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import java.util.ArrayList; import java.util.List; @@ -60,18 +60,44 @@ public class MetricsFeatureProvider { } } + /** + * Logs a user action. Includes the elapsed time since the containing + * fragment has been visible. + */ + public void action(VisibilityLoggerMixin visibilityLogger, int category, int value) { + for (LogWriter writer : mLoggerWriters) { + writer.action(category, value, + sinceVisibleTaggedData(visibilityLogger.elapsedTimeSinceVisible())); + } + } + + /** + * Logs a user action. Includes the elapsed time since the containing + * fragment has been visible. + */ + public void action(VisibilityLoggerMixin visibilityLogger, int category, boolean value) { + for (LogWriter writer : mLoggerWriters) { + writer.action(category, value, + sinceVisibleTaggedData(visibilityLogger.elapsedTimeSinceVisible())); + } + } + public void action(Context context, int category, Pair... taggedData) { for (LogWriter writer : mLoggerWriters) { writer.action(context, category, taggedData); } } + /** @deprecated use {@link #action(VisibilityLoggerMixin, int, int)} */ + @Deprecated public void action(Context context, int category, int value) { for (LogWriter writer : mLoggerWriters) { writer.action(context, category, value); } } + /** @deprecated use {@link #action(VisibilityLoggerMixin, int, boolean)} */ + @Deprecated public void action(Context context, int category, boolean value) { for (LogWriter writer : mLoggerWriters) { writer.action(context, category, value); @@ -99,7 +125,7 @@ public class MetricsFeatureProvider { public int getMetricsCategory(Object object) { if (object == null || !(object instanceof Instrumentable)) { - return MetricsProto.MetricsEvent.VIEW_UNKNOWN; + return MetricsEvent.VIEW_UNKNOWN; } return ((Instrumentable) object).getMetricsCategory(); } @@ -116,15 +142,19 @@ public class MetricsFeatureProvider { // Not loggable return; } - action(context, MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK, action, - Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); + action(context, MetricsEvent.ACTION_SETTINGS_TILE_CLICK, action, + Pair.create(MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); return; } else if (TextUtils.equals(cn.getPackageName(), context.getPackageName())) { // Going to a Setting internal page, skip click logging in favor of page's own // visibility logging. return; } - action(context, MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK, cn.flattenToString(), - Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); + action(context, MetricsEvent.ACTION_SETTINGS_TILE_CLICK, cn.flattenToString(), + Pair.create(MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); + } + + private Pair sinceVisibleTaggedData(long timestamp) { + return Pair.create(MetricsEvent.NOTIFICATION_SINCE_VISIBLE_MILLIS, timestamp); } } diff --git a/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java b/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java index bbdf8c94983..697f0a3c902 100644 --- a/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java +++ b/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java @@ -45,6 +45,14 @@ public class SettingSuggestionsLogWriter implements LogWriter { public void actionWithSource(Context context, int source, int category) { } + @Override + public void action(int category, int value, Pair... taggedData) { + } + + @Override + public void action(int category, boolean value, Pair... taggedData) { + } + @Override public void action(Context context, int category, int value) { } diff --git a/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java b/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java index 8de35adfd79..2fe2a3beb1d 100644 --- a/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java +++ b/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java @@ -20,6 +20,7 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; +import android.os.SystemClock; import com.android.internal.logging.nano.MetricsProto; import com.android.settings.SettingsActivity; import com.android.settings.overlay.FeatureFactory; @@ -41,6 +42,7 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPau private MetricsFeatureProvider mMetricsFeature; private int mSourceMetricsCategory = MetricsProto.MetricsEvent.VIEW_UNKNOWN; + private long mVisibleTimestamp; public VisibilityLoggerMixin(int metricsCategory) { // MetricsFeature will be set during onAttach. @@ -59,6 +61,7 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPau @Override public void onResume() { + mVisibleTimestamp = SystemClock.elapsedRealtime(); if (mMetricsFeature != null && mMetricsCategory != METRICS_CATEGORY_UNKNOWN) { mMetricsFeature.visible(null /* context */, mSourceMetricsCategory, mMetricsCategory); } @@ -66,6 +69,7 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPau @Override public void onPause() { + mVisibleTimestamp = 0; if (mMetricsFeature != null && mMetricsCategory != METRICS_CATEGORY_UNKNOWN) { mMetricsFeature.hidden(null /* context */, mMetricsCategory); } @@ -85,4 +89,12 @@ public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPau mSourceMetricsCategory = intent.getIntExtra(SettingsActivity.EXTRA_SOURCE_METRICS_CATEGORY, MetricsProto.MetricsEvent.VIEW_UNKNOWN); } + + /** Returns elapsed time since onResume() */ + public long elapsedTimeSinceVisible() { + if (mVisibleTimestamp == 0) { + return 0; + } + return SystemClock.elapsedRealtime() - mVisibleTimestamp; + } } diff --git a/src/com/android/settings/wifi/WifiSettings.java b/src/com/android/settings/wifi/WifiSettings.java index b47391d0dd7..b143f5854de 100644 --- a/src/com/android/settings/wifi/WifiSettings.java +++ b/src/com/android/settings/wifi/WifiSettings.java @@ -1070,7 +1070,7 @@ public class WifiSettings extends RestrictedSettingsFragment protected void connect(final WifiConfiguration config, boolean isSavedNetwork) { // Log subtype if configuration is a saved network. - mMetricsFeatureProvider.action(getActivity(), MetricsEvent.ACTION_WIFI_CONNECT, + mMetricsFeatureProvider.action(getVisibilityLogger(), MetricsEvent.ACTION_WIFI_CONNECT, isSavedNetwork); mWifiManager.connect(config, mConnectListener); mClickedConnect = true; diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java index ea33c83391f..ff91c4099d3 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java @@ -28,6 +28,8 @@ import com.android.settings.overlay.FeatureFactory; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RuntimeEnvironment; @@ -42,24 +44,35 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class MetricsFeatureProviderTest { + private static int CATEGORY = 10; + private static boolean SUBTYPE_BOOLEAN = true; + private static int SUBTYPE_INTEGER = 1; + private static long ELAPSED_TIME = 1000; + + @Mock private LogWriter mockLogWriter; + @Mock private VisibilityLoggerMixin mockVisibilityLogger; - @Mock - private LogWriter mLogWriter; private Context mContext; private MetricsFeatureProvider mProvider; + @Captor + private ArgumentCaptor mPairCaptor; + @Before public void setUp() { MockitoAnnotations.initMocks(this); mContext = RuntimeEnvironment.application; mProvider = new MetricsFeatureProvider(); List writers = new ArrayList<>(); - writers.add(mLogWriter); + writers.add(mockLogWriter); ReflectionHelpers.setField(mProvider, "mLoggerWriters", writers); + + when(mockVisibilityLogger.elapsedTimeSinceVisible()).thenReturn(ELAPSED_TIME); } @Test @@ -77,7 +90,7 @@ public class MetricsFeatureProviderTest { mProvider.logDashboardStartIntent(mContext, null /* intent */, MetricsEvent.SETTINGS_GESTURES); - verifyNoMoreInteractions(mLogWriter); + verifyNoMoreInteractions(mockLogWriter); } @Test @@ -86,7 +99,7 @@ public class MetricsFeatureProviderTest { mProvider.logDashboardStartIntent(mContext, intent, MetricsEvent.SETTINGS_GESTURES); - verify(mLogWriter).action( + verify(mockLogWriter).action( eq(mContext), eq(MetricsEvent.ACTION_SETTINGS_TILE_CLICK), anyString(), @@ -99,10 +112,32 @@ public class MetricsFeatureProviderTest { mProvider.logDashboardStartIntent(mContext, intent, MetricsEvent.SETTINGS_GESTURES); - verify(mLogWriter).action( + verify(mockLogWriter).action( eq(mContext), eq(MetricsEvent.ACTION_SETTINGS_TILE_CLICK), anyString(), eq(Pair.create(MetricsEvent.FIELD_CONTEXT, MetricsEvent.SETTINGS_GESTURES))); } + + @Test + public void action_BooleanLogsElapsedTime() { + mProvider.action(mockVisibilityLogger, CATEGORY, SUBTYPE_BOOLEAN); + verify(mockLogWriter).action(eq(CATEGORY), eq(SUBTYPE_BOOLEAN), mPairCaptor.capture()); + + Pair value = mPairCaptor.getValue(); + assertThat(value.first instanceof Integer).isTrue(); + assertThat((int) value.first).isEqualTo(MetricsEvent.NOTIFICATION_SINCE_VISIBLE_MILLIS); + assertThat(value.second).isEqualTo(ELAPSED_TIME); + } + + @Test + public void action_IntegerLogsElapsedTime() { + mProvider.action(mockVisibilityLogger, CATEGORY, SUBTYPE_INTEGER); + verify(mockLogWriter).action(eq(CATEGORY), eq(SUBTYPE_INTEGER), mPairCaptor.capture()); + + Pair value = mPairCaptor.getValue(); + assertThat(value.first instanceof Integer).isTrue(); + assertThat((int) value.first).isEqualTo(MetricsEvent.NOTIFICATION_SINCE_VISIBLE_MILLIS); + assertThat(value.second).isEqualTo(ELAPSED_TIME); + } }