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

Commit 909bc019 authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Provide more feedback to user on protolog to logcat command 

Add some feedback to the user when they use the logcat protolog command to help understand what is going on and when the command might be failing or executing unexpectedly.

Change-Id: I69f734f5da444eaf6939bae91a0ffb76b68fbb54
Test: atest TracingTests
Flag: EXEMPT logging only
Bug: 397637453
parent c9450097

core/java/com/android/internal/protolog/ProtoLogCommandHandler.java

+2 −2
Original line number Diff line number Diff line
@@ -145,11 +145,11 @@ public class ProtoLogCommandHandler extends ShellCommand { 


        switch (cmd) {

            case "enable" -> {

                mProtoLogConfigurationService.enableProtoLogToLogcat(processGroups());

                mProtoLogConfigurationService.enableProtoLogToLogcat(pw, processGroups());

                return 0;

            }

            case "disable" -> {

                mProtoLogConfigurationService.disableProtoLogToLogcat(processGroups());

                mProtoLogConfigurationService.disableProtoLogToLogcat(pw, processGroups());

                return 0;

            }

            default -> {

core/java/com/android/internal/protolog/ProtoLogConfigurationService.java

+4 −2
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ package com.android.internal.protolog; 


import android.annotation.NonNull;



import java.io.PrintWriter;



public interface ProtoLogConfigurationService extends IProtoLogConfigurationService {

    /**

     * Get the list of groups clients have registered to the protolog service.
@@ -37,11 +39,11 @@ public interface ProtoLogConfigurationService extends IProtoLogConfigurationServ 
     * Enable logging target groups to logcat.

     * @param groups we want to enable logging them to logcat for.

     */

    void enableProtoLogToLogcat(@NonNull String... groups);

    void enableProtoLogToLogcat(@NonNull PrintWriter pw, @NonNull String... groups);



    /**

     * Disable logging target groups to logcat.

     * @param groups we want to disable from being logged to logcat.

     */

    void disableProtoLogToLogcat(@NonNull String... groups);

    void disableProtoLogToLogcat(@NonNull PrintWriter pw, @NonNull String... groups);

}

core/java/com/android/internal/protolog/ProtoLogConfigurationServiceImpl.java

+19 −8
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ import java.io.FileDescriptor; 
import java.io.FileInputStream;

import java.io.FileNotFoundException;

import java.io.IOException;

import java.io.PrintWriter;

import java.util.HashMap;

import java.util.HashSet;

import java.util.Map;
@@ -183,8 +184,8 @@ public class ProtoLogConfigurationServiceImpl extends IProtoLogConfigurationServ 
     * @param groups we want to enable logging them to logcat for.

     */

    @Override

    public void enableProtoLogToLogcat(@NonNull String... groups) {

        toggleProtoLogToLogcat(true, groups);

    public void enableProtoLogToLogcat(@NonNull PrintWriter pw, @NonNull String... groups) {

        toggleProtoLogToLogcat(pw, true, groups);

    }



    /**
@@ -192,8 +193,8 @@ public class ProtoLogConfigurationServiceImpl extends IProtoLogConfigurationServ 
     * @param groups we want to disable from being logged to logcat.

     */

    @Override

    public void disableProtoLogToLogcat(@NonNull String... groups) {

        toggleProtoLogToLogcat(false, groups);

    public void disableProtoLogToLogcat(@NonNull PrintWriter pw, @NonNull String... groups) {

        toggleProtoLogToLogcat(pw, false, groups);

    }



    /**
@@ -249,7 +250,9 @@ public class ProtoLogConfigurationServiceImpl extends IProtoLogConfigurationServ 
        }

    }



    private void toggleProtoLogToLogcat(boolean enabled, @NonNull String[] groups) {

    private void toggleProtoLogToLogcat(

            @NonNull PrintWriter pw, boolean enabled, @NonNull String[] groups

    ) {

        final var clientToGroups = new HashMap<IProtoLogClient, Set<String>>();



        for (String group : groups) {
@@ -257,8 +260,10 @@ public class ProtoLogConfigurationServiceImpl extends IProtoLogConfigurationServ 


            if (clients == null) {

                // No clients associated to this group

                Log.w(LOG_TAG, "Attempting to toggle log to logcat for group " + group

                        + " with no registered clients.");

                var warning = "Attempting to toggle log to logcat for group " + group

                        + " with no registered clients. This is a no-op.";

                Log.w(LOG_TAG, warning);

                pw.println("WARNING: " + warning);

                continue;

            }
@@ -270,8 +275,14 @@ public class ProtoLogConfigurationServiceImpl extends IProtoLogConfigurationServ 


        for (IProtoLogClient client : clientToGroups.keySet()) {

            try {

                client.toggleLogcat(enabled, clientToGroups.get(client).toArray(new String[0]));

                final var clientGroups = clientToGroups.get(client).toArray(new String[0]);

                pw.println("Toggling logcat logging for client " + client.toString()

                        + " to " + enabled + " for groups: ["

                        + String.join(", ", clientGroups) + "]");

                client.toggleLogcat(enabled, clientGroups);

                pw.println("- Done");

            } catch (RemoteException e) {

                pw.println("- Failed");

                throw new RuntimeException(

                        "Failed to toggle logcat status for groups on client", e);

            }

tests/Tracing/src/com/android/internal/protolog/ProtoLogCommandHandlerTest.java

+9 −4
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.internal.protolog; 
import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.contains;

import static org.mockito.ArgumentMatchers.endsWith;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.atLeast;

import static org.mockito.Mockito.times;
@@ -157,13 +158,15 @@ public class ProtoLogCommandHandlerTest { 


        cmdHandler.exec(mMockBinder, FileDescriptor.in, FileDescriptor.out,

                FileDescriptor.err, new String[] { "logcat", "enable", "MY_GROUP" });

        Mockito.verify(mProtoLogConfigurationService).enableProtoLogToLogcat("MY_GROUP");

        Mockito.verify(mProtoLogConfigurationService)

                .enableProtoLogToLogcat(Mockito.any(), eq("MY_GROUP"));



        cmdHandler.exec(mMockBinder, FileDescriptor.in, FileDescriptor.out,

                FileDescriptor.err,

                new String[] { "logcat", "enable", "MY_GROUP", "MY_OTHER_GROUP" });

        Mockito.verify(mProtoLogConfigurationService)

                .enableProtoLogToLogcat("MY_GROUP", "MY_OTHER_GROUP");

                .enableProtoLogToLogcat(Mockito.any(),

                        eq("MY_GROUP"), eq("MY_OTHER_GROUP"));

    }



    @Test
@@ -173,13 +176,15 @@ public class ProtoLogCommandHandlerTest { 


        cmdHandler.exec(mMockBinder, FileDescriptor.in, FileDescriptor.out,

                FileDescriptor.err, new String[] { "logcat", "disable", "MY_GROUP" });

        Mockito.verify(mProtoLogConfigurationService).disableProtoLogToLogcat("MY_GROUP");

        Mockito.verify(mProtoLogConfigurationService)

                .disableProtoLogToLogcat(Mockito.any(), eq("MY_GROUP"));



        cmdHandler.exec(mMockBinder, FileDescriptor.in, FileDescriptor.out,

                FileDescriptor.err,

                new String[] { "logcat", "disable", "MY_GROUP", "MY_OTHER_GROUP" });

        Mockito.verify(mProtoLogConfigurationService)

                .disableProtoLogToLogcat("MY_GROUP", "MY_OTHER_GROUP");

                .disableProtoLogToLogcat(Mockito.any(),

                        eq("MY_GROUP"), eq("MY_OTHER_GROUP"));

    }



    @Test

tests/Tracing/src/com/android/internal/protolog/ProtoLogConfigurationServiceTest.java

+5 −4
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ import java.io.File; 
import java.io.FileNotFoundException;

import java.io.FileOutputStream;

import java.io.IOException;

import java.io.PrintWriter;

import java.util.List;



/**
@@ -234,7 +235,7 @@ public class ProtoLogConfigurationServiceTest { 
        service.registerClient(mMockClient, args);



        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isFalse();

        service.enableProtoLogToLogcat(TEST_GROUP);

        service.enableProtoLogToLogcat(Mockito.mock(PrintWriter.class), TEST_GROUP);

        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isTrue();



        Mockito.verify(mMockClient).toggleLogcat(eq(true),
@@ -251,7 +252,7 @@ public class ProtoLogConfigurationServiceTest { 
        service.registerClient(mMockClient, args);



        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isTrue();

        service.disableProtoLogToLogcat(TEST_GROUP);

        service.disableProtoLogToLogcat(Mockito.mock(PrintWriter.class), TEST_GROUP);

        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isFalse();



        Mockito.verify(mMockClient).toggleLogcat(eq(false),
@@ -269,7 +270,7 @@ public class ProtoLogConfigurationServiceTest { 
        service.registerClient(mMockClient, args);



        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isFalse();

        service.enableProtoLogToLogcat(OTHER_TEST_GROUP);

        service.enableProtoLogToLogcat(Mockito.mock(PrintWriter.class), OTHER_TEST_GROUP);

        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isFalse();



        Mockito.verify(mMockClient, never()).toggleLogcat(anyBoolean(), any());
@@ -280,7 +281,7 @@ public class ProtoLogConfigurationServiceTest { 
        final ProtoLogConfigurationService service = new ProtoLogConfigurationServiceImpl();



        Truth.assertThat(service.getGroups()).asList().doesNotContain(TEST_GROUP);

        service.enableProtoLogToLogcat(TEST_GROUP);

        service.enableProtoLogToLogcat(Mockito.mock(PrintWriter.class), TEST_GROUP);

        Truth.assertThat(service.isLoggingToLogcat(TEST_GROUP)).isTrue();



        final RegisterClientArgs args = new RegisterClientArgs();