From 79967fe6b1ad05787e87de03046347cc93af80cb Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 09:32:56 +0100 Subject: [PATCH 1/6] Replace `log` with `tracing` on `pallet-bridge-grandpa` --- Cargo.lock | 2 +- bridges/modules/grandpa/Cargo.toml | 4 +-- bridges/modules/grandpa/src/call_ext.rs | 30 ++++++++--------- bridges/modules/grandpa/src/lib.rs | 43 ++++++++++++------------- 4 files changed, 38 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e50fd52007c3..4ae3ef84b0b42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11516,7 +11516,6 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log", "parity-scale-codec", "scale-info", "sp-consensus-grandpa", @@ -11524,6 +11523,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-std 14.0.0", + "tracing", ] [[package]] diff --git a/bridges/modules/grandpa/Cargo.toml b/bridges/modules/grandpa/Cargo.toml index 22a8439ed46d1..31ff0b43101d9 100644 --- a/bridges/modules/grandpa/Cargo.toml +++ b/bridges/modules/grandpa/Cargo.toml @@ -14,8 +14,8 @@ workspace = true [dependencies] codec = { workspace = true } -log = { workspace = true } scale-info = { features = ["derive"], workspace = true } +tracing = { workspace = true } # Bridge Dependencies bp-header-chain = { workspace = true } @@ -47,11 +47,11 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", - "log/std", "scale-info/std", "sp-consensus-grandpa/std", "sp-runtime/std", "sp-std/std", + "tracing/std", ] runtime-benchmarks = [ "bp-test-utils", diff --git a/bridges/modules/grandpa/src/call_ext.rs b/bridges/modules/grandpa/src/call_ext.rs index d964901ba4bc6..c491e3a40d6c7 100644 --- a/bridges/modules/grandpa/src/call_ext.rs +++ b/bridges/modules/grandpa/src/call_ext.rs @@ -85,7 +85,7 @@ impl, I: 'static> SubmitFinalityProofHelper { // else - if we can not accept more free headers, "reject" the transaction if !Self::has_free_header_slots() { - log::trace!( + tracing::trace!( target: crate::LOG_TARGET, "Cannot accept free {:?} header {:?}. No more free slots remaining", T::BridgedChain::ID, @@ -99,14 +99,12 @@ impl, I: 'static> SubmitFinalityProofHelper { if !call_info.is_mandatory { if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { if improved_by < free_headers_interval.into() { - log::trace!( + tracing::trace!( target: crate::LOG_TARGET, "Cannot accept free {:?} header {:?}. Too small difference \ - between submitted headers: {:?} vs {}", + between submitted headers: {improved_by:?} vs {free_headers_interval}", T::BridgedChain::ID, call_info.block_number, - improved_by, - free_headers_interval, ); return Err(Error::::BelowFreeHeaderInterval); @@ -137,10 +135,10 @@ impl, I: 'static> SubmitFinalityProofHelper { current_set_id: Option, ) -> Result, Error> { let best_finalized = BestFinalized::::get().ok_or_else(|| { - log::trace!( + tracing::trace!( target: crate::LOG_TARGET, - "Cannot finalize header {:?} because pallet is not yet initialized", - finality_target, + header=?finality_target, + "Cannot finalize header because pallet is not yet initialized" ); >::NotInitialized })?; @@ -148,11 +146,11 @@ impl, I: 'static> SubmitFinalityProofHelper { let improved_by = match finality_target.checked_sub(&best_finalized.number()) { Some(improved_by) if improved_by > Zero::zero() => improved_by, _ => { - log::trace!( + tracing::trace!( target: crate::LOG_TARGET, - "Cannot finalize obsolete header: bundled {:?}, best {:?}", - finality_target, - best_finalized, + bundled=?finality_target, + best=?best_finalized, + "Cannot finalize obsolete header" ); return Err(Error::::OldHeader) @@ -162,11 +160,11 @@ impl, I: 'static> SubmitFinalityProofHelper { if let Some(current_set_id) = current_set_id { let actual_set_id = >::get().set_id; if current_set_id != actual_set_id { - log::trace!( + tracing::trace!( target: crate::LOG_TARGET, - "Cannot finalize header signed by unknown authority set: bundled {:?}, best {:?}", - current_set_id, - actual_set_id, + bundled=?current_set_id, + best=?actual_set_id, + "Cannot finalize header signed by unknown authority set" ); return Err(Error::::InvalidAuthoritySetId) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 81cf14e21ed70..1317a480a43be 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -219,10 +219,10 @@ pub mod pallet { ensure!(init_allowed, >::AlreadyInitialized); initialize_bridge::(init_data.clone())?; - log::info!( + tracing::info!( target: LOG_TARGET, - "Pallet has been initialized with the following parameters: {:?}", - init_data + parameters=?init_data, + "Pallet has been initialized" ); Ok(().into()) @@ -292,10 +292,10 @@ pub mod pallet { ensure_signed(origin)?; let (hash, number) = (finality_target.hash(), *finality_target.number()); - log::trace!( + tracing::trace!( target: LOG_TARGET, - "Going to try and finalize header {:?}", - finality_target + header=?finality_target, + "Going to try and finalize header" ); // it checks whether the `number` is better than the current best block number @@ -332,10 +332,10 @@ pub mod pallet { // to pay for the transaction. let pays_fee = if may_refund_call_fee { Pays::No } else { Pays::Yes }; - log::info!( + tracing::info!( target: LOG_TARGET, - "Successfully imported finalized header with hash {:?}! Free: {}", - hash, + ?hash, + "Successfully imported finalized header! Free: {}", if may_refund_call_fee { "Yes" } else { "No" }, ); @@ -656,12 +656,12 @@ pub mod pallet { >::put(&next_authorities); - log::info!( + tracing::info!( target: LOG_TARGET, - "Transitioned from authority set {} to {}! New authorities are: {:?}", - old_current_set_id, - new_current_set_id, - next_authorities, + %old_current_set_id, + %new_current_set_id, + ?next_authorities, + "Transitioned from authority set!" ); Ok(Some(next_authorities.into())) @@ -687,11 +687,11 @@ pub mod pallet { justification, ) .map_err(|e| { - log::error!( + tracing::error!( target: LOG_TARGET, - "Received invalid justification for {:?}: {:?}", - hash, - e, + error=?e, + ?hash, + "Received invalid justification" ); >::InvalidJustification })?) @@ -714,7 +714,7 @@ pub mod pallet { // Update ring buffer pointer and remove old header. >::put((index + 1) % T::HeadersToKeep::get()); if let Ok(hash) = pruning { - log::debug!(target: LOG_TARGET, "Pruning old header: {:?}.", hash); + tracing::debug!(target: LOG_TARGET, ?hash, "Pruning old header."); >::remove(hash); } } @@ -729,10 +729,9 @@ pub mod pallet { let authority_set_length = authority_list.len(); let authority_set = StoredAuthoritySet::::try_new(authority_list, set_id) .inspect_err(|_| { - log::error!( + tracing::error!( target: LOG_TARGET, - "Failed to initialize bridge. Number of authorities in the set {} is larger than the configured value {}", - authority_set_length, + "Failed to initialize bridge. Number of authorities in the set {authority_set_length} is larger than the configured value {}", T::BridgedChain::MAX_AUTHORITIES_COUNT, ); })?; From 858235862a23e06d756158e442aa673da3e136b9 Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 09:35:08 +0100 Subject: [PATCH 2/6] Add PRDoc --- prdoc/pr_9294.prdoc | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 prdoc/pr_9294.prdoc diff --git a/prdoc/pr_9294.prdoc b/prdoc/pr_9294.prdoc new file mode 100644 index 0000000000000..37375124e7ddf --- /dev/null +++ b/prdoc/pr_9294.prdoc @@ -0,0 +1,8 @@ +title: Replace `log` with `tracing` on `pallet-bridge-grandpa` +doc: +- audience: Runtime Dev + description: This PR replaces `log` with `tracing` instrumentation on `pallet-bridge-grandpa` + by providing structured logging. +crates: +- name: bridge-runtime-common + bump: minor From 45632ae2ed7a871a985c1063a8f3f720583b9592 Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 12:11:37 +0100 Subject: [PATCH 3/6] Update PRDoc --- bridges/modules/grandpa/src/call_ext.rs | 4 +++- prdoc/pr_9294.prdoc | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bridges/modules/grandpa/src/call_ext.rs b/bridges/modules/grandpa/src/call_ext.rs index c491e3a40d6c7..fe7c3f1a5ac0b 100644 --- a/bridges/modules/grandpa/src/call_ext.rs +++ b/bridges/modules/grandpa/src/call_ext.rs @@ -101,8 +101,10 @@ impl, I: 'static> SubmitFinalityProofHelper { if improved_by < free_headers_interval.into() { tracing::trace!( target: crate::LOG_TARGET, + ?improved_by, + %free_headers_interval, "Cannot accept free {:?} header {:?}. Too small difference \ - between submitted headers: {improved_by:?} vs {free_headers_interval}", + between submitted headers", T::BridgedChain::ID, call_info.block_number, ); diff --git a/prdoc/pr_9294.prdoc b/prdoc/pr_9294.prdoc index 37375124e7ddf..4de9a3ad26367 100644 --- a/prdoc/pr_9294.prdoc +++ b/prdoc/pr_9294.prdoc @@ -4,5 +4,5 @@ doc: description: This PR replaces `log` with `tracing` instrumentation on `pallet-bridge-grandpa` by providing structured logging. crates: -- name: bridge-runtime-common +- name: pallet-bridge-grandpa bump: minor From 1be97ce38f1d33fe2a70a6f01f5f85cb44bcffcd Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 13:55:09 +0100 Subject: [PATCH 4/6] Update tracing --- bridges/modules/grandpa/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 1317a480a43be..b2eebd200afdd 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -335,8 +335,8 @@ pub mod pallet { tracing::info!( target: LOG_TARGET, ?hash, - "Successfully imported finalized header! Free: {}", - if may_refund_call_fee { "Yes" } else { "No" }, + ?pays_fee, + "Successfully imported finalized header!" ); // the proof size component of the call weight assumes that there are @@ -731,7 +731,8 @@ pub mod pallet { .inspect_err(|_| { tracing::error!( target: LOG_TARGET, - "Failed to initialize bridge. Number of authorities in the set {authority_set_length} is larger than the configured value {}", + %authority_set_length, + "Failed to initialize bridge. Number of authorities in the set is larger than the configured value {}", T::BridgedChain::MAX_AUTHORITIES_COUNT, ); })?; From 7da8b77dbd86237cac8d590cfa37b4a8418ab5ea Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:00:43 +0100 Subject: [PATCH 5/6] Update tracing v2 --- bridges/modules/grandpa/src/call_ext.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bridges/modules/grandpa/src/call_ext.rs b/bridges/modules/grandpa/src/call_ext.rs index fe7c3f1a5ac0b..1d819a3073935 100644 --- a/bridges/modules/grandpa/src/call_ext.rs +++ b/bridges/modules/grandpa/src/call_ext.rs @@ -87,9 +87,9 @@ impl, I: 'static> SubmitFinalityProofHelper { if !Self::has_free_header_slots() { tracing::trace!( target: crate::LOG_TARGET, - "Cannot accept free {:?} header {:?}. No more free slots remaining", - T::BridgedChain::ID, - call_info.block_number, + chain_id=?T::BridgedChain::ID, + block_number=?call_info.block_number, + "Cannot accept free header. No more free slots remaining" ); return Err(Error::::FreeHeadersLimitExceded); @@ -101,12 +101,11 @@ impl, I: 'static> SubmitFinalityProofHelper { if improved_by < free_headers_interval.into() { tracing::trace!( target: crate::LOG_TARGET, + chain_id=?T::BridgedChain::ID, + block_number=?call_info.block_number, ?improved_by, %free_headers_interval, - "Cannot accept free {:?} header {:?}. Too small difference \ - between submitted headers", - T::BridgedChain::ID, - call_info.block_number, + "Cannot accept free header. Too small difference between submitted headers" ); return Err(Error::::BelowFreeHeaderInterval); From 746c791c63bf7ce75895db7df4ba60e8b681cd30 Mon Sep 17 00:00:00 2001 From: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:03:52 +0100 Subject: [PATCH 6/6] Update tracing v3 --- bridges/modules/grandpa/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index b2eebd200afdd..aa70a8911ba6e 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -732,8 +732,8 @@ pub mod pallet { tracing::error!( target: LOG_TARGET, %authority_set_length, - "Failed to initialize bridge. Number of authorities in the set is larger than the configured value {}", - T::BridgedChain::MAX_AUTHORITIES_COUNT, + max_count=%T::BridgedChain::MAX_AUTHORITIES_COUNT, + "Failed to initialize bridge. Number of authorities in the set is larger than the configured value" ); })?; let initial_hash = header.hash();