From 3ec09344860a9ecc73e258d6940c1870c1570225 Mon Sep 17 00:00:00 2001 From: Will Brockman Date: Fri, 19 Jun 2020 09:11:06 -0400 Subject: [PATCH] Statsd notif logs: important conversations. Adds an importance level for "important conversation" to the notification posted and notification channel updated logs. Bug: 155064930 Change-Id: Ib5a47aeb2da4d8614bfc778f3771479256220b23 Test: atest PreferencesHelperTest NotificationManagerServiceTest Test: statsd_testdrive --- .../stats/sysui/notification_enums.proto | 1 + .../NotificationChannelLogger.java | 34 ++++++++++++++++--- .../NotificationRecordLogger.java | 17 +++++++++- .../NotificationRecordLoggerImpl.java | 3 +- .../notification/PreferencesHelper.java | 6 ++-- 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/core/proto/android/stats/sysui/notification_enums.proto b/core/proto/android/stats/sysui/notification_enums.proto index 09837022e50d3..30bdecae07d14 100644 --- a/core/proto/android/stats/sysui/notification_enums.proto +++ b/core/proto/android/stats/sysui/notification_enums.proto @@ -26,4 +26,5 @@ enum NotificationImportance { // Constants from NotificationManager.java IMPORTANCE_LOW = 2; // Shows in shade, maybe status bar, no buzz/beep. IMPORTANCE_DEFAULT = 3; // Shows everywhere, makes noise, no heads-up. IMPORTANCE_HIGH = 4; // Shows everywhere, makes noise, heads-up, may full-screen. + IMPORTANCE_IMPORTANT_CONVERSATION = 5; // High + isImportantConversation(). } diff --git a/services/core/java/com/android/server/notification/NotificationChannelLogger.java b/services/core/java/com/android/server/notification/NotificationChannelLogger.java index a7b18778f8681..b70264b24319e 100644 --- a/services/core/java/com/android/server/notification/NotificationChannelLogger.java +++ b/services/core/java/com/android/server/notification/NotificationChannelLogger.java @@ -16,9 +16,12 @@ package com.android.server.notification; +import static android.app.NotificationManager.IMPORTANCE_HIGH; + import android.annotation.NonNull; import android.app.NotificationChannel; import android.app.NotificationChannelGroup; +import android.stats.sysui.NotificationEnums; import com.android.internal.logging.UiEvent; import com.android.internal.logging.UiEventLogger; @@ -42,7 +45,7 @@ public interface NotificationChannelLogger { String pkg) { logNotificationChannel( NotificationChannelEvent.getCreated(channel), - channel, uid, pkg, 0, 0); + channel, uid, pkg, 0, getLoggingImportance(channel)); } /** @@ -55,7 +58,7 @@ public interface NotificationChannelLogger { String pkg) { logNotificationChannel( NotificationChannelEvent.getDeleted(channel), - channel, uid, pkg, 0, 0); + channel, uid, pkg, getLoggingImportance(channel), 0); } /** @@ -63,13 +66,13 @@ public interface NotificationChannelLogger { * @param channel The channel. * @param uid UID of app that owns the channel. * @param pkg Package of app that owns the channel. - * @param oldImportance Previous importance level of the channel. + * @param oldLoggingImportance Previous logging importance level of the channel. * @param byUser True if the modification was user-specified. */ default void logNotificationChannelModified(@NonNull NotificationChannel channel, int uid, - String pkg, int oldImportance, boolean byUser) { + String pkg, int oldLoggingImportance, boolean byUser) { logNotificationChannel(NotificationChannelEvent.getUpdated(byUser), - channel, uid, pkg, oldImportance, channel.getImportance()); + channel, uid, pkg, oldLoggingImportance, getLoggingImportance(channel)); } /** @@ -194,6 +197,27 @@ public interface NotificationChannelLogger { return SmallHash.hash(group.getId()); } + /** + * @return Logging importance for a channel: the regular importance, or + * IMPORTANCE_IMPORTANT_CONVERSATION for a HIGH-importance conversation tagged important. + */ + static int getLoggingImportance(@NonNull NotificationChannel channel) { + return getLoggingImportance(channel, channel.getImportance()); + } + + /** + * @return Logging importance for a channel or notification: the regular importance, or + * IMPORTANCE_IMPORTANT_CONVERSATION for a HIGH-importance conversation tagged important. + */ + static int getLoggingImportance(@NonNull NotificationChannel channel, int importance) { + if (channel.getConversationId() == null || importance < IMPORTANCE_HIGH) { + return importance; + } + return (channel.isImportantConversation()) + ? NotificationEnums.IMPORTANCE_IMPORTANT_CONVERSATION + : importance; + } + /** * @return "Importance" for a channel group */ diff --git a/services/core/java/com/android/server/notification/NotificationRecordLogger.java b/services/core/java/com/android/server/notification/NotificationRecordLogger.java index e06e01e53852d..0e2ff7523c85e 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLogger.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLogger.java @@ -20,8 +20,10 @@ import static android.service.notification.NotificationListenerService.REASON_CA import static android.service.notification.NotificationListenerService.REASON_CLICK; import static android.service.notification.NotificationListenerService.REASON_TIMEOUT; +import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Notification; +import android.app.NotificationChannel; import android.app.Person; import android.os.Bundle; import android.service.notification.NotificationListenerService; @@ -346,7 +348,8 @@ public interface NotificationRecordLogger { == old.getSbn().getNotification().isGroupSummary()) && Objects.equals(r.getSbn().getNotification().category, old.getSbn().getNotification().category) - && (r.getImportance() == old.getImportance())); + && (r.getImportance() == old.getImportance()) + && (getLoggingImportance(r) == getLoggingImportance(old))); } /** @@ -413,5 +416,17 @@ public interface NotificationRecordLogger { } + /** + * @param r NotificationRecord + * @return Logging importance of record, taking important conversation channels into account. + */ + static int getLoggingImportance(@NonNull NotificationRecord r) { + final int importance = r.getImportance(); + final NotificationChannel channel = r.getChannel(); + if (channel == null) { + return importance; + } + return NotificationChannelLogger.getLoggingImportance(channel, importance); + } } diff --git a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java index 2b8ee92e00d90..1a99fb0e55f3f 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java @@ -51,7 +51,8 @@ public class NotificationRecordLoggerImpl implements NotificationRecordLogger { /* int32 style = 11 */ p.getStyle(), /* int32 num_people = 12 */ p.getNumPeople(), /* int32 position = 13 */ position, - /* android.stats.sysui.NotificationImportance importance = 14 */ r.getImportance(), + /* android.stats.sysui.NotificationImportance importance = 14 */ + NotificationRecordLogger.getLoggingImportance(r), /* int32 alerting = 15 */ buzzBeepBlink, /* NotificationImportanceExplanation importance_source = 16 */ r.getImportanceExplanationCode(), diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index e472e30977778..03e2fc76057aa 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -838,6 +838,8 @@ public class PreferencesHelper implements RankingConfig { // Apps are allowed to downgrade channel importance if the user has not changed any // fields on this channel yet. final int previousExistingImportance = existing.getImportance(); + final int previousLoggingImportance = + NotificationChannelLogger.getLoggingImportance(existing); if (existing.getUserLockedFields() == 0 && channel.getImportance() < existing.getImportance()) { existing.setImportance(channel.getImportance()); @@ -867,7 +869,7 @@ public class PreferencesHelper implements RankingConfig { updateConfig(); if (needsPolicyFileChange && !wasUndeleted) { mNotificationChannelLogger.logNotificationChannelModified(existing, uid, pkg, - previousExistingImportance, false); + previousLoggingImportance, false); } return needsPolicyFileChange; } @@ -985,7 +987,7 @@ public class PreferencesHelper implements RankingConfig { MetricsLogger.action(getChannelLog(updatedChannel, pkg) .setSubtype(fromUser ? 1 : 0)); mNotificationChannelLogger.logNotificationChannelModified(updatedChannel, uid, pkg, - channel.getImportance(), fromUser); + NotificationChannelLogger.getLoggingImportance(channel), fromUser); } if (updatedChannel.canBypassDnd() != mAreChannelsBypassingDnd