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

Commit ae21ae5d authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Use distinct group ids between groups 

Make sure groups processed through ProtoLogTool in different runs have distinct groups ids

Otherwise we end up with ProtoLog groups collisions

Test: atest protologtool-tests
Flag: EXEMPT non-platform change
Bug: 424792568
Bug: 398937095
Change-Id: I0d365c1fb92a48fab28732f4300f734ea9a1e5a2
parent 049c61a0

tests/Tracing/Android.bp

+6 −5
Original line number Diff line number Diff line
@@ -17,14 +17,15 @@ android_test { 
    srcs: ["src/**/*.java"],

    libs: ["android.test.runner.stubs.system"],

    static_libs: [

        "junit",

        "androidx.test.rules",

        "mockito-target-minus-junit4",

        "truth",

        "platform-test-annotations",

        "flickerlib-parsers",

        "perfetto_trace_java_protos",

        "flickerlib-trace_processor_shell",

        "junit",

        "mockito-target-minus-junit4",

        "perfetto_trace_java_protos",

        "platform-test-annotations",

        "truth",

        "wm_shell_protolog-groups",

    ],

    java_resource_dirs: ["res"],

    certificate: "platform",

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

+29 −6
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertThrows; 
import android.platform.test.annotations.Presubmit;



import com.android.internal.protolog.common.IProtoLogGroup;

import com.android.wm.shell.protolog.ShellProtoLogGroup;



import com.google.common.truth.Truth;
@@ -30,6 +31,9 @@ import org.junit.Test; 
import org.junit.runner.RunWith;

import org.junit.runners.JUnit4;



import java.util.ArrayList;

import java.util.List;



/** Test class for {@link ProtoLog}. */

@SuppressWarnings("ConstantConditions")

@Presubmit
@@ -82,23 +86,42 @@ public class ProtoLogTest { 
        Truth.assertThat(assertion).hasMessageThat().contains("collision");

    }



    private static final IProtoLogGroup TEST_GROUP_1 = new ProtoLogGroup("TEST_TAG_1", 1);

    private static final IProtoLogGroup TEST_GROUP_2 = new ProtoLogGroup("TEST_TAG_2", 2);

    @Test

    public void noIdCollisionsBetweenGroups() {

        final var collectionOfKnowGroups = new ArrayList<IProtoLogGroup>();

        collectionOfKnowGroups.addAll(List.of(WmProtoLogGroups.values()));

        collectionOfKnowGroups.addAll(List.of(ShellProtoLogGroup.values()));

        collectionOfKnowGroups.add(new ProtoLogGroup("TEST_GROUP"));

        collectionOfKnowGroups.add(new ProtoLogGroup("OTHER_TEST_GROUP"));



        for (int i = 0; i < collectionOfKnowGroups.size(); i++) {

            for (int j = i + 1; j < collectionOfKnowGroups.size(); j++) {

                final var group1 = collectionOfKnowGroups.get(i);

                final var group2 = collectionOfKnowGroups.get(j);

                Truth.assertWithMessage(

                        "ID collision between " + group1.name() + " and " + group2.name())

                        .that(group1.getId()).isNotEqualTo(group2.getId());

            }

        }

    }



    private static final IProtoLogGroup TEST_GROUP_1 = new TestProtoLogGroup("TEST_TAG_1", 1);

    private static final IProtoLogGroup TEST_GROUP_2 = new TestProtoLogGroup("TEST_TAG_2", 2);

    private static final IProtoLogGroup TEST_GROUP_WITH_COLLISION =

            new ProtoLogGroup("TEST_TAG_WITH_COLLISION", 1);

            new TestProtoLogGroup("TEST_TAG_WITH_COLLISION", 1);



    private static class ProtoLogGroup implements IProtoLogGroup {

    private static class TestProtoLogGroup implements IProtoLogGroup {

        private final boolean mEnabled;

        private volatile boolean mLogToProto;

        private volatile boolean mLogToLogcat;

        private final String mTag;

        private final int mId;



        ProtoLogGroup(String tag, int id) {

        TestProtoLogGroup(String tag, int id) {

            this(true, true, false, tag, id);

        }



        ProtoLogGroup(

        TestProtoLogGroup(

                boolean enabled, boolean logToProto, boolean logToLogcat, String tag, int id) {

            this.mEnabled = enabled;

            this.mLogToProto = logToProto;

tools/protologtool/src/com/android/protolog/tool/LogGroup.kt

+2 −1
Original line number Diff line number Diff line
@@ -20,5 +20,6 @@ data class LogGroup( 
    val name: String,

    val enabled: Boolean,

    val textEnabled: Boolean,

    val tag: String

    val tag: String,

    val id: Int,

)

tools/protologtool/src/com/android/protolog/tool/ProtoLogGroupReader.kt

+7 −2
Original line number Diff line number Diff line
@@ -40,8 +40,13 @@ class ProtoLogGroupReader { 
            val clazz = classLoader.loadClass(className)

            val values = getEnumValues(clazz)

            return values.map { group ->

                group.name() to

                        LogGroup(group.name(), group.isEnabled, group.isLogToLogcat, group.tag)

                group.name() to LogGroup(

                    group.name(),

                    group.isEnabled,

                    group.isLogToLogcat,

                    group.tag,

                    group.id

                )

            }.toMap()

        } catch (ex: ReflectiveOperationException) {

            throw RuntimeException("Unable to load ProtoLogGroup enum class", ex)

tools/protologtool/src/com/android/protolog/tool/ViewerConfigProtoBuilder.kt

+3 −10
Original line number Diff line number Diff line
@@ -36,29 +36,22 @@ class ViewerConfigProtoBuilder : ProtoLogTool.ProtologViewerConfigBuilder { 
        //  logcat we try and load the viewer configurations for this group, so the group must exist

        //  in the viewer config. Once Kotlin is pre-processed or this logic changes we should only

        //  use the groups that are actually used as an optimization.

        val groupIds = mutableMapOf<LogGroup, Int>()

        groups.forEach {

            groupIds.putIfAbsent(it, groupIds.size + 1)

        }



        groupIds.forEach { (group, id) ->

        groups.forEach { group ->

            configBuilder.addGroups(ProtoLogViewerConfig.Group.newBuilder()

                    .setId(id)

                    .setId(group.id)

                    .setName(group.name)

                    .setTag(group.tag)

                    .build())

        }



        statements.forEach { (log, key) ->

            val groupId = groupIds[log.logGroup] ?: error("missing group id")



            configBuilder.addMessages(

                ProtoLogViewerConfig.MessageData.newBuilder()

                        .setMessageId(key)

                        .setMessage(log.messageString)

                        .setLevel(

                            ProtoLogLevel.forNumber(log.logLevel.protoMessageId))

                        .setGroupId(groupId)

                        .setGroupId(log.logGroup.id)

                        .setLocation("${log.position}:${log.lineNumber}")

            )

        }