[oxlog] sort log files before printing them out (#5356)

Found this while attempting to debug core files during today's mupdate -- I was
confused about logs getting skipped for a bit. We should sort log files so
they're printed in ascending order, disregarding the directories they're from.

Sort by filename only, because log files can go in. Sorting by file name
sorts by (service name, timestamp), which is the order we want 99% of the
time.

There is one wrinkle here, which is that for log files that are written
before NTP sync, the timestamps are of the form:

```
/pool/ext/f522118c-5dcd-4116-8044-07f0cceec52e/crypt/debug/oxz_switch/oxide-wicketd:default.log.536112897
```

But for log files written after NTP sync, they're of the form:

```
/pool/ext/f522118c-5dcd-4116-8044-07f0cceec52e/crypt/debug/oxz_switch/oxide-wicketd:default.log.1699421382
```

The latter gets sorted before the former, which isn't ideal. But also it
works fine and achieves the goal of keeping related logs together.

(Note that sorting by file name is possible with `sort` but a bit hard. Something like `sort -t/ -k7` would work: the `7` here indicates the depth at which log files are found.)

Author: sunshowers

Diff for: dev-tools/oxlog/src/lib.rs

@@ -98,6 +98,10 @@ impl LogFile {
             }
         }
     }
+
+    pub fn file_name_cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.path.file_name().cmp(&other.path.file_name())
+    }
 }
 
 impl PartialEq for LogFile {
@@ -142,6 +146,22 @@ pub struct SvcLogs {
     pub extra: Vec<LogFile>,
 }
 
+impl SvcLogs {
+    /// Sort the archived and extra log files by filename.
+    ///
+    /// readdir traverses over directories in indeterminate order, so sort by
+    /// filename (which is enough to sort by service name and timestamp in most
+    /// cases).
+    ///
+    /// Generally we don't want to sort by full path, because log files may be
+    /// scattered across several different directories -- and we care more
+    /// about filename than which directory they are in.
+    pub fn sort_by_file_name(&mut self) {
+        self.archived.sort_unstable_by(LogFile::file_name_cmp);
+        self.extra.sort_unstable_by(LogFile::file_name_cmp);
+    }
+}
+
 // These probably don't warrant newtypes. They are just to make the
 // keys in maps a bit easier to read.
 type ZoneName = String;
@@ -284,10 +304,19 @@ impl Zones {
                 load_extra_logs(dir, svc_name, &mut output, filter.show_empty);
             }
         }
+
+        sort_logs(&mut output);
+
         output
     }
 }
 
+fn sort_logs(output: &mut BTreeMap<String, SvcLogs>) {
+    for svc_logs in output.values_mut() {
+        svc_logs.sort_by_file_name();
+    }
+}
+
 const OX_SMF_PREFIXES: [&str; 2] = ["oxide-", "system-illumos-"];
 
 /// Return true if the provided file name appears to be a valid log file for an
@@ -464,4 +493,60 @@ mod tests {
         )
         .is_none());
     }
+
+    #[test]
+    fn test_sort_logs() {
+        use super::{LogFile, SvcLogs};
+        use std::collections::BTreeMap;
+
+        let mut logs = BTreeMap::new();
+        logs.insert(
+            "blah".to_string(),
+            SvcLogs {
+                current: None,
+                archived: vec![
+                    // "foo" comes after "bar", but the sorted order should
+                    // have 1600000000 before 1700000000.
+                    LogFile {
+                        path: "/bar/blah:default.log.1700000000".into(),
+                        size: None,
+                        modified: None,
+                    },
+                    LogFile {
+                        path: "/foo/blah:default.log.1600000000".into(),
+                        size: None,
+                        modified: None,
+                    },
+                ],
+                extra: vec![
+                    // "foo" comes after "bar", but the sorted order should
+                    // have log1 before log2.
+                    LogFile {
+                        path: "/foo/blah/sub.default.log1".into(),
+                        size: None,
+                        modified: None,
+                    },
+                    LogFile {
+                        path: "/bar/blah/sub.default.log2".into(),
+                        size: None,
+                        modified: None,
+                    },
+                ],
+            },
+        );
+
+        super::sort_logs(&mut logs);
+
+        let svc_logs = logs.get("blah").unwrap();
+        assert_eq!(
+            svc_logs.archived[0].path,
+            "/foo/blah:default.log.1600000000"
+        );
+        assert_eq!(
+            svc_logs.archived[1].path,
+            "/bar/blah:default.log.1700000000"
+        );
+        assert_eq!(svc_logs.extra[0].path, "/foo/blah/sub.default.log1");
+        assert_eq!(svc_logs.extra[1].path, "/bar/blah/sub.default.log2");
+    }
 }