From b5a45b2072ba7c943af297d9800cb0bc43b28b94 Mon Sep 17 00:00:00 2001 From: Sergey Nikolaienkov Date: Tue, 11 Jan 2022 09:20:15 +0100 Subject: [PATCH] [7/X] Clean up logging in CDM Make sure all log tags in com.android.server.companion prefixed with "CompanionDevice" (allows for easy log filtering all CDM logs). Make sure all Log.x() calls in com.android.server.companion are behind DEBUG flags, while none of the Slog.x() calls are behind the DEBUG flag. Bug: 211398735 Test: atest CtsCompanionDeviceManagerCoreTestCases Test: atest CtsCompanionDeviceManagerUiAutomationTestCases Test: atest CtsOsTestCases:CompanionDeviceManagerTest Change-Id: Ied5cc355de463ab262d7ce73583283317c6d9d28 --- .../companion/AssociationCleanUpService.java | 3 +- .../companion/AssociationStoreImpl.java | 4 ++- .../CompanionDeviceShellCommand.java | 9 ++--- .../server/companion/DataStoreUtils.java | 11 +++--- .../server/companion/PersistentDataStore.java | 36 ++++++++++--------- .../android/server/companion/RolesUtils.java | 13 ++++--- 6 files changed, 41 insertions(+), 35 deletions(-) diff --git a/services/companion/java/com/android/server/companion/AssociationCleanUpService.java b/services/companion/java/com/android/server/companion/AssociationCleanUpService.java index f1d98f09aba3c..0509e0cf5ccc7 100644 --- a/services/companion/java/com/android/server/companion/AssociationCleanUpService.java +++ b/services/companion/java/com/android/server/companion/AssociationCleanUpService.java @@ -36,7 +36,6 @@ import com.android.server.LocalServices; * will be killed if association/role are revoked. */ public class AssociationCleanUpService extends JobService { - private static final String TAG = LOG_TAG + ".AssociationCleanUpService"; private static final int JOB_ID = AssociationCleanUpService.class.hashCode(); private static final long ONE_DAY_INTERVAL = 3 * 24 * 60 * 60 * 1000; // 1 Day private CompanionDeviceManagerServiceInternal mCdmServiceInternal = LocalServices.getService( @@ -56,7 +55,7 @@ public class AssociationCleanUpService extends JobService { @Override public boolean onStopJob(final JobParameters params) { - Slog.d(TAG, "Association cleanup job stopped; id=" + params.getJobId() + Slog.i(LOG_TAG, "Association cleanup job stopped; id=" + params.getJobId() + ", reason=" + JobParameters.getInternalReasonCodeDescription( params.getInternalStopReasonCode())); diff --git a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java index 3ccabaaea2fa2..21a677b8383c8 100644 --- a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java +++ b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java @@ -18,6 +18,7 @@ package com.android.server.companion; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.SuppressLint; import android.annotation.UserIdInt; import android.companion.AssociationInfo; import android.net.MacAddress; @@ -52,9 +53,10 @@ import java.util.StringJoiner; * Other system component (both inside and outside if the com.android.server.companion package) * should use public {@link AssociationStore} interface. */ +@SuppressLint("LongLogTag") class AssociationStoreImpl implements AssociationStore { private static final boolean DEBUG = false; - private static final String TAG = "AssociationStore"; + private static final String TAG = "CompanionDevice_AssociationStore"; private final Object mLock = new Object(); diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceShellCommand.java b/services/companion/java/com/android/server/companion/CompanionDeviceShellCommand.java index f2e660779e9ec..fd130852a43a4 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceShellCommand.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceShellCommand.java @@ -16,16 +16,17 @@ package com.android.server.companion; -import static com.android.server.companion.CompanionDeviceManagerService.LOG_TAG; - import android.companion.AssociationInfo; +import android.os.ShellCommand; import android.util.Log; import android.util.Slog; import java.io.PrintWriter; import java.util.List; -class CompanionDeviceShellCommand extends android.os.ShellCommand { +class CompanionDeviceShellCommand extends ShellCommand { + private static final String TAG = "CompanionDevice_ShellCommand"; + private final CompanionDeviceManagerService mService; private final AssociationStore mAssociationStore; @@ -84,7 +85,7 @@ class CompanionDeviceShellCommand extends android.os.ShellCommand { } return 0; } catch (Throwable t) { - Slog.e(LOG_TAG, "Error running a command: $ " + cmd, t); + Slog.e(TAG, "Error running a command: $ " + cmd, t); getErrPrintWriter().println(Log.getStackTraceString(t)); return 1; } diff --git a/services/companion/java/com/android/server/companion/DataStoreUtils.java b/services/companion/java/com/android/server/companion/DataStoreUtils.java index 6055a81c7ca70..8ac741a44ee57 100644 --- a/services/companion/java/com/android/server/companion/DataStoreUtils.java +++ b/services/companion/java/com/android/server/companion/DataStoreUtils.java @@ -25,7 +25,7 @@ import android.os.Environment; import android.util.AtomicFile; import android.util.Slog; -import com.android.internal.util.FunctionalUtils; +import com.android.internal.util.FunctionalUtils.ThrowingConsumer; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -34,8 +34,7 @@ import java.io.File; import java.io.FileOutputStream; final class DataStoreUtils { - - private static final String LOG_TAG = DataStoreUtils.class.getSimpleName(); + private static final String TAG = "CompanionDevice_DataStoreUtils"; static boolean isStartOfTag(@NonNull XmlPullParser parser, @NonNull String tag) throws XmlPullParserException { @@ -71,12 +70,12 @@ final class DataStoreUtils { * Writing to file could fail, for example, if the user has been recently removed and so was * their DE (/data/system_de//) directory. */ - static void writeToFileSafely(@NonNull AtomicFile file, - @NonNull FunctionalUtils.ThrowingConsumer consumer) { + static void writeToFileSafely( + @NonNull AtomicFile file, @NonNull ThrowingConsumer consumer) { try { file.write(consumer); } catch (Exception e) { - Slog.e(LOG_TAG, "Error while writing to file " + file, e); + Slog.e(TAG, "Error while writing to file " + file, e); } } diff --git a/services/companion/java/com/android/server/companion/PersistentDataStore.java b/services/companion/java/com/android/server/companion/PersistentDataStore.java index da33b44468404..d0cc12286b128 100644 --- a/services/companion/java/com/android/server/companion/PersistentDataStore.java +++ b/services/companion/java/com/android/server/companion/PersistentDataStore.java @@ -32,6 +32,7 @@ import static com.android.server.companion.DataStoreUtils.writeToFileSafely; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.SuppressLint; import android.annotation.UserIdInt; import android.companion.AssociationInfo; import android.content.pm.UserInfo; @@ -39,6 +40,7 @@ import android.net.MacAddress; import android.os.Environment; import android.util.ArrayMap; import android.util.AtomicFile; +import android.util.Log; import android.util.Slog; import android.util.SparseArray; import android.util.TypedXmlPullParser; @@ -146,8 +148,9 @@ import java.util.concurrent.ConcurrentMap; * * } */ +@SuppressLint("LongLogTag") final class PersistentDataStore { - private static final String LOG_TAG = CompanionDeviceManagerService.LOG_TAG + ".DataStore"; + private static final String TAG = "CompanionDevice_PersistentDataStore"; private static final boolean DEBUG = CompanionDeviceManagerService.DEBUG; private static final int CURRENT_PERSISTENCE_VERSION = 1; @@ -208,10 +211,9 @@ final class PersistentDataStore { void readStateForUser(@UserIdInt int userId, @NonNull Collection associationsOut, @NonNull Map> previouslyUsedIdsPerPackageOut) { - Slog.i(LOG_TAG, "Reading associations for user " + userId + " from disk"); - + Slog.i(TAG, "Reading associations for user " + userId + " from disk"); final AtomicFile file = getStorageFileForUser(userId); - if (DEBUG) Slog.d(LOG_TAG, " > File=" + file.getBaseFile().getPath()); + if (DEBUG) Log.d(TAG, " > File=" + file.getBaseFile().getPath()); // getStorageFileForUser() ALWAYS returns the SAME OBJECT, which allows us to synchronize // accesses to the file on the file system using this AtomicFile object. @@ -220,12 +222,12 @@ final class PersistentDataStore { final AtomicFile readFrom; final String rootTag; if (!file.getBaseFile().exists()) { - if (DEBUG) Slog.d(LOG_TAG, " > File does not exist -> Try to read legacy file"); + if (DEBUG) Log.d(TAG, " > File does not exist -> Try to read legacy file"); legacyBaseFile = getBaseLegacyStorageFileForUser(userId); - if (DEBUG) Slog.d(LOG_TAG, " > Legacy file=" + legacyBaseFile.getPath()); + if (DEBUG) Log.d(TAG, " > Legacy file=" + legacyBaseFile.getPath()); if (!legacyBaseFile.exists()) { - if (DEBUG) Slog.d(LOG_TAG, " > Legacy file does not exist -> Abort"); + if (DEBUG) Log.d(TAG, " > Legacy file does not exist -> Abort"); return; } @@ -236,13 +238,13 @@ final class PersistentDataStore { rootTag = XML_TAG_STATE; } - if (DEBUG) Slog.d(LOG_TAG, " > Reading associations..."); + if (DEBUG) Log.d(TAG, " > Reading associations..."); final int version = readStateFromFileLocked(userId, readFrom, rootTag, associationsOut, previouslyUsedIdsPerPackageOut); if (DEBUG) { - Slog.d(LOG_TAG, " > Done reading: " + associationsOut); + Log.d(TAG, " > Done reading: " + associationsOut); if (version < CURRENT_PERSISTENCE_VERSION) { - Slog.d(LOG_TAG, " > File used old format: v." + version + " -> Re-write"); + Log.d(TAG, " > File used old format: v." + version + " -> Re-write"); } } @@ -250,13 +252,13 @@ final class PersistentDataStore { // The data is either in the legacy file or in the legacy format, or both. // Save the data to right file in using the current format. if (DEBUG) { - Slog.d(LOG_TAG, " > Writing the data to " + file.getBaseFile().getPath()); + Log.d(TAG, " > Writing the data to " + file.getBaseFile().getPath()); } persistStateToFileLocked(file, associationsOut, previouslyUsedIdsPerPackageOut); if (legacyBaseFile != null) { // We saved the data to the right file, can delete the old file now. - if (DEBUG) Slog.d(LOG_TAG, " > Deleting legacy file"); + if (DEBUG) Log.d(TAG, " > Deleting legacy file"); legacyBaseFile.delete(); } } @@ -273,11 +275,11 @@ final class PersistentDataStore { void persistStateForUser(@UserIdInt int userId, @NonNull Collection associations, @NonNull Map> previouslyUsedIdsPerPackage) { - Slog.i(LOG_TAG, "Writing associations for user " + userId + " to disk"); - if (DEBUG) Slog.d(LOG_TAG, " > " + associations); + Slog.i(TAG, "Writing associations for user " + userId + " to disk"); + if (DEBUG) Slog.d(TAG, " > " + associations); final AtomicFile file = getStorageFileForUser(userId); - if (DEBUG) Slog.d(LOG_TAG, " > File=" + file.getBaseFile().getPath()); + if (DEBUG) Log.d(TAG, " > File=" + file.getBaseFile().getPath()); // getStorageFileForUser() ALWAYS returns the SAME OBJECT, which allows us to synchronize // accesses to the file on the file system using this AtomicFile object. synchronized (file) { @@ -312,7 +314,7 @@ final class PersistentDataStore { } return version; } catch (XmlPullParserException | IOException e) { - Slog.e(LOG_TAG, "Error while reading associations file", e); + Slog.e(TAG, "Error while reading associations file", e); return -1; } } @@ -528,7 +530,7 @@ final class PersistentDataStore { associationInfo = new AssociationInfo(associationId, userId, appPackage, macAddress, displayName, profile, selfManaged, notify, timeApproved, lastTimeConnected); } catch (Exception e) { - if (DEBUG) Slog.w(LOG_TAG, "Could not create AssociationInfo", e); + if (DEBUG) Log.w(TAG, "Could not create AssociationInfo", e); } return associationInfo; } diff --git a/services/companion/java/com/android/server/companion/RolesUtils.java b/services/companion/java/com/android/server/companion/RolesUtils.java index 76340fc1d6a8f..904283f4e60ea 100644 --- a/services/companion/java/com/android/server/companion/RolesUtils.java +++ b/services/companion/java/com/android/server/companion/RolesUtils.java @@ -19,20 +19,23 @@ package com.android.server.companion; import static android.app.role.RoleManager.MANAGE_HOLDERS_FLAG_DONT_KILL_APP; import static com.android.server.companion.CompanionDeviceManagerService.DEBUG; -import static com.android.server.companion.CompanionDeviceManagerService.LOG_TAG; import android.annotation.NonNull; +import android.annotation.SuppressLint; import android.annotation.UserIdInt; import android.app.role.RoleManager; import android.companion.AssociationInfo; import android.content.Context; import android.os.UserHandle; +import android.util.Log; import android.util.Slog; import java.util.List; /** Utility methods for accessing {@link RoleManager} APIs. */ +@SuppressLint("LongLogTag") final class RolesUtils { + private static final String TAG = CompanionDeviceManagerService.LOG_TAG; static boolean isRoleHolder(@NonNull Context context, @UserIdInt int userId, @NonNull String packageName, @NonNull String role) { @@ -45,7 +48,7 @@ final class RolesUtils { static void addRoleHolderForAssociation( @NonNull Context context, @NonNull AssociationInfo associationInfo) { if (DEBUG) { - Slog.d(LOG_TAG, "addRoleHolderForAssociation() associationInfo=" + associationInfo); + Log.d(TAG, "addRoleHolderForAssociation() associationInfo=" + associationInfo); } final String deviceProfile = associationInfo.getDeviceProfile(); @@ -61,7 +64,7 @@ final class RolesUtils { MANAGE_HOLDERS_FLAG_DONT_KILL_APP, userHandle, context.getMainExecutor(), success -> { if (!success) { - Slog.e(LOG_TAG, "Failed to add u" + userId + "\\" + packageName + Slog.e(TAG, "Failed to add u" + userId + "\\" + packageName + " to the list of " + deviceProfile + " holders."); } }); @@ -70,7 +73,7 @@ final class RolesUtils { static void removeRoleHolderForAssociation( @NonNull Context context, @NonNull AssociationInfo associationInfo) { if (DEBUG) { - Slog.d(LOG_TAG, "removeRoleHolderForAssociation() associationInfo=" + associationInfo); + Log.d(TAG, "removeRoleHolderForAssociation() associationInfo=" + associationInfo); } final String deviceProfile = associationInfo.getDeviceProfile(); @@ -86,7 +89,7 @@ final class RolesUtils { MANAGE_HOLDERS_FLAG_DONT_KILL_APP, userHandle, context.getMainExecutor(), success -> { if (!success) { - Slog.e(LOG_TAG, "Failed to remove u" + userId + "\\" + packageName + Slog.e(TAG, "Failed to remove u" + userId + "\\" + packageName + " from the list of " + deviceProfile + " holders."); } });