Merge "Add since_visible_millis to Settings action logs."
diff --git a/src/com/android/settings/core/InstrumentedPreferenceFragment.java b/src/com/android/settings/core/InstrumentedPreferenceFragment.java
index bfb69e7..a5d0715 100644
--- a/src/com/android/settings/core/InstrumentedPreferenceFragment.java
+++ b/src/com/android/settings/core/InstrumentedPreferenceFragment.java
@@ -65,4 +65,8 @@
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 e7628e8..3196f76 100644
--- a/src/com/android/settings/core/instrumentation/EventLogWriter.java
+++ b/src/com/android/settings/core/instrumentation/EventLogWriter.java
@@ -18,6 +18,7 @@
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 @@
*/
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 @@
MetricsLogger.hidden(context, category);
}
+ public void action(int category, int value, Pair<Integer, Object>... 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<Integer, Object> pair : taggedData) {
+ logMaker.addTaggedData(pair.first, pair.second);
+ }
+ mMetricsLogger.write(logMaker);
+ }
+ }
+
+ public void action(int category, boolean value, Pair<Integer, Object>... taggedData) {
+ action(category, value ? 1 : 0, taggedData);
+ }
+
public void action(Context context, int category, Pair<Integer, Object>... taggedData) {
action(context, category, "", taggedData);
}
@@ -52,12 +73,16 @@
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 584217d..062d46f 100644
--- a/src/com/android/settings/core/instrumentation/LogWriter.java
+++ b/src/com/android/settings/core/instrumentation/LogWriter.java
@@ -34,6 +34,16 @@
void hidden(Context context, int category);
/**
+ * Logs a user action.
+ */
+ void action(int category, int value, Pair<Integer, Object>... taggedData);
+
+ /**
+ * Logs a user action.
+ */
+ void action(int category, boolean value, Pair<Integer, Object>... taggedData);
+
+ /**
* Logs an user action.
*/
void action(Context context, int category, Pair<Integer, Object>... taggedData);
@@ -45,12 +55,16 @@
/**
* 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 afdec55..532ec66 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.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 @@
}
}
+ /**
+ * 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<Integer, Object>... 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 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 @@
// 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<Integer, Object> 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 bbdf8c9..697f0a3 100644
--- a/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java
+++ b/src/com/android/settings/core/instrumentation/SettingSuggestionsLogWriter.java
@@ -46,6 +46,14 @@
}
@Override
+ public void action(int category, int value, Pair<Integer, Object>... taggedData) {
+ }
+
+ @Override
+ public void action(int category, boolean value, Pair<Integer, Object>... 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 8de35ad..2fe2a3b 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.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 @@
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 @@
@Override
public void onResume() {
+ mVisibleTimestamp = SystemClock.elapsedRealtime();
if (mMetricsFeature != null && mMetricsCategory != METRICS_CATEGORY_UNKNOWN) {
mMetricsFeature.visible(null /* context */, mSourceMetricsCategory, mMetricsCategory);
}
@@ -66,6 +69,7 @@
@Override
public void onPause() {
+ mVisibleTimestamp = 0;
if (mMetricsFeature != null && mMetricsCategory != METRICS_CATEGORY_UNKNOWN) {
mMetricsFeature.hidden(null /* context */, mMetricsCategory);
}
@@ -85,4 +89,12 @@
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 b47391d..b143f58 100644
--- a/src/com/android/settings/wifi/WifiSettings.java
+++ b/src/com/android/settings/wifi/WifiSettings.java
@@ -1070,7 +1070,7 @@
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 ea33c83..ff91c40 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 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.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 mLogWriter;
+ @Mock private LogWriter mockLogWriter;
+ @Mock private VisibilityLoggerMixin mockVisibilityLogger;
+
private Context mContext;
private MetricsFeatureProvider mProvider;
+ @Captor
+ private ArgumentCaptor<Pair> mPairCaptor;
+
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
mContext = RuntimeEnvironment.application;
mProvider = new MetricsFeatureProvider();
List<LogWriter> 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 @@
mProvider.logDashboardStartIntent(mContext, null /* intent */,
MetricsEvent.SETTINGS_GESTURES);
- verifyNoMoreInteractions(mLogWriter);
+ verifyNoMoreInteractions(mockLogWriter);
}
@Test
@@ -86,7 +99,7 @@
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 @@
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);
+ }
}