more testable MetricsLogger interface
Begin migration to non-static methods to allow easier mocking.
New clients should use the non-static methods.
Old clients that want to unit test metrics
should move to the new interface.
Bug: 35138327
Test: runtest --path frameworks/base/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
Change-Id: I168f3787ee50ddde8aa2d42c05b2b816d1d3a30e
diff --git a/core/java/com/android/internal/logging/MetricsLogger.java b/core/java/com/android/internal/logging/MetricsLogger.java
index 949e7ac..a482929 100644
--- a/core/java/com/android/internal/logging/MetricsLogger.java
+++ b/core/java/com/android/internal/logging/MetricsLogger.java
@@ -31,109 +31,188 @@
// define metric categories in frameworks/base/proto/src/metrics_constants.proto.
// mirror changes in native version at system/core/libmetricslogger/metrics_logger.cpp
+ private static MetricsLogger sMetricsLogger;
+
+ private static MetricsLogger getLogger() {
+ if (sMetricsLogger == null) {
+ sMetricsLogger = new MetricsLogger();
+ }
+ return sMetricsLogger;
+ }
+
+ protected void saveLog(Object[] rep) {
+ EventLogTags.writeSysuiMultiAction(rep);
+ }
+
public static final int VIEW_UNKNOWN = MetricsEvent.VIEW_UNKNOWN;
public static final int LOGTAG = EventLogTags.SYSUI_MULTI_ACTION;
- public static void visible(Context context, int category) throws IllegalArgumentException {
+ public void write(LogMaker content) {
+ if (content.getType() == MetricsEvent.TYPE_UNKNOWN) {
+ content.setType(MetricsEvent.TYPE_ACTION);
+ }
+ saveLog(content.serialize());
+ }
+
+ public void visible(int category) throws IllegalArgumentException {
if (Build.IS_DEBUGGABLE && category == VIEW_UNKNOWN) {
throw new IllegalArgumentException("Must define metric category");
}
EventLogTags.writeSysuiViewVisibility(category, 100);
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_OPEN)
.serialize());
}
- public static void hidden(Context context, int category) throws IllegalArgumentException {
+ public void hidden(int category) throws IllegalArgumentException {
if (Build.IS_DEBUGGABLE && category == VIEW_UNKNOWN) {
throw new IllegalArgumentException("Must define metric category");
}
EventLogTags.writeSysuiViewVisibility(category, 0);
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_CLOSE)
.serialize());
}
- public static void visibility(Context context, int category, boolean visibile)
+ public void visibility(int category, boolean visibile)
throws IllegalArgumentException {
if (visibile) {
- visible(context, category);
+ visible(category);
} else {
- hidden(context, category);
+ hidden(category);
}
}
- public static void visibility(Context context, int category, int vis)
+ public void visibility(int category, int vis)
throws IllegalArgumentException {
- visibility(context, category, vis == View.VISIBLE);
+ visibility(category, vis == View.VISIBLE);
}
- public static void action(Context context, int category) {
+ public void action(int category) {
EventLogTags.writeSysuiAction(category, "");
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_ACTION)
.serialize());
}
- public static void action(Context context, int category, int value) {
+ public void action(int category, int value) {
EventLogTags.writeSysuiAction(category, Integer.toString(value));
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_ACTION)
.setSubtype(value)
.serialize());
}
- public static void action(Context context, int category, boolean value) {
+ public void action(int category, boolean value) {
EventLogTags.writeSysuiAction(category, Boolean.toString(value));
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_ACTION)
.setSubtype(value ? 1 : 0)
.serialize());
}
- public static void action(LogMaker content) {
- if (content.getType() == MetricsEvent.TYPE_UNKNOWN) {
- content.setType(MetricsEvent.TYPE_ACTION);
- }
- EventLogTags.writeSysuiMultiAction(content.serialize());
- }
-
-
- public static void action(Context context, int category, String pkg) {
+ public void action(int category, String pkg) {
if (Build.IS_DEBUGGABLE && category == VIEW_UNKNOWN) {
throw new IllegalArgumentException("Must define metric category");
}
EventLogTags.writeSysuiAction(category, pkg);
- EventLogTags.writeSysuiMultiAction(new LogMaker(category)
+ saveLog(new LogMaker(category)
.setType(MetricsEvent.TYPE_ACTION)
.setPackageName(pkg)
.serialize());
}
/** Add an integer value to the monotonically increasing counter with the given name. */
- public static void count(Context context, String name, int value) {
+ public void count(String name, int value) {
EventLogTags.writeSysuiCount(name, value);
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(MetricsEvent.RESERVED_FOR_LOGBUILDER_COUNTER)
+ saveLog(new LogMaker(MetricsEvent.RESERVED_FOR_LOGBUILDER_COUNTER)
.setCounterName(name)
.setCounterValue(value)
.serialize());
}
/** Increment the bucket with the integer label on the histogram with the given name. */
- public static void histogram(Context context, String name, int bucket) {
+ public void histogram(String name, int bucket) {
// see LogHistogram in system/core/libmetricslogger/metrics_logger.cpp
EventLogTags.writeSysuiHistogram(name, bucket);
- EventLogTags.writeSysuiMultiAction(
- new LogMaker(MetricsEvent.RESERVED_FOR_LOGBUILDER_HISTOGRAM)
+ saveLog(new LogMaker(MetricsEvent.RESERVED_FOR_LOGBUILDER_HISTOGRAM)
.setCounterName(name)
.setCounterBucket(bucket)
.setCounterValue(1)
.serialize());
}
+
+ /** @deprecated use {@link #visible(int)} */
+ @Deprecated
+ public static void visible(Context context, int category) throws IllegalArgumentException {
+ getLogger().visible(category);
+ }
+
+ /** @deprecated use {@link #hidden(int)} */
+ @Deprecated
+ public static void hidden(Context context, int category) throws IllegalArgumentException {
+ getLogger().hidden(category);
+ }
+
+ /** @deprecated use {@link #visibility(int, boolean)} */
+ @Deprecated
+ public static void visibility(Context context, int category, boolean visibile)
+ throws IllegalArgumentException {
+ getLogger().visibility(category, visibile);
+ }
+
+ /** @deprecated use {@link #visibility(int, int)} */
+ @Deprecated
+ public static void visibility(Context context, int category, int vis)
+ throws IllegalArgumentException {
+ visibility(context, category, vis == View.VISIBLE);
+ }
+
+ /** @deprecated use {@link #action(int)} */
+ @Deprecated
+ public static void action(Context context, int category) {
+ getLogger().action(category);
+ }
+
+ /** @deprecated use {@link #action(int, int)} */
+ @Deprecated
+ public static void action(Context context, int category, int value) {
+ getLogger().action(category, value);
+ }
+
+ /** @deprecated use {@link #action(int, boolean)} */
+ @Deprecated
+ public static void action(Context context, int category, boolean value) {
+ getLogger().action(category, value);
+ }
+
+ /** @deprecated use {@link #write(LogMaker)} */
+ @Deprecated
+ public static void action(LogMaker content) {
+ getLogger().write(content);
+ }
+
+ /** @deprecated use {@link #action(int, String)} */
+ @Deprecated
+ public static void action(Context context, int category, String pkg) {
+ getLogger().action(category, pkg);
+ }
+
+ /**
+ * Add an integer value to the monotonically increasing counter with the given name.
+ * @deprecated use {@link #count(String, int)}
+ */
+ @Deprecated
+ public static void count(Context context, String name, int value) {
+ getLogger().count(name, value);
+ }
+
+ /**
+ * Increment the bucket with the integer label on the histogram with the given name.
+ * @deprecated use {@link #histogram(String, int)}
+ */
+ @Deprecated
+ public static void histogram(Context context, String name, int bucket) {
+ getLogger().histogram(name, bucket);
+ }
}
diff --git a/core/java/com/android/internal/logging/testing/FakeMetricsLogger.java b/core/java/com/android/internal/logging/testing/FakeMetricsLogger.java
new file mode 100644
index 0000000..fbaf87a
--- /dev/null
+++ b/core/java/com/android/internal/logging/testing/FakeMetricsLogger.java
@@ -0,0 +1,30 @@
+package com.android.internal.logging.testing;
+
+import android.metrics.LogMaker;
+
+import com.android.internal.logging.MetricsLogger;
+
+import java.util.LinkedList;
+import java.util.Queue;
+
+/**
+ * Fake logger that queues up logged events for inspection.
+ *
+ * @hide.
+ */
+public class FakeMetricsLogger extends MetricsLogger {
+ private Queue<LogMaker> logs = new LinkedList<>();
+
+ @Override
+ protected void saveLog(Object[] rep) {
+ logs.offer(new LogMaker(rep));
+ }
+
+ public Queue<LogMaker> getLogs() {
+ return logs;
+ }
+
+ public void reset() {
+ logs.clear();
+ }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java
index 005b701..6c729dc 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java
@@ -487,6 +487,8 @@
private ScreenPinningRequest mScreenPinningRequest;
+ MetricsLogger mMetricsLogger = new MetricsLogger();
+
// ensure quick settings is disabled until the current user makes it through the setup wizard
private boolean mUserSetup = false;
private DeviceProvisionedListener mUserSetupObserver = new DeviceProvisionedListener() {
@@ -1360,7 +1362,7 @@
mDismissView.setOnButtonClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
- MetricsLogger.action(mContext, MetricsEvent.ACTION_DISMISS_ALL_NOTES);
+ mMetricsLogger.action(MetricsEvent.ACTION_DISMISS_ALL_NOTES);
clearAllNotifications();
}
});
@@ -1539,7 +1541,7 @@
} else {
EventBus.getDefault().send(new UndockingTaskEvent());
if (metricsUndockAction != -1) {
- MetricsLogger.action(mContext, metricsUndockAction);
+ mMetricsLogger.action(metricsUndockAction);
}
}
}
@@ -1597,7 +1599,7 @@
notification.getKey());
notification.getNotification().fullScreenIntent.send();
shadeEntry.notifyFullScreenIntentLaunched();
- MetricsLogger.count(mContext, "note_fullscreen", 1);
+ mMetricsLogger.count("note_fullscreen", 1);
} catch (PendingIntent.CanceledException e) {
}
}
@@ -2801,16 +2803,16 @@
if (!mUserSetup) return;
if (KeyEvent.KEYCODE_SYSTEM_NAVIGATION_UP == key) {
- MetricsLogger.action(mContext, MetricsEvent.ACTION_SYSTEM_NAVIGATION_KEY_UP);
+ mMetricsLogger.action(MetricsEvent.ACTION_SYSTEM_NAVIGATION_KEY_UP);
mNotificationPanel.collapse(false /* delayed */, 1.0f /* speedUpFactor */);
} else if (KeyEvent.KEYCODE_SYSTEM_NAVIGATION_DOWN == key) {
- MetricsLogger.action(mContext, MetricsEvent.ACTION_SYSTEM_NAVIGATION_KEY_DOWN);
+ mMetricsLogger.action(MetricsEvent.ACTION_SYSTEM_NAVIGATION_KEY_DOWN);
if (mNotificationPanel.isFullyCollapsed()) {
mNotificationPanel.expand(true /* animate */);
- MetricsLogger.count(mContext, NotificationPanelView.COUNTER_PANEL_OPEN, 1);
+ mMetricsLogger.count(NotificationPanelView.COUNTER_PANEL_OPEN, 1);
} else if (!mNotificationPanel.isInSettings() && !mNotificationPanel.isExpanding()){
mNotificationPanel.flingSettings(0 /* velocity */, true /* expand */);
- MetricsLogger.count(mContext, NotificationPanelView.COUNTER_PANEL_OPEN_QS, 1);
+ mMetricsLogger.count(NotificationPanelView.COUNTER_PANEL_OPEN_QS, 1);
}
}
@@ -3689,7 +3691,7 @@
if (pinnedHeadsUp && isPanelFullyCollapsed()) {
notificationLoad = 1;
} else {
- MetricsLogger.histogram(mContext, "note_load", notificationLoad);
+ mMetricsLogger.histogram("note_load", notificationLoad);
}
mBarService.onPanelRevealed(clearNotificationEffects, notificationLoad);
} else {
@@ -3772,7 +3774,7 @@
if (mStatusBarStateLog == null) {
mStatusBarStateLog = new LogMaker(MetricsEvent.VIEW_UNKNOWN);
}
- MetricsLogger.action(mStatusBarStateLog
+ mMetricsLogger.write(mStatusBarStateLog
.setCategory(isBouncerShowing ? MetricsEvent.BOUNCER : MetricsEvent.LOCKSCREEN)
.setType(isShowing ? MetricsEvent.TYPE_OPEN : MetricsEvent.TYPE_CLOSE)
.setSubtype(isSecure ? 1 : 0));
@@ -5776,7 +5778,7 @@
NotificationInfo info = (NotificationInfo) item.gutsContent;
final NotificationInfo.OnSettingsClickListener onSettingsClick = (View v,
int appUid) -> {
- MetricsLogger.action(mContext, MetricsEvent.ACTION_NOTE_INFO);
+ mMetricsLogger.action(MetricsEvent.ACTION_NOTE_INFO);
guts.resetFalsingCheck();
startAppNotificationSettingsActivity(pkg, appUid, channel.getId());
};
@@ -5848,7 +5850,7 @@
return false;
}
- MetricsLogger.action(mContext, MetricsEvent.ACTION_NOTE_CONTROLS);
+ mMetricsLogger.action(MetricsEvent.ACTION_NOTE_CONTROLS);
// ensure that it's laid but not visible until actually laid out
guts.setVisibility(View.INVISIBLE);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
index 9f56da7..09f6b55 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
@@ -23,14 +23,15 @@
import static org.mockito.Mockito.when;
import android.metrics.LogMaker;
-import android.metrics.MetricsReader;
import android.support.test.filters.FlakyTest;
import android.support.test.filters.SmallTest;
import android.support.test.metricshelper.MetricsAsserts;
import android.support.test.runner.AndroidJUnit4;
import android.util.DisplayMetrics;
+import com.android.internal.logging.MetricsLogger;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
+import com.android.internal.logging.testing.FakeMetricsLogger;
import com.android.keyguard.KeyguardHostView.OnDismissAction;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.ActivatableNotificationView;
@@ -43,9 +44,6 @@
import org.junit.Test;
import org.junit.runner.RunWith;
-// TODO(gpitsch): We have seen some flakes in these tests, needs some investigation.
-// Q: How is mMetricsReader being used by the tested code?
-// A: StatusBar uses MetricsLogger to write to the event log, then read back by MetricsReader
@SmallTest
@RunWith(AndroidJUnit4.class)
public class StatusBarTest extends SysuiTestCase {
@@ -55,8 +53,8 @@
KeyguardIndicationController mKeyguardIndicationController;
NotificationStackScrollLayout mStackScroller;
StatusBar mStatusBar;
+ FakeMetricsLogger mMetricsLogger;
- private MetricsReader mMetricsReader;
private DisplayMetrics mDisplayMetrics = new DisplayMetrics();
@Before
@@ -65,8 +63,9 @@
mUnlockMethodCache = mock(UnlockMethodCache.class);
mKeyguardIndicationController = mock(KeyguardIndicationController.class);
mStackScroller = mock(NotificationStackScrollLayout.class);
+ mMetricsLogger = new FakeMetricsLogger();
mStatusBar = new TestableStatusBar(mStatusBarKeyguardViewManager, mUnlockMethodCache,
- mKeyguardIndicationController, mStackScroller);
+ mKeyguardIndicationController, mStackScroller, mMetricsLogger);
doAnswer(invocation -> {
OnDismissAction onDismissAction = (OnDismissAction) invocation.getArguments()[0];
@@ -81,15 +80,6 @@
}).when(mStatusBarKeyguardViewManager).addAfterKeyguardGoneRunnable(any());
when(mStackScroller.getActivatedChild()).thenReturn(null);
-
- mMetricsReader = new MetricsReader();
- mMetricsReader.checkpoint(); // clear out old logs
- try {
- // pause so that no new events arrive in the rest of this millisecond.
- Thread.sleep(2);
- } catch (InterruptedException e) {
- // pass
- }
}
@Test
@@ -127,10 +117,10 @@
when(mStatusBarKeyguardViewManager.isShowing()).thenReturn(false);
when(mStatusBarKeyguardViewManager.isBouncerShowing()).thenReturn(false);
when(mUnlockMethodCache.isMethodSecure()).thenReturn(false);
-
mStatusBar.onKeyguardViewManagerStatesUpdated();
- MetricsAsserts.assertHasLog("missing hidden insecure lockscreen log", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing hidden insecure lockscreen log",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.LOCKSCREEN)
.setType(MetricsEvent.TYPE_CLOSE)
.setSubtype(0));
@@ -150,7 +140,8 @@
mStatusBar.onKeyguardViewManagerStatesUpdated();
- MetricsAsserts.assertHasLog("missing hidden secure lockscreen log", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing hidden secure lockscreen log",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.LOCKSCREEN)
.setType(MetricsEvent.TYPE_CLOSE)
.setSubtype(1));
@@ -170,7 +161,8 @@
mStatusBar.onKeyguardViewManagerStatesUpdated();
- MetricsAsserts.assertHasLog("missing insecure lockscreen showing", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing insecure lockscreen showing",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.LOCKSCREEN)
.setType(MetricsEvent.TYPE_OPEN)
.setSubtype(0));
@@ -190,7 +182,8 @@
mStatusBar.onKeyguardViewManagerStatesUpdated();
- MetricsAsserts.assertHasLog("missing secure lockscreen showing log", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing secure lockscreen showing log",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.LOCKSCREEN)
.setType(MetricsEvent.TYPE_OPEN)
.setSubtype(1));
@@ -210,7 +203,8 @@
mStatusBar.onKeyguardViewManagerStatesUpdated();
- MetricsAsserts.assertHasLog("missing bouncer log", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing bouncer log",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.BOUNCER)
.setType(MetricsEvent.TYPE_OPEN)
.setSubtype(1));
@@ -223,7 +217,8 @@
ActivatableNotificationView view = mock(ActivatableNotificationView.class);
mStatusBar.onActivated(view);
- MetricsAsserts.assertHasLog("missing lockscreen note tap log", mMetricsReader,
+ MetricsAsserts.assertHasLog("missing lockscreen note tap log",
+ mMetricsLogger.getLogs(),
new LogMaker(MetricsEvent.ACTION_LS_NOTE)
.setType(MetricsEvent.TYPE_ACTION));
}
@@ -231,11 +226,12 @@
static class TestableStatusBar extends StatusBar {
public TestableStatusBar(StatusBarKeyguardViewManager man,
UnlockMethodCache unlock, KeyguardIndicationController key,
- NotificationStackScrollLayout stack) {
+ NotificationStackScrollLayout stack, MetricsLogger logger) {
mStatusBarKeyguardViewManager = man;
mUnlockMethodCache = unlock;
mKeyguardIndicationController = key;
mStackScroller = stack;
+ mMetricsLogger = logger;
}
@Override