Merge plaintext and protobuf tombstones

src/mvt/android/artifacts/tombstone_crashes.py

@@ -131,6 +131,13 @@ class TombstoneCrashArtifact(AndroidArtifact):
         self, file_name: str, file_timestamp: datetime.datetime, data: bytes
     ) -> None:
         """Parse Android tombstone crash files from a protobuf object."""
+        self.results.append(self.parse_protobuf_record(file_name, file_timestamp, data))
+
+    def parse_protobuf_record(
+        self, file_name: str, file_timestamp: datetime.datetime, data: bytes
+    ) -> dict:
+        if not data:
+            raise ValueError("empty protobuf tombstone")
         tombstone_pb = Tombstone().parse(data)
         tombstone_dict = tombstone_pb.to_dict(
             casing=betterproto2.Casing.SNAKE, include_default_values=True
@@ -143,20 +150,31 @@ class TombstoneCrashArtifact(AndroidArtifact):
         tombstone_dict["file_name"] = file_name
         tombstone_dict["file_timestamp"] = convert_datetime_to_iso(file_timestamp)
         tombstone_dict["process_name"] = self._proccess_name_from_thread(tombstone_dict)
+        if isinstance(tombstone_dict.get("selinux_label"), str):
+            tombstone_dict["selinux_label"] = tombstone_dict["selinux_label"].rstrip(
+                "\x00"
+            )
 
         # Confirm the tombstone is valid, and matches the output model
         tombstone = TombstoneCrashResult.model_validate(tombstone_dict)
-        self.results.append(tombstone.model_dump())
+        return tombstone.model_dump()
 
     def parse(
         self, file_name: str, file_timestamp: datetime.datetime, content: bytes
     ) -> None:
         """Parse text Android tombstone crash files."""
+        self.results.append(self.parse_text_record(file_name, file_timestamp, content))
+
+    def parse_text_record(
+        self, file_name: str, file_timestamp: datetime.datetime, content: bytes
+    ) -> dict:
+        if not content:
+            raise ValueError("empty plaintext tombstone")
         tombstone_dict = {
             "file_name": file_name,
             "file_timestamp": convert_datetime_to_iso(file_timestamp),
         }
-        lines = content.decode("utf-8").splitlines()
+        lines = content.decode("utf-8", errors="replace").splitlines()
         for line_num, line in enumerate(lines, 1):
             if not line.strip() or TOMBSTONE_DELIMITER in line:
                 continue
@@ -171,7 +189,7 @@ class TombstoneCrashArtifact(AndroidArtifact):
 
         # Validate the tombstone and add it to the results
         tombstone = TombstoneCrashResult.model_validate(tombstone_dict)
-        self.results.append(tombstone.model_dump())
+        return tombstone.model_dump()
 
     def _parse_tombstone_line(
         self, line: str, key: str, destination_key: str, tombstone: dict
@@ -195,7 +213,9 @@ class TombstoneCrashArtifact(AndroidArtifact):
         if line_key != key:
             raise ValueError(f"Expected key {key}, got {line_key}")
 
-        value_clean = value.strip().strip("'")
+        value_clean = value.strip()
+        if len(value_clean) >= 2 and value_clean[0] == value_clean[-1] == "'":
+            value_clean = value_clean[1:-1]
         if destination_key == "uid":
             tombstone[destination_key] = int(value_clean)
         elif destination_key == "process_uptime":
@@ -269,9 +289,7 @@ class TombstoneCrashArtifact(AndroidArtifact):
     @staticmethod
     def _parse_timestamp_string(timestamp: str) -> str:
         timestamp_parsed = parser.parse(timestamp)
-        # Preserve the source wall-clock time while returning the project-wide ISO format.
-        local_timestamp = timestamp_parsed.replace(tzinfo=datetime.timezone.utc)
-        return convert_datetime_to_iso(local_timestamp)
+        return convert_datetime_to_iso(timestamp_parsed)
 
     @staticmethod
     def _proccess_name_from_thread(tombstone_dict: dict) -> str:

src/mvt/android/modules/bugreport/tombstones.py

@@ -43,21 +43,76 @@ class Tombstones(TombstoneCrashArtifact, BugReportModule):
             )
             return
 
-        for tombstone_file in sorted(tombstone_files):
-            tombstone_filename = tombstone_file.split("/")[-1]
-            modification_time = self._get_file_modification_time(tombstone_file)
-            tombstone_data = self._get_file_content(tombstone_file)
+        grouped: dict[str, dict[str, str]] = {}
+        for file_path in tombstone_files:
+            file_name = file_path.rsplit("/", 1)[-1]
+            source = "protobuf" if file_name.endswith(".pb") else "text"
+            crash_id = file_name.removesuffix(".pb")
+            grouped.setdefault(crash_id, {})[source] = file_path
 
-            try:
-                if tombstone_file.endswith(".pb"):
-                    self.parse_protobuf(
-                        tombstone_filename, modification_time, tombstone_data
+        for crash_id, paths in sorted(grouped.items()):
+            parsed_sources: dict[str, dict] = {}
+            source_records: dict[str, dict] = {}
+            for source in ("text", "protobuf"):
+                file_path = paths.get(source)
+                if file_path is None:
+                    continue
+                file_name = file_path.rsplit("/", 1)[-1]
+                file_timestamp = self._get_file_modification_time(file_path)
+                source_info = {
+                    "file_name": file_name,
+                    "file_timestamp": file_timestamp.isoformat(),
+                    "parsed": False,
+                    "error": None,
+                    "record": None,
+                }
+                try:
+                    data = self._get_file_content(file_path)
+                    if source == "protobuf":
+                        record = self.parse_protobuf_record(
+                            file_name, file_timestamp, data
+                        )
+                    else:
+                        record = self.parse_text_record(file_name, file_timestamp, data)
+                    source_info["parsed"] = True
+                    source_info["record"] = record
+                    source_records[source] = record
+                except Exception as exc:
+                    source_info["error"] = str(exc)
+                    self.log.error(
+                        "Error parsing tombstone file %s: %s", file_path, exc
                     )
-                else:
-                    self.parse(tombstone_filename, modification_time, tombstone_data)
-            except ValueError as e:
-                # Catch any exceptions raised during parsing or validation.
-                self.log.error(f"Error parsing tombstone file {tombstone_file}: {e}")
+                parsed_sources[source] = source_info
+
+            if not source_records:
+                continue
+            preferred = source_records.get("protobuf") or source_records["text"]
+            canonical = dict(preferred)
+            text_record = source_records.get("text")
+            if text_record:
+                for key, value in text_record.items():
+                    if canonical.get(key) in (None, "", [], {}):
+                        canonical[key] = value
+
+            differences = {}
+            protobuf_record = source_records.get("protobuf")
+            if text_record and protobuf_record:
+                for key in text_record.keys() & protobuf_record.keys():
+                    if key in ("file_name", "file_timestamp"):
+                        continue
+                    if text_record[key] != protobuf_record[key]:
+                        differences[key] = {
+                            "text": text_record[key],
+                            "protobuf": protobuf_record[key],
+                        }
+            canonical.update(
+                {
+                    "crash_id": crash_id,
+                    "sources": parsed_sources,
+                    "differences": differences,
+                }
+            )
+            self.results.append(canonical)
 
         self.log.info(
             "Extracted a total of %d tombstone files",

tests/android/test_artifact_tombstones.py

@@ -128,8 +128,5 @@ class TestTombstoneCrashArtifact:
         assert tombstone_result.get("pid") == 25541
         assert tombstone_result.get("process_name") == "mtk.ape.decoder"
 
-        # With Android logs we want to keep timestamps as device local time for consistency.
-        # We often don't know the time offset for a log entry and so can't convert everything to UTC.
-        # MVT should output the local time only:
-        # So original 2023-04-12 12:32:40.518290770+0200 -> 2023-04-12 12:32:40.000000
-        assert tombstone_result.get("timestamp") == "2023-04-12 12:32:40.518290"
+        # Tombstones include an explicit offset, so normalize them to UTC.
+        assert tombstone_result.get("timestamp") == "2023-04-12 10:32:40.518290"