Merge "Make sure the ProtoLogTool includes all groups in the viewer config" into main

5 files changed, 81 insertions, 8 deletions

diff --git a/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt b/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
index 22858803b597..108942ee754a 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
@@ -368,7 +368,7 @@ object ProtoLogTool {
     }
 
     interface ProtologViewerConfigBuilder {
-        fun build(statements: Map<LogCall, Long>): ByteArray
+        fun build(groups: Collection<LogGroup>, statements: Map<LogCall, Long>): ByteArray
     }
 
     private fun viewerConf(command: CommandOptions) {
@@ -416,7 +416,7 @@ object ProtoLogTool {
         }
 
         val outFile = injector.fileOutputStream(command.viewerConfigFileNameArg)
-        outFile.write(configBuilder.build(logCallRegistry.getStatements()))
+        outFile.write(configBuilder.build(groups.values, logCallRegistry.getStatements()))
         outFile.close()
     }
 
diff --git a/tools/protologtool/src/com/android/protolog/tool/ViewerConfigJsonBuilder.kt b/tools/protologtool/src/com/android/protolog/tool/ViewerConfigJsonBuilder.kt
index 7714db212c9f..16a3d7cdda21 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ViewerConfigJsonBuilder.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ViewerConfigJsonBuilder.kt
@@ -21,8 +21,7 @@ import com.android.protolog.tool.Constants.VERSION
 import java.io.StringWriter
 
 class ViewerConfigJsonBuilder : ProtoLogTool.ProtologViewerConfigBuilder {
-    override fun build(statements: Map<ProtoLogTool.LogCall, Long>): ByteArray {
-        val groups = statements.map { it.key.logGroup }.toSet()
+    override fun build(groups: Collection<LogGroup>, statements: Map<ProtoLogTool.LogCall, Long>): ByteArray {
         val stringWriter = StringWriter()
         val writer = JsonWriter(stringWriter)
         writer.setIndent("  ")
diff --git a/tools/protologtool/src/com/android/protolog/tool/ViewerConfigProtoBuilder.kt b/tools/protologtool/src/com/android/protolog/tool/ViewerConfigProtoBuilder.kt
index 245e802df49b..de85411e4ffc 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ViewerConfigProtoBuilder.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ViewerConfigProtoBuilder.kt
@@ -28,10 +28,14 @@ class ViewerConfigProtoBuilder : ProtoLogTool.ProtologViewerConfigBuilder {
      * @return a byte array of a ProtoLogViewerConfig proto message encoding all the viewer
      * configurations mapping protolog hashes to message information and log group information.
      */
-    override fun build(statements: Map<ProtoLogTool.LogCall, Long>): ByteArray {
+    override fun build(groups: Collection<LogGroup>, statements: Map<ProtoLogTool.LogCall, Long>): ByteArray {
         val configBuilder = ProtoLogViewerConfig.newBuilder()
 
-        val groups = statements.map { it.key.logGroup }.toSet()
+        // TODO(b/373754057): We are passing all the groups now, because some groups might only be
+        //  used by Kotlin code that is not processed, but for group that get enabled to log to
+        //  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)
diff --git a/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigJsonBuilderTest.kt b/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigJsonBuilderTest.kt
index d27ae88fc488..1a20d4c5bad6 100644
--- a/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigJsonBuilderTest.kt
+++ b/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigJsonBuilderTest.kt
@@ -36,6 +36,8 @@ class ViewerConfigJsonBuilderTest {
         private val GROUP_DISABLED = LogGroup("DEBUG_GROUP", false, true, TAG2)
         private val GROUP_TEXT_DISABLED = LogGroup("DEBUG_GROUP", true, false, TAG2)
         private const val PATH = "/tmp/test.java"
+
+        private val GROUPS = listOf(GROUP1, GROUP2, GROUP3)
     }
 
     private val configBuilder = ViewerConfigJsonBuilder()
@@ -53,7 +55,7 @@ class ViewerConfigJsonBuilderTest {
                 LogCall(TEST3.messageString, LogLevel.ERROR, GROUP3, PATH)))
 
         val parsedConfig = parseConfig(
-            configBuilder.build(logCallRegistry.getStatements()).toString(Charsets.UTF_8))
+            configBuilder.build(GROUPS, logCallRegistry.getStatements()).toString(Charsets.UTF_8))
         assertEquals(3, parsedConfig.size)
         assertEquals(TEST1, parsedConfig[CodeUtils.hash(PATH,
 	           TEST1.messageString, LogLevel.INFO, GROUP1)])
@@ -72,7 +74,7 @@ class ViewerConfigJsonBuilderTest {
                 LogCall(TEST1.messageString, LogLevel.INFO, GROUP1, PATH)))
 
         val parsedConfig = parseConfig(
-            configBuilder.build(logCallRegistry.getStatements()).toString(Charsets.UTF_8))
+            configBuilder.build(GROUPS, logCallRegistry.getStatements()).toString(Charsets.UTF_8))
         assertEquals(1, parsedConfig.size)
         assertEquals(TEST1, parsedConfig[CodeUtils.hash(PATH, TEST1.messageString,
             LogLevel.INFO, GROUP1)])
diff --git a/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigProtoBuilderTest.kt b/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigProtoBuilderTest.kt
new file mode 100644
index 000000000000..74a8de7f70c0
--- /dev/null
+++ b/tools/protologtool/tests/com/android/protolog/tool/ViewerConfigProtoBuilderTest.kt
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.protolog.tool
+
+import com.android.internal.protolog.common.LogLevel
+import com.android.protolog.tool.ProtoLogTool.LogCall
+import com.google.common.truth.Truth
+import org.junit.Test
+import perfetto.protos.PerfettoTrace.ProtoLogViewerConfig
+
+class ViewerConfigProtoBuilderTest {
+    companion object {
+        private val TAG1 = "WM_TEST"
+        private val TAG2 = "WM_DEBUG"
+
+        private val TEST1 = ViewerConfigParser.ConfigEntry("test1", LogLevel.INFO.name,
+            TAG1
+        )
+        private val TEST2 = ViewerConfigParser.ConfigEntry("test2", LogLevel.DEBUG.name,
+            TAG2
+        )
+
+        private val GROUP1 = LogGroup("TEST_GROUP", true, true, TAG1)
+        private val GROUP2 = LogGroup("DEBUG_GROUP", true, true, TAG2)
+        private val GROUP3 = LogGroup("UNUSED_GROUP", true, true, TAG1)
+
+        private val GROUPS = listOf(
+            GROUP1,
+            GROUP2,
+            GROUP3
+        )
+
+        private const val PATH = "/tmp/test.java"
+    }
+
+    @Test
+    fun includesUnusedProtoLogGroups() {
+        // Added because of b/373754057. This test might need to be removed in the future.
+
+        val configBuilder = ViewerConfigProtoBuilder()
+
+        val logCallRegistry = ProtoLogTool.LogCallRegistry()
+        logCallRegistry.addLogCalls(listOf(
+            LogCall(TEST1.messageString, LogLevel.INFO, GROUP1, PATH),
+            LogCall(TEST2.messageString, LogLevel.INFO, GROUP2, PATH),
+        ))
+
+        val rawProto = configBuilder.build(GROUPS, logCallRegistry.getStatements())
+
+        val viewerConfig = ProtoLogViewerConfig.parseFrom(rawProto)
+        Truth.assertThat(viewerConfig.groupsCount).isEqualTo(GROUPS.size)
+        Truth.assertThat(viewerConfig.messagesCount).isLessThan(GROUPS.size)
+    }
+}
\ No newline at end of file