From 999480444107beed86fcbad18c18e89d72c7404c Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sat, 27 Jun 2026 20:15:16 -0700 Subject: [PATCH 1/3] fix: honor --time-format rfc3339 for JSON output formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, --time-format rfc3339 was silently ignored when using --format json, json-line, or json-pretty — the timestamp field always remained a numeric Unix value. build_json_object() now accepts a TimestampFormat parameter and emits a formatted RFC 3339 string when Rfc3339 is selected, while Unix (default) preserves the numeric f64 output byte-for-byte via the existing native serialization fallback. Fixes #123 --- CHANGELOG.md | 8 ++ README.md | 11 +-- src/bin/commands/elem_format.rs | 130 ++++++++++++++++++++++++++++++-- src/bin/commands/parse.rs | 2 +- src/bin/commands/search.rs | 2 +- src/utils.rs | 6 +- 6 files changed, 141 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c01302..8c9b390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. +## Unreleased changes + +### Bug Fixes + +* Fixed `--time-format rfc3339` being ignored for `json`, `json-line`, and `json-pretty` + output formats. JSON output now honors `--time-format`: `unix` (default) emits a numeric + `timestamp` field (backward compatible); `rfc3339` emits an RFC 3339 string (#123). + ## v1.3.0 - 2026-05-27 ### Breaking Changes diff --git a/README.md b/README.md index d1e41b2..4c8a73d 100644 --- a/README.md +++ b/README.md @@ -340,7 +340,7 @@ Options: [default: asc] --time-format - Timestamp output format for non-JSON output (unix or rfc3339) + Timestamp output format (unix or rfc3339), applied to all output formats including JSON Possible values: - unix: Unix timestamp (integer or float) - default for backward compatibility @@ -452,13 +452,14 @@ monocle parse file.mrt --order-by prefix --order desc #### Timestamp Format -Use `--time-format` to change timestamp output format: +Use `--time-format` to change timestamp output format (applies to all output formats +including `json`, `json-line`, and `json-pretty`): ```bash -# Unix timestamp (default) +# Unix timestamp (default) — numeric `timestamp` field in JSON monocle parse file.mrt --time-format unix -# RFC3339/ISO 8601 format +# RFC3339/ISO 8601 format — string `timestamp` field in JSON monocle parse file.mrt --time-format rfc3339 ``` @@ -588,7 +589,7 @@ Options: [default: asc] --time-format - Timestamp output format for non-JSON output (unix or rfc3339) + Timestamp output format (unix or rfc3339), applied to all output formats including JSON Possible values: - unix: Unix timestamp (integer or float) - default for backward compatibility diff --git a/src/bin/commands/elem_format.rs b/src/bin/commands/elem_format.rs index 452029d..862735f 100644 --- a/src/bin/commands/elem_format.rs +++ b/src/bin/commands/elem_format.rs @@ -240,7 +240,8 @@ pub fn get_field_value_with_time_format( /// Format a BgpElem according to the output format and selected fields. /// The `collector` parameter provides the collector value for the "collector" field. -/// The `time_format` parameter controls how timestamps are displayed in non-JSON formats. +/// The `time_format` parameter controls how timestamps are displayed, including JSON +/// output: `Unix` (default) emits a numeric timestamp; `Rfc3339` emits an RFC 3339 string. /// Note: For Table format, this returns an empty string - use format_elems_table() instead /// after collecting all elements. pub fn format_elem( @@ -252,13 +253,11 @@ pub fn format_elem( ) -> Option { match output_format { OutputFormat::Json | OutputFormat::JsonLine => { - // JSON always uses Unix timestamp as number for backward compatibility - let obj = build_json_object(elem, fields, collector); + let obj = build_json_object(elem, fields, collector, time_format); Some(serde_json::to_string(&obj).unwrap_or_else(|_| elem.to_string())) } OutputFormat::JsonPretty => { - // JSON always uses Unix timestamp as number for backward compatibility - let obj = build_json_object(elem, fields, collector); + let obj = build_json_object(elem, fields, collector, time_format); Some(serde_json::to_string_pretty(&obj).unwrap_or_else(|_| elem.to_string())) } OutputFormat::Psv => { @@ -287,13 +286,21 @@ pub fn format_elem( /// Build a JSON object with only the selected fields /// The `collector` parameter provides the collector value for the "collector" field. +/// The `time_format` parameter controls the `timestamp` field representation: +/// `Unix` (default) emits a numeric f64; `Rfc3339` emits a formatted string. pub fn build_json_object( elem: &BgpElem, fields: &[&str], collector: Option<&str>, + time_format: TimestampFormat, ) -> serde_json::Value { - // If all default parse fields are selected and no collector field, use the original serialization - if fields == DEFAULT_FIELDS_PARSE && !fields.contains(&"collector") { + // If all default parse fields are selected, no collector field, and the default + // Unix timestamp format is in use, fall back to the original element serialization + // to preserve backward-compatible output byte-for-byte. + if fields == DEFAULT_FIELDS_PARSE + && !fields.contains(&"collector") + && time_format == TimestampFormat::Unix + { return json!(elem); } @@ -301,7 +308,13 @@ pub fn build_json_object( for field in fields { let value = match *field { "type" => json!(elem.elem_type), - "timestamp" => json!(elem.timestamp), + "timestamp" => { + if time_format == TimestampFormat::Unix { + json!(elem.timestamp) + } else { + json!(time_format.format_timestamp(elem.timestamp)) + } + } "peer_ip" => json!(elem.peer_ip.to_string()), "peer_asn" => json!(elem.peer_asn), "prefix" => json!(elem.prefix.to_string()), @@ -431,3 +444,104 @@ pub fn sort_elems( } }); } + +#[cfg(test)] +mod tests { + use super::*; + use bgpkit_parser::models::{AsPath, AsPathSegment, ElemType, NetworkPrefix, Origin}; + use bgpkit_parser::BgpElem; + use std::net::{IpAddr, Ipv4Addr}; + use std::str::FromStr; + + fn test_elem() -> BgpElem { + BgpElem { + timestamp: 1234567890.0, + elem_type: ElemType::ANNOUNCE, + peer_ip: IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1)), + peer_asn: 65000.into(), + prefix: NetworkPrefix::from_str("10.0.0.0/8").unwrap(), + next_hop: Some(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1))), + as_path: Some(AsPath { + segments: vec![AsPathSegment::AsSequence(vec![65000.into(), 65001.into()])], + }), + origin_asns: Some(vec![65001.into()]), + origin: Some(Origin::IGP), + local_pref: Some(100), + med: Some(0), + communities: None, + atomic: false, + aggr_asn: None, + aggr_ip: None, + only_to_customer: None, + unknown: None, + deprecated: None, + } + } + + #[test] + fn test_json_timestamp_unix_is_numeric() { + let elem = test_elem(); + let fields = vec!["timestamp", "prefix"]; + let obj = build_json_object(&elem, &fields, None, TimestampFormat::Unix); + let ts = obj.get("timestamp").unwrap(); + assert_eq!(ts.as_f64().unwrap(), 1234567890.0); + } + + #[test] + fn test_json_timestamp_rfc3339_is_string() { + let elem = test_elem(); + let fields = vec!["timestamp", "prefix"]; + let obj = build_json_object(&elem, &fields, None, TimestampFormat::Rfc3339); + let ts = obj.get("timestamp").unwrap(); + assert!(ts.is_string(), "expected string for rfc3339 timestamp"); + let s = ts.as_str().unwrap(); + assert!(s.contains("2009")); + assert!(s.contains('T')); + } + + #[test] + fn test_json_line_format_unix_vs_rfc3339() { + let elem = test_elem(); + let fields = vec!["timestamp", "prefix"]; + + let unix_out = format_elem( + &elem, + OutputFormat::JsonLine, + &fields, + None, + TimestampFormat::Unix, + ) + .unwrap(); + let rfc_out = format_elem( + &elem, + OutputFormat::JsonLine, + &fields, + None, + TimestampFormat::Rfc3339, + ) + .unwrap(); + + let unix_json: serde_json::Value = serde_json::from_str(&unix_out).unwrap(); + let rfc_json: serde_json::Value = serde_json::from_str(&rfc_out).unwrap(); + + assert!(unix_json.get("timestamp").unwrap().is_number()); + assert!(rfc_json.get("timestamp").unwrap().is_string()); + } + + #[test] + fn test_table_timestamp_rfc3339() { + let elem = test_elem(); + let value = + get_field_value_with_time_format(&elem, "timestamp", None, TimestampFormat::Rfc3339); + assert!(value.contains('T')); + } + + #[test] + fn test_default_fields_unix_falls_back_to_native_serialization() { + let elem = test_elem(); + let obj = build_json_object(&elem, DEFAULT_FIELDS_PARSE, None, TimestampFormat::Unix); + // With default fields + Unix, build_json_object should use native elem serialization + // which contains the full element (e.g. a `peer_ip` field). + assert!(obj.get("peer_ip").is_some()); + } +} diff --git a/src/bin/commands/parse.rs b/src/bin/commands/parse.rs index c9e4c72..2ad20f1 100644 --- a/src/bin/commands/parse.rs +++ b/src/bin/commands/parse.rs @@ -39,7 +39,7 @@ pub(crate) struct ParseArgs { #[clap(long, value_enum, default_value = "asc")] pub order: OrderDirection, - /// Timestamp output format for non-JSON output (unix or rfc3339) + /// Timestamp output format (unix or rfc3339), applied to all output formats including JSON #[clap(long, value_enum, default_value = "unix")] pub time_format: TimestampFormat, diff --git a/src/bin/commands/search.rs b/src/bin/commands/search.rs index b598fc4..e293ad0 100644 --- a/src/bin/commands/search.rs +++ b/src/bin/commands/search.rs @@ -55,7 +55,7 @@ pub struct SearchArgs { #[clap(long, value_enum, default_value = "asc")] pub order: OrderDirection, - /// Timestamp output format for non-JSON output (unix or rfc3339) + /// Timestamp output format (unix or rfc3339), applied to all output formats including JSON #[clap(long, value_enum, default_value = "unix")] pub time_format: TimestampFormat, diff --git a/src/utils.rs b/src/utils.rs index fa3209f..979e13c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -687,9 +687,9 @@ impl FromStr for OrderDirection { /// Format for timestamp output in parse and search commands /// -/// This enum controls how timestamps are displayed in non-JSON output formats -/// (table, psv, markdown). JSON output always uses Unix timestamps as numbers -/// for backward compatibility. +/// This enum controls how timestamps are displayed across output formats +/// (table, psv, markdown, and JSON). `Unix` (default) emits a numeric timestamp; +/// `Rfc3339` emits an RFC 3339 / ISO 8601 string. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] From 992979b3eb43d67c8a8535d680926203b2f22a45 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sun, 28 Jun 2026 10:22:30 -0700 Subject: [PATCH 2/3] fix: preserve native JSON shape for rfc3339 + default fields build_json_object now always uses native BgpElem serialization for the default-fields case, overriding only the timestamp key when rfc3339 is selected. Previously, rfc3339 + default fields took the field-by-field path, dropping fields like origin_asns/only_to_customer that exist in native serialization but not in DEFAULT_FIELDS_PARSE. Also fix test to assert origin_asns (not in DEFAULT_FIELDS_PARSE) instead of peer_ip (which is), and add test for rfc3339 shape preservation. Addresses PR review feedback. --- src/bin/commands/elem_format.rs | 51 ++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/src/bin/commands/elem_format.rs b/src/bin/commands/elem_format.rs index 862735f..5404de9 100644 --- a/src/bin/commands/elem_format.rs +++ b/src/bin/commands/elem_format.rs @@ -294,14 +294,22 @@ pub fn build_json_object( collector: Option<&str>, time_format: TimestampFormat, ) -> serde_json::Value { - // If all default parse fields are selected, no collector field, and the default - // Unix timestamp format is in use, fall back to the original element serialization - // to preserve backward-compatible output byte-for-byte. - if fields == DEFAULT_FIELDS_PARSE - && !fields.contains(&"collector") - && time_format == TimestampFormat::Unix - { - return json!(elem); + // If all default parse fields are selected and no collector field, use the + // original element serialization to preserve the full JSON shape (which + // includes fields like `origin_asns`, `only_to_customer`, etc. that are not + // listed in DEFAULT_FIELDS_PARSE). When RFC3339 timestamps are requested, + // override just the `timestamp` key on top of the native serialization. + if fields == DEFAULT_FIELDS_PARSE && !fields.contains(&"collector") { + let mut obj = json!(elem); + if time_format != TimestampFormat::Unix { + if let Some(map) = obj.as_object_mut() { + map.insert( + "timestamp".to_string(), + json!(time_format.format_timestamp(elem.timestamp)), + ); + } + } + return obj; } let mut obj = serde_json::Map::new(); @@ -540,8 +548,29 @@ mod tests { fn test_default_fields_unix_falls_back_to_native_serialization() { let elem = test_elem(); let obj = build_json_object(&elem, DEFAULT_FIELDS_PARSE, None, TimestampFormat::Unix); - // With default fields + Unix, build_json_object should use native elem serialization - // which contains the full element (e.g. a `peer_ip` field). - assert!(obj.get("peer_ip").is_some()); + // With default fields, build_json_object should use native elem serialization + // which contains fields NOT in DEFAULT_FIELDS_PARSE (e.g. `origin_asns`). + // If the fallback broke, origin_asns would be absent from the output. + assert!( + obj.get("origin_asns").is_some(), + "native serialization should include origin_asns (not in DEFAULT_FIELDS_PARSE)" + ); + // Timestamp stays numeric for Unix + assert!(obj.get("timestamp").unwrap().is_number()); + } + + #[test] + fn test_default_fields_rfc3339_preserves_shape_overrides_timestamp() { + let elem = test_elem(); + let obj = build_json_object(&elem, DEFAULT_FIELDS_PARSE, None, TimestampFormat::Rfc3339); + // The full native shape should be preserved (origin_asns present) + assert!( + obj.get("origin_asns").is_some(), + "rfc3339 should still use native serialization shape, not field-filtered map" + ); + // But the timestamp should be overridden to a string + let ts = obj.get("timestamp").unwrap(); + assert!(ts.is_string(), "rfc3339 timestamp should be a string"); + assert!(ts.as_str().unwrap().contains('T')); } } From ef93b28131d2a86aacf68159d2364a9a38367b78 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sun, 28 Jun 2026 16:44:09 -0700 Subject: [PATCH 3/3] fix: allow clippy::unwrap_used in elem_format test module The crate denies clippy::unwrap_used but .unwrap() is idiomatic in test code where panicking is the correct behavior. Add #[allow] on the test module to satisfy clippy --tests while keeping tests readable. Addresses PR review feedback. --- src/bin/commands/elem_format.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bin/commands/elem_format.rs b/src/bin/commands/elem_format.rs index 5404de9..4745b65 100644 --- a/src/bin/commands/elem_format.rs +++ b/src/bin/commands/elem_format.rs @@ -454,6 +454,7 @@ pub fn sort_elems( } #[cfg(test)] +#[allow(clippy::unwrap_used)] mod tests { use super::*; use bgpkit_parser::models::{AsPath, AsPathSegment, ElemType, NetworkPrefix, Origin};