From 4dfe469aa39fd7d004a81898bcfb071242828a17 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Tue, 31 Mar 2026 21:05:48 +0200 Subject: [PATCH 1/4] Add context to bgpkit warning messages When reading logs, lines like ``` WARN worker-13 mrt_file: not enough bytes: input bytes left - 3, want to read - 1024; skipping ^]]3mfile=./2025-11-05/rrc00/2025.11/updates.20251105.0600.gz WARN worker-26 mrt_file: not enough bytes: input bytes left - 3, want to read - 1024; skipping ^]]3mfile=./2025-11-05/rrc15/2025.11/updates.20251105.0605.gz ``` Made me think the full file was skipped, while only the attributes for this message were skipped. I think adding context to the warning messages clarifies them. --- src/parser/bgp/attributes/attr_07_18_aggregator.rs | 2 +- src/parser/bgp/attributes/attr_14_15_nlri.rs | 8 ++++---- src/parser/bgp/attributes/mod.rs | 4 ++-- src/parser/bgp/messages.rs | 12 ++++++------ src/parser/bmp/messages/headers.rs | 4 ++-- src/parser/bmp/messages/peer_up_notification.rs | 6 +++--- src/parser/bmp/messages/route_monitoring.rs | 2 +- .../mrt/messages/table_dump_v2/rib_afi_entries.rs | 2 +- src/parser/mrt/mrt_record.rs | 2 +- 9 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/parser/bgp/attributes/attr_07_18_aggregator.rs b/src/parser/bgp/attributes/attr_07_18_aggregator.rs index 0a481cfd..db031a5b 100644 --- a/src/parser/bgp/attributes/attr_07_18_aggregator.rs +++ b/src/parser/bgp/attributes/attr_07_18_aggregator.rs @@ -32,7 +32,7 @@ pub fn parse_aggregator( }; if asn_len_found != *asn_len { warn!( - "Aggregator attribute with ASN length set to {:?} but found {:?}", + "Aggregator attribute with ASN length set to {:?} but found {:?} (parsing Aggregator attribute)", asn_len, asn_len_found ); } diff --git a/src/parser/bgp/attributes/attr_14_15_nlri.rs b/src/parser/bgp/attributes/attr_14_15_nlri.rs index 25b3b7b1..aaa9e814 100644 --- a/src/parser/bgp/attributes/attr_14_15_nlri.rs +++ b/src/parser/bgp/attributes/attr_14_15_nlri.rs @@ -70,7 +70,7 @@ pub fn parse_nlri( if reachable { // skip reserved byte for reachable NRLI if input.read_u8()? != 0 { - warn!("NRLI reserved byte not 0"); + warn!("NLRI reserved byte not 0 (parsing NLRI prefixes)"); } } let ls_nlri = parse_link_state_nlri(input, afi, safi, next_hop, reachable)?; @@ -85,7 +85,7 @@ pub fn parse_nlri( if reachable { // skip reserved byte for reachable NRLI if input.read_u8()? != 0 { - warn!("NRLI reserved byte not 0"); + warn!("NLRI reserved byte not 0 (parsing NLRI Link-State)"); } } parse_nlri_list(input, additional_paths, &afi)? @@ -97,7 +97,7 @@ pub fn parse_nlri( if reachable { // skip reserved byte for reachable NRLI if input.read_u8()? != 0 { - warn!("NRLI reserved byte not 0"); + warn!("NLRI reserved byte not 0 (parsing NLRI prefixes)"); } } parse_nlri_list(input, additional_paths, &afi)? @@ -131,7 +131,7 @@ pub fn encode_nlri(nlri: &Nlri, reachable: bool) -> Bytes { if let Some(next_hop) = &nlri.next_hop { if !reachable { - warn!("NLRI next hop should not be set for unreachable NLRI"); + warn!("NLRI next hop should not be set for unreachable NLRI (encoding NLRI)"); } // encode next hop let next_hop_bytes = match next_hop { diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index b94b1c50..5cab0ec9 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -261,8 +261,8 @@ pub fn parse_attributes( if data.remaining() < attr_length { warn!( - "not enough bytes: input bytes left - {}, want to read - {}; skipping", - bytes_left, attr_length + "{:?} attribute encodes a length ({}) that is longer than the remaining attribute data ({}). Skipping remaining attribute data for BGP message", + attr_type, attr_length, bytes_left ); // break and return already parsed attributes break; diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index f72ac88b..ca9ce58d 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -120,7 +120,7 @@ pub fn parse_bgp_message( if data.remaining() != bgp_msg_length { warn!( - "BGP message length {} does not match the actual length {}", + "BGP message length {} does not match the actual length {} (parsing BgpMessage)", bgp_msg_length, data.remaining() ); @@ -256,10 +256,10 @@ pub fn parse_bgp_open_message(input: &mut Bytes) -> Result Result Result 255 { warn!( - "RFC 9069: VrTableName TLV length must be 1-255 bytes, found {} bytes", + "RFC 9069: VrTableName TLV length must be 1-255 bytes, found {} bytes (parsing PeerUpNotification)", info_value.len() ); } @@ -132,7 +132,7 @@ pub fn parse_peer_up_notification( // RFC 9069: Local RIB instances SHOULD include VrTableName TLV if let Some(BmpPeerType::LocalRib) = peer_type { if !has_vr_table_name { - warn!("RFC 9069: Local RIB peer up notification should include VrTableName TLV"); + warn!("RFC 9069: Local RIB peer up notification should include VrTableName TLV (parsing PeerUpNotification)"); } } Ok(PeerUpNotification { diff --git a/src/parser/bmp/messages/route_monitoring.rs b/src/parser/bmp/messages/route_monitoring.rs index 8aafd8e6..079e9e45 100644 --- a/src/parser/bmp/messages/route_monitoring.rs +++ b/src/parser/bmp/messages/route_monitoring.rs @@ -19,7 +19,7 @@ pub fn parse_route_monitoring( // RFC 9069: Local RIB MUST use 4-byte ASN encoding if let Some(BmpPeerType::LocalRib) = peer_type { if *asn_len != AsnLength::Bits32 { - warn!("RFC 9069 violation: Local RIB route monitoring MUST use 4-byte ASN encoding"); + warn!("RFC 9069 violation: Local RIB route monitoring MUST use 4-byte ASN encoding (parsing RouteMonitoring)"); } } diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index f1be2e81..84f623cb 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -78,7 +78,7 @@ pub fn parse_rib_afi_entries( let entry = match parse_rib_entry(data, is_add_path, &afi, &safi, prefix) { Ok(entry) => entry, Err(e) => { - warn!("early break due to error {}", e); + warn!("early break due to error {} while parsing RibAfiEntries", e); break; } }; diff --git a/src/parser/mrt/mrt_record.rs b/src/parser/mrt/mrt_record.rs index 21d68ec8..4e1c8a12 100644 --- a/src/parser/mrt/mrt_record.rs +++ b/src/parser/mrt/mrt_record.rs @@ -230,7 +230,7 @@ impl MrtRecord { let mut new_header = self.common_header; if message_bytes.len() != new_header.length as usize { warn!( - "message length {} does not match the length in the header {}", + "message length {} does not match the length in the header {} (encoding MrtRecord)", message_bytes.len(), new_header.length ); From e3fdac7fd132e396ca56633b45007b542ca990b9 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Tue, 31 Mar 2026 21:14:09 +0200 Subject: [PATCH 2/4] Fix messages on correct path --- src/parser/bgp/attributes/attr_14_15_nlri.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parser/bgp/attributes/attr_14_15_nlri.rs b/src/parser/bgp/attributes/attr_14_15_nlri.rs index aaa9e814..91eff5c8 100644 --- a/src/parser/bgp/attributes/attr_14_15_nlri.rs +++ b/src/parser/bgp/attributes/attr_14_15_nlri.rs @@ -70,7 +70,7 @@ pub fn parse_nlri( if reachable { // skip reserved byte for reachable NRLI if input.read_u8()? != 0 { - warn!("NLRI reserved byte not 0 (parsing NLRI prefixes)"); + warn!("NLRI reserved byte not 0 (parsing NLRI Link-State)"); } } let ls_nlri = parse_link_state_nlri(input, afi, safi, next_hop, reachable)?; @@ -85,7 +85,7 @@ pub fn parse_nlri( if reachable { // skip reserved byte for reachable NRLI if input.read_u8()? != 0 { - warn!("NLRI reserved byte not 0 (parsing NLRI Link-State)"); + warn!("NLRI reserved byte not 0 (parsing NLRI prefixes)"); } } parse_nlri_list(input, additional_paths, &afi)? From 272edd0ad9b8af4f725c941d7184c7001ced34d7 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Tue, 31 Mar 2026 21:33:00 +0200 Subject: [PATCH 3/4] Use record names from RFCs --- src/parser/bgp/messages.rs | 6 +++--- src/parser/bmp/messages/headers.rs | 4 ++-- src/parser/bmp/messages/peer_up_notification.rs | 6 +++--- src/parser/bmp/messages/route_monitoring.rs | 2 +- src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs | 2 +- src/parser/mrt/mrt_record.rs | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index ca9ce58d..81d07aad 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -120,7 +120,7 @@ pub fn parse_bgp_message( if data.remaining() != bgp_msg_length { warn!( - "BGP message length {} does not match the actual length {} (parsing BgpMessage)", + "BGP message length {} does not match the actual length {} (parsing BGP message)", bgp_msg_length, data.remaining() ); @@ -256,7 +256,7 @@ pub fn parse_bgp_open_message(input: &mut Bytes) -> Result Result Result 255 { warn!( - "RFC 9069: VrTableName TLV length must be 1-255 bytes, found {} bytes (parsing PeerUpNotification)", + "RFC 9069: VrTableName TLV length must be 1-255 bytes, found {} bytes (parsing Peer Up Notification)", info_value.len() ); } @@ -132,7 +132,7 @@ pub fn parse_peer_up_notification( // RFC 9069: Local RIB instances SHOULD include VrTableName TLV if let Some(BmpPeerType::LocalRib) = peer_type { if !has_vr_table_name { - warn!("RFC 9069: Local RIB peer up notification should include VrTableName TLV (parsing PeerUpNotification)"); + warn!("RFC 9069: Local RIB peer up notification should include VrTableName TLV (parsing Peer Up Notification)"); } } Ok(PeerUpNotification { diff --git a/src/parser/bmp/messages/route_monitoring.rs b/src/parser/bmp/messages/route_monitoring.rs index 079e9e45..98b98c25 100644 --- a/src/parser/bmp/messages/route_monitoring.rs +++ b/src/parser/bmp/messages/route_monitoring.rs @@ -19,7 +19,7 @@ pub fn parse_route_monitoring( // RFC 9069: Local RIB MUST use 4-byte ASN encoding if let Some(BmpPeerType::LocalRib) = peer_type { if *asn_len != AsnLength::Bits32 { - warn!("RFC 9069 violation: Local RIB route monitoring MUST use 4-byte ASN encoding (parsing RouteMonitoring)"); + warn!("RFC 9069 violation: Local RIB route monitoring MUST use 4-byte ASN encoding (parsing Route Monitoring)"); } } diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index 84f623cb..c7176d7b 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -78,7 +78,7 @@ pub fn parse_rib_afi_entries( let entry = match parse_rib_entry(data, is_add_path, &afi, &safi, prefix) { Ok(entry) => entry, Err(e) => { - warn!("early break due to error {} while parsing RibAfiEntries", e); + warn!("early break due to error {} while parsing RIB AFI entries", e); break; } }; diff --git a/src/parser/mrt/mrt_record.rs b/src/parser/mrt/mrt_record.rs index 4e1c8a12..7de422e7 100644 --- a/src/parser/mrt/mrt_record.rs +++ b/src/parser/mrt/mrt_record.rs @@ -230,7 +230,7 @@ impl MrtRecord { let mut new_header = self.common_header; if message_bytes.len() != new_header.length as usize { warn!( - "message length {} does not match the length in the header {} (encoding MrtRecord)", + "message length {} does not match the length in the header {} (encoding MRT record)", message_bytes.len(), new_header.length ); From a2acfdb86b5a5c52017ad2c997ab697d9e44b9cb Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 31 Mar 2026 14:03:04 -0700 Subject: [PATCH 4/4] Apply cargo fmt formatting fixes and make coverage checks informational --- CHANGELOG.md | 4 ++++ codecov.yml | 8 +++++++- src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs | 5 ++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bff9ea52..16293c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Improvements + +* **Enhanced warning messages**: Added context to 14 warning messages across BGP, BMP, and MRT parsers to help identify which parsing stage encountered an issue. Also fixed typo in NLRI warning ("NRLI" → "NLRI"). + ### Bug fixes * **WASM `only_to_customer` serialization**: Added test to verify that messages without the OTC (Only To Customer) attribute correctly serialize `only_to_customer` as `null` (not `0`) in JSON output. This ensures JavaScript consumers can properly distinguish between "no OTC attribute" (`null`) and "OTC with AS 0" (`0`). diff --git a/codecov.yml b/codecov.yml index 09273ce3..97c4f36e 100644 --- a/codecov.yml +++ b/codecov.yml @@ -14,4 +14,10 @@ coverage: target: auto # adjust accordingly based on how flaky your tests are # this allows a 10% drop from the previous base commit coverage - threshold: 10% \ No newline at end of file + threshold: 10% + # make coverage check informational (non-blocking) + informational: true + patch: + default: + # make patch coverage check informational (non-blocking) + informational: true \ No newline at end of file diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index c7176d7b..c2513136 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -78,7 +78,10 @@ pub fn parse_rib_afi_entries( let entry = match parse_rib_entry(data, is_add_path, &afi, &safi, prefix) { Ok(entry) => entry, Err(e) => { - warn!("early break due to error {} while parsing RIB AFI entries", e); + warn!( + "early break due to error {} while parsing RIB AFI entries", + e + ); break; } };