Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b5a45b20 authored by Sergey Nikolaienkov's avatar Sergey Nikolaienkov
Browse files

[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
parent f225f4f1
Loading
Loading
Loading
Loading
+1 −2
Original line number Diff line number Diff line
@@ -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()));
+3 −1
Original line number Diff line number Diff line
@@ -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();

+5 −4
Original line number Diff line number Diff line
@@ -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;
        }
+5 −6
Original line number Diff line number Diff line
@@ -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/<user-id>/) directory.
     */
    static void writeToFileSafely(@NonNull AtomicFile file,
            @NonNull FunctionalUtils.ThrowingConsumer<FileOutputStream> consumer) {
    static void writeToFileSafely(
            @NonNull AtomicFile file, @NonNull ThrowingConsumer<FileOutputStream> 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);
        }
    }

+19 −17
Original line number Diff line number Diff line
@@ -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;
 * </state>
 * }</pre>
 */
@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<AssociationInfo> associationsOut,
            @NonNull Map<String, Set<Integer>> 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<AssociationInfo> associations,
            @NonNull Map<String, Set<Integer>> 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;
    }
Loading