Skip to content

Commit fef36e5

Browse files
wfchandlerwfchandler
authored
Capture log file mtimes in support bundles (#9222)
Currently the mtime for log files captured in support bundles is always set as the default value of 1980-01-01T00:00. To more easily sort files by time, it would be convenient to capture their actual mtimes in the bundle zip. Attempt to get the mtime for logs captured by `sled-diagnostics`, falling back on the default if this fails, and copy the mtime into the final bundle zip in nexus.
1 parent 0eb83da commit fef36e5

File tree

6 files changed

+96
-56
lines changed

6 files changed

+96
-56
lines changed

‎Cargo.lock‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎nexus/Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ internal-dns-resolver.workspace = true
5353
internal-dns-types.workspace = true
5454
ipnetwork.workspace = true
5555
itertools.workspace = true
56+
jiff.workspace = true
5657
lldpd_client.workspace = true
5758
macaddr.workspace = true
5859
maplit.workspace = true
@@ -144,7 +145,7 @@ update-common.workspace = true
144145
update-engine.workspace = true
145146
omicron-workspace-hack.workspace = true
146147
omicron-uuid-kinds.workspace = true
147-
zip.workspace = true
148+
zip = { workspace = true, features = ["jiff-02"] }
148149

149150
[dev-dependencies]
150151
async-bb8-diesel.workspace = true

‎nexus/src/app/background/tasks/support_bundle_collector.rs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1318,10 +1318,23 @@ fn recursively_add_directory_to_zipfile(
13181318

13191319
let file_type = entry.file_type()?;
13201320
if file_type.is_file() {
1321+
let src = entry.path();
1322+
1323+
let zip_time = entry
1324+
.path()
1325+
.metadata()
1326+
.and_then(|m| m.modified())
1327+
.ok()
1328+
.and_then(|sys_time| jiff::Zoned::try_from(sys_time).ok())
1329+
.and_then(|zoned| {
1330+
zip::DateTime::try_from(zoned.datetime()).ok()
1331+
})
1332+
.unwrap_or_else(zip::DateTime::default);
1333+
13211334
let opts = FullFileOptions::default()
1335+
.last_modified_time(zip_time)
13221336
.compression_method(zip::CompressionMethod::Deflated)
13231337
.large_file(true);
1324-
let src = entry.path();
13251338

13261339
zip.start_file_from_path(dst, opts)?;
13271340
let mut file = std::fs::File::open(&src)?;

‎sled-diagnostics/Cargo.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ workspace = true
1010
anyhow.workspace = true
1111
camino.workspace = true
1212
cfg-if.workspace = true
13+
chrono.workspace = true
1314
fs-err = { workspace = true, features = ["tokio"] }
1415
futures.workspace = true
1516
illumos-utils.workspace = true
17+
jiff.workspace = true
1618
libc.workspace = true
1719
omicron-workspace-hack.workspace = true
1820
once_cell.workspace = true
@@ -26,7 +28,7 @@ slog.workspace = true
2628
thiserror.workspace = true
2729
tokio = { workspace = true, features = ["full"] }
2830
zfs-test-harness.workspace = true
29-
zip = { workspace = true, features = ["zstd"] }
31+
zip = { workspace = true, features = ["jiff-02","zstd"] }
3032

3133
[dev-dependencies]
3234
omicron-common.workspace = true

‎sled-diagnostics/src/logs.rs‎

Lines changed: 71 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,11 @@ impl LogsHandle {
451451
service: &str,
452452
zip: &mut zip::ZipWriter<W>,
453453
log_snapshots: &mut LogSnapshots,
454-
logfile: &Utf8Path,
454+
logfile: &LogFile,
455455
logtype: LogType,
456456
) -> Result<(), LogError> {
457457
let snapshot_logfile =
458-
self.find_log_in_snapshot(log_snapshots, logfile).await?;
458+
self.find_log_in_snapshot(log_snapshots, &logfile.path).await?;
459459

460460
if logtype == LogType::Current {
461461
// Since we are processing the current log files in a zone we need
@@ -501,13 +501,24 @@ impl LogsHandle {
501501
.filter(|f| is_log_file(f.path(), filename))
502502
{
503503
let logfile = f.path();
504+
let system_mtime =
505+
f.metadata().and_then(|m| m.modified()).inspect_err(|e| {
506+
warn!(&self.log, "sled-diagnostic failed to get mtime of logfile";
507+
"error" => %e,
508+
"logfile" => %logfile,
509+
);
510+
}).ok();
511+
let mtime = system_mtime
512+
.and_then(|m| jiff::Timestamp::try_from(m).ok());
513+
504514
if logfile.is_file() {
505515
write_log_to_zip(
506516
&self.log,
507517
service,
508518
zip,
509519
LogType::Current,
510520
logfile,
521+
mtime,
511522
)?;
512523
}
513524
}
@@ -522,6 +533,7 @@ impl LogsHandle {
522533
zip,
523534
logtype,
524535
&snapshot_logfile,
536+
logfile.modified,
525537
)?;
526538
}
527539
false => {
@@ -612,7 +624,7 @@ impl LogsHandle {
612624
&service,
613625
&mut zip,
614626
&mut log_snapshots,
615-
&current.path,
627+
&current,
616628
LogType::Current,
617629
)
618630
.await?;
@@ -628,13 +640,13 @@ impl LogsHandle {
628640
.archived
629641
.into_iter()
630642
.filter(|log| log.path.as_str().contains("crypt/debug"))
631-
.map(|log| log.path)
632643
.collect();
633644

634645
// Since these logs can be spread out across multiple U.2 devices
635646
// we need to sort them by timestamp.
636647
archived.sort_by_key(|log| {
637-
log.as_str()
648+
log.path
649+
.as_str()
638650
.rsplit_once(".")
639651
.and_then(|(_, date)| date.parse::<u64>().ok())
640652
.unwrap_or(0)
@@ -703,6 +715,7 @@ fn write_log_to_zip<W: Write + Seek>(
703715
zip: &mut zip::ZipWriter<W>,
704716
logtype: LogType,
705717
snapshot_logfile: &Utf8Path,
718+
mtime: Option<jiff::Timestamp>,
706719
) -> Result<(), LogError> {
707720
let Some(log_name) = snapshot_logfile.file_name() else {
708721
warn!(
@@ -715,10 +728,19 @@ fn write_log_to_zip<W: Write + Seek>(
715728
};
716729

717730
let mut src = File::open(&snapshot_logfile)?;
731+
732+
let zip_mtime = mtime
733+
.and_then(|ts| {
734+
let zoned = ts.in_tz("UTC").ok()?;
735+
zip::DateTime::try_from(zoned.datetime()).ok()
736+
})
737+
.unwrap_or_else(zip::DateTime::default);
738+
718739
let zip_path = format!("{service}/{logtype}/{log_name}");
719740
zip.start_file_from_path(
720741
zip_path,
721742
FullFileOptions::default()
743+
.last_modified_time(zip_mtime)
722744
.compression_method(zip::CompressionMethod::Zstd)
723745
.compression_level(Some(3))
724746
// NB: From the docs
@@ -757,8 +779,8 @@ enum ExtraLogKind<'a> {
757779

758780
#[derive(Debug, Default, PartialEq)]
759781
struct ExtraLogs<'a> {
760-
current: Option<&'a Utf8Path>,
761-
rotated: Vec<&'a Utf8Path>,
782+
current: Option<&'a LogFile>,
783+
rotated: Vec<&'a LogFile>,
762784
}
763785

764786
fn sort_extra_logs<'a>(
@@ -780,15 +802,15 @@ fn sort_extra_logs<'a>(
780802
warn!(
781803
logger,
782804
"found multiple current log files for {name}";
783-
"old" => %old_path,
805+
"old" => %old_path.path,
784806
"new" => %log.path,
785807
);
786808
}
787-
entry.current = Some(&log.path);
809+
entry.current = Some(&log);
788810
}
789811
ExtraLogKind::Rotated { name, log } => {
790812
let entry = res.entry(name).or_default();
791-
entry.rotated.push(&log.path);
813+
entry.rotated.push(&log);
792814
}
793815
}
794816
}
@@ -838,9 +860,9 @@ fn sort_cockroach_extra_logs(logs: &[LogFile]) -> HashMap<&str, ExtraLogs<'_>> {
838860
let entry = interested.entry(prefix).or_default();
839861

840862
if file_name == format!("{prefix}.log") {
841-
entry.current = Some(log.path.as_path());
863+
entry.current = Some(log);
842864
} else {
843-
entry.rotated.push(log.path.as_path());
865+
entry.rotated.push(log);
844866
}
845867
}
846868
}
@@ -917,52 +939,30 @@ mod test {
917939
"bogus.log",
918940
"some/dir"
919941
].into_iter().map(|l| {
920-
oxlog::LogFile { path: Utf8PathBuf::from(l), size: None, modified: None }
921-
}).collect();
942+
oxlog::LogFile { path: Utf8PathBuf::from(l), size: None, modified: None }
943+
}).collect();
944+
let logs_map: HashMap<_, _> =
945+
logs.iter().map(|l| (l.path.as_str(), l)).collect();
922946

923947
let mut expected: HashMap<&str, ExtraLogs<'_>> = HashMap::new();
924948

925949
// cockroach
926950
expected.entry("cockroach").or_default().current =
927-
Some(Utf8Path::new("cockroach.log"));
928-
expected
929-
.entry("cockroach")
930-
.or_default()
931-
.rotated
932-
.push(Utf8Path::new("cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T17_11_45Z.011435.log"));
933-
expected
934-
.entry("cockroach")
935-
.or_default()
936-
.rotated
937-
.push(Utf8Path::new("cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_51Z.011486.log"));
951+
Some(&logs_map["cockroach.log"]);
952+
expected.entry("cockroach").or_default().rotated.push(&logs_map["cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T17_11_45Z.011435.log"]);
953+
expected.entry("cockroach").or_default().rotated.push(&logs_map["cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_51Z.011486.log"]);
938954

939955
// cockroach-health
940956
expected.entry("cockroach-health").or_default().current =
941-
Some(Utf8Path::new("cockroach-health.log"));
942-
expected
943-
.entry("cockroach-health")
944-
.or_default()
945-
.rotated
946-
.push(Utf8Path::new("cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T21_43_26Z.011435.log"));
947-
expected
948-
.entry("cockroach-health")
949-
.or_default()
950-
.rotated
951-
.push(Utf8Path::new("cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_53Z.011486.log"));
957+
Some(&logs_map["cockroach-health.log"]);
958+
expected.entry("cockroach-health").or_default().rotated.push(&logs_map["cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T21_43_26Z.011435.log"]);
959+
expected.entry("cockroach-health").or_default().rotated.push(&logs_map["cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_53Z.011486.log"]);
952960

953961
// cockroach-stderr
954962
expected.entry("cockroach-stderr").or_default().current =
955-
Some(Utf8Path::new("cockroach-stderr.log"));
956-
expected
957-
.entry("cockroach-stderr")
958-
.or_default()
959-
.rotated
960-
.push(Utf8Path::new("cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-30T18_56_19Z.011950.log"));
961-
expected
962-
.entry("cockroach-stderr")
963-
.or_default()
964-
.rotated
965-
.push(Utf8Path::new("cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-31T02_59_24Z.010479.log"));
963+
Some(&logs_map["cockroach-stderr.log"]);
964+
expected.entry("cockroach-stderr").or_default().rotated.push(&logs_map["cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-30T18_56_19Z.011950.log"]);
965+
expected.entry("cockroach-stderr").or_default().rotated.push(&logs_map["cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-31T02_59_24Z.010479.log"]);
966966

967967
let extra = sort_cockroach_extra_logs(logs.as_slice());
968968
assert_eq!(
@@ -1177,6 +1177,12 @@ mod illumos_tests {
11771177
let mut logfile_handle =
11781178
fs_err::tokio::File::create_new(&logfile).await.unwrap();
11791179
logfile_handle.write_all(data.as_bytes()).await.unwrap();
1180+
1181+
// 2025-10-15T00:00:00
1182+
let mtime = std::time::SystemTime::UNIX_EPOCH
1183+
+ std::time::Duration::from_secs(1760486400);
1184+
let std_file = logfile_handle.into_std().await.into_file();
1185+
std_file.set_modified(mtime).unwrap();
11801186
}
11811187

11821188
// Populate some file with similar names that should be skipped over
@@ -1197,27 +1203,40 @@ mod illumos_tests {
11971203
let zipfile_path = mountpoint.join("test.zip");
11981204
let zipfile = File::create_new(&zipfile_path).unwrap();
11991205
let mut zip = ZipWriter::new(zipfile);
1206+
let log = LogFile {
1207+
path: mountpoint
1208+
.join(format!("var/svc/log/{}", logfile_to_data[0].0)),
1209+
size: None,
1210+
modified: None,
1211+
};
12001212

12011213
loghandle
12021214
.process_logs(
12031215
"mg-ddm",
12041216
&mut zip,
12051217
&mut log_snapshots,
1206-
&mountpoint
1207-
.join(format!("var/svc/log/{}", logfile_to_data[0].0)),
1218+
&log,
12081219
LogType::Current,
12091220
)
12101221
.await
12111222
.unwrap();
12121223

12131224
zip.finish().unwrap();
12141225

1215-
// Confirm the zip has our file and data
1226+
let expected_zip_mtime =
1227+
zip::DateTime::from_date_and_time(2025, 10, 15, 0, 0, 0)
1228+
.unwrap();
1229+
1230+
// Confirm the zip has our file and data with the right mtime
12161231
let mut archive =
12171232
ZipArchive::new(File::open(zipfile_path).unwrap()).unwrap();
12181233
for (name, data) in logfile_to_data {
12191234
let mut file_in_zip =
12201235
archive.by_name(&format!("mg-ddm/current/{name}")).unwrap();
1236+
1237+
let mtime = file_in_zip.last_modified().unwrap();
1238+
assert_eq!(mtime, expected_zip_mtime, "file mtime matches");
1239+
12211240
let mut contents = String::new();
12221241
file_in_zip.read_to_string(&mut contents).unwrap();
12231242

@@ -1283,13 +1302,14 @@ mod illumos_tests {
12831302
let zipfile_path = mountpoint.join("test.zip");
12841303
let zipfile = File::create_new(&zipfile_path).unwrap();
12851304
let mut zip = ZipWriter::new(zipfile);
1305+
let log = LogFile { path: logfile, size: None, modified: None };
12861306

12871307
loghandle
12881308
.process_logs(
12891309
"mg-ddm",
12901310
&mut zip,
12911311
&mut log_snapshots,
1292-
&logfile,
1312+
&log,
12931313
LogType::Current,
12941314
)
12951315
.await

‎workspace-hack/Cargo.toml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ x509-cert = { version = "0.2.5" }
154154
zerocopy-c38e5c1d305a1b54 = { package = "zerocopy", version = "0.8.27", default-features = false, features = ["derive", "simd"] }
155155
zerocopy-ca01ad9e24f5d932 = { package = "zerocopy", version = "0.7.35", features = ["derive", "simd"] }
156156
zeroize = { version = "1.8.1", features = ["std", "zeroize_derive"] }
157-
zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "zstd"] }
157+
zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "jiff-02", "zstd"] }
158158
zip-3b31131e45eafb45 = { package = "zip", version = "0.6.6", default-features = false, features = ["bzip2", "deflate"] }
159159

160160
[build-dependencies]
@@ -299,7 +299,7 @@ x509-cert = { version = "0.2.5" }
299299
zerocopy-c38e5c1d305a1b54 = { package = "zerocopy", version = "0.8.27", default-features = false, features = ["derive", "simd"] }
300300
zerocopy-ca01ad9e24f5d932 = { package = "zerocopy", version = "0.7.35", features = ["derive", "simd"] }
301301
zeroize = { version = "1.8.1", features = ["std", "zeroize_derive"] }
302-
zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "zstd"] }
302+
zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "jiff-02", "zstd"] }
303303
zip-3b31131e45eafb45 = { package = "zip", version = "0.6.6", default-features = false, features = ["bzip2", "deflate"] }
304304

305305
[target.x86_64-unknown-linux-gnu.dependencies]

0 commit comments

Comments
 (0)