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

Commit c9bbae6f authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Provide more feedback to user on protolog to logcat command" into main

parents 241cfec9 909bc019

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();