Post-process log records of dumpstate to assign process names

Some dumpstates have process information after the logcat sections, so additional pass is needed to make sure LogRecords are properly configured. Issue: #279
mlopatkin · Dec 26, 2023 · d5a1624 · d5a1624
1 parent 11c9cd5
commit d5a1624
Show file tree

Hide file tree

Showing 4 changed files with 6,255 additions and 12 deletions.
diff --git a/logmodel/src/main/java/name/mlopatkin/andlogview/logmodel/LogRecord.java b/logmodel/src/main/java/name/mlopatkin/andlogview/logmodel/LogRecord.java
@@ -17,7 +17,6 @@
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import com.google.common.collect.ComparisonChain;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -94,7 +93,7 @@ public String getCaption() {
     private final String tag;
     private final String message;
     private final @Nullable Buffer buffer;
-    private final String appName;
+    private final @Nullable String appName;
 
     LogRecord(
             SequenceNumber seqNo,
@@ -110,7 +109,7 @@ public String getCaption() {
         this.time = time;
         this.pid = pid;
         this.tid = tid;
-        this.appName = CharMatcher.whitespace().trimFrom(Strings.nullToEmpty(appName));
+        this.appName = appName != null ? CharMatcher.whitespace().trimFrom(appName) : null;
         this.priority = priority;
         this.tag = tag;
         this.message = message;
@@ -149,8 +148,12 @@ public String getMessage() {
         return buffer;
     }
 
+    public boolean hasAppName() {
+        return appName != null;
+    }
+
     public String getAppName() {
-        return appName;
+        return appName != null ? appName : "";
     }
 
     @Override

diff --git a/src/name/mlopatkin/andlogview/liblogcat/file/DumpstateFileDataSource.java b/src/name/mlopatkin/andlogview/liblogcat/file/DumpstateFileDataSource.java
@@ -213,27 +213,40 @@ public ImportResult readFrom(LineReader in) throws IOException, UnrecognizedForm
             if (availableBuffers.isEmpty()) {
                 throw new UnrecognizedFormatException("Cannot load dumpstate file, no valid logcat section found");
             }
-
-            OfflineSorter sorter = new OfflineSorter();
-            records.forEach(sorter::add);
-
-            if (sorter.hasTimeTravels()) {
-                problems.add(new ImportProblem(
-                        "Dumpstate file has time travels. Record ordering across buffers may not be consistent."));
-            }
             if (!hasPsSection && !hasProcessWaitSection) {
                 problems.add(
                         new ImportProblem("Failed to find Processes section. Application names are not available."));
             } else if ((psSectionHadUnparseableLines || processWaitSectionHadUnparseableLines)
                     && pidToProcessConverter.isEmpty()) {
                 problems.add(
                         new ImportProblem("Failed to parse Processes section. Application names are not available."));
+            } else {
+                updateProcesses();
+            }
+
+            OfflineSorter sorter = new OfflineSorter();
+            records.forEach(sorter::add);
+
+            if (sorter.hasTimeTravels()) {
+                problems.add(new ImportProblem(
+                        "Dumpstate file has time travels. Record ordering across buffers may not be consistent."));
             }
             return new ImportResult(
                     new DumpstateFileDataSource(
                             fileName, sorter.buildTimestampOrdered(), EnumSet.allOf(Field.class), availableBuffers,
                             pidToProcessConverter),
                     problems);
         }
+
+        private void updateProcesses() {
+            var iterator = records.listIterator();
+            while (iterator.hasNext()) {
+                var record = iterator.next();
+                final String appName;
+                if (!record.hasAppName() && (appName = pidToProcessConverter.get(record.getPid())) != null) {
+                    iterator.set(record.withAppName(appName));
+                }
+            }
+        }
     }
 }
diff --git a/test/name/mlopatkin/andlogview/liblogcat/file/FileDataSourceFactoryTest.java b/test/name/mlopatkin/andlogview/liblogcat/file/FileDataSourceFactoryTest.java
@@ -141,6 +141,22 @@ public void openDumpstateWithTimeTravel() throws Exception {
         assertThat(FileDataSourceFactory.createDataSource("time-travel.dump", log).getProblems()).isNotEmpty();
     }
 
+    @Test
+    public void openDumpstateWithProcessAfterLogs() throws Exception {
+        CharSource log = openTestData("emulator_api34.minimized.dump");
+
+        var source = FileDataSourceFactory.createDataSource("process-names.dump", log).getDataSource();
+
+        assertThat(source.getPidToProcessConverter()).isNotEmpty();
+
+        assertThat(getRecords(source)).as("have some log records with app names").anyMatch(LogRecord::hasAppName);
+
+        assertThat(getRecords(source))
+                .filteredOn(r -> !r.hasAppName())
+                .as("All records without app names are because the process is not known")
+                .allSatisfy(r -> assertThat(source.getPidToProcessConverter()).doesNotContainKey(r.getPid()));
+    }
+
     private CharSource openTestData(String testDataName) {
         return Resources.asCharSource(Resources.getResource(getClass(), testDataName), StandardCharsets.UTF_8);
     }