Merge pull request #2010 from input-output-hk/djo/1981/standardize_lo…

…gs_in_common_signer_persitence

Standardize logs in common, signer, and persistence

internal/mithril-persistence/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-persistence"
-version = "0.2.29"
+version = "0.2.30"
 description = "Common types, interfaces, and utilities to persist data for Mithril nodes."
 authors = { workspace = true }
 edition = { workspace = true }

internal/mithril-persistence/src/database/version_checker.rs

@@ -54,7 +54,7 @@ impl<'conn> DatabaseVersionChecker<'conn> {
 
     /// Apply migrations
     pub fn apply(&self) -> StdResult<()> {
-        debug!(&self.logger, "check database version",);
+        debug!(&self.logger, "Check database version",);
         self.create_table_if_not_exists(&self.application_type)
             .with_context(|| "Can not create table 'db_version' while applying migrations")?;
         let db_version = self
@@ -80,7 +80,7 @@ impl<'conn> DatabaseVersionChecker<'conn> {
                 self.apply_migrations(&db_version, self.connection)?;
                 info!(
                     &self.logger,
-                    "database upgraded to version '{}'", migration_version
+                    "Database upgraded to version '{migration_version}'"
                 );
             }
             Ordering::Less => {

mithril-aggregator/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.5.83"
+version = "0.5.84"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-aggregator/src/dependency_injection/builder.rs

@@ -1118,7 +1118,6 @@ impl DependenciesBuilder {
         >::new(
             transactions_importer,
             block_range_root_retriever,
-            self.root_logger(),
         ));
         let cardano_stake_distribution_builder = Arc::new(
             CardanoStakeDistributionSignableBuilder::new(self.get_stake_store().await?),
@@ -1131,6 +1130,7 @@ impl DependenciesBuilder {
             immutable_signable_builder,
             cardano_transactions_builder,
             cardano_stake_distribution_builder,
+            self.root_logger(),
         ));
 
         Ok(signable_builder_service)

mithril-common/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-common"
-version = "0.4.69"
+version = "0.4.70"
 description = "Common types, interfaces, and utilities for Mithril nodes."
 authors = { workspace = true }
 edition = { workspace = true }

mithril-common/src/cardano_block_scanner/chain_reader_block_streamer.rs

@@ -32,7 +32,7 @@ pub struct ChainReaderBlockStreamer {
 #[async_trait]
 impl BlockStreamer for ChainReaderBlockStreamer {
     async fn poll_next(&mut self) -> StdResult<Option<ChainScannedBlocks>> {
-        debug!(self.logger, "polls next");
+        debug!(self.logger, ">> poll_next");
 
         let chain_scanned_blocks: ChainScannedBlocks;
         let mut roll_forwards = vec![];
@@ -123,14 +123,14 @@ impl ChainReaderBlockStreamer {
                 if parsed_block.block_number > self.until {
                     trace!(
                         self.logger,
-                        "received a RollForward above threshold block number ({})",
+                        "Received a RollForward above threshold block number ({})",
                         parsed_block.block_number
                     );
                     Ok(None)
                 } else {
                     trace!(
                         self.logger,
-                        "received a RollForward below threshold block number ({})",
+                        "Received a RollForward below threshold block number ({})",
                         parsed_block.block_number
                     );
                     Ok(Some(BlockStreamerNextAction::ChainBlockNextAction(
@@ -143,7 +143,7 @@ impl ChainReaderBlockStreamer {
             }) => {
                 trace!(
                     self.logger,
-                    "received a RollBackward({rollback_slot_number:?})"
+                    "Received a RollBackward({rollback_slot_number:?})"
                 );
                 let block_streamer_next_action = if rollback_slot_number == self.from.slot_number {
                     BlockStreamerNextAction::SkipToNextAction
@@ -157,7 +157,7 @@ impl ChainReaderBlockStreamer {
                 Ok(Some(block_streamer_next_action))
             }
             None => {
-                trace!(self.logger, "received nothing");
+                trace!(self.logger, "Received nothing");
                 Ok(None)
             }
         }

mithril-common/src/certificate_chain/certificate_verifier.rs

@@ -223,8 +223,8 @@ impl MithrilCertificateVerifier {
             None => Ok(None),
             _ => {
                 debug!(
-                    self.logger,
-                    "Previous certificate {:#?}", previous_certificate
+                    self.logger, "Certificate chain AVK unmatch";
+                    "previous_certificate" => #?previous_certificate
                 );
                 Err(anyhow!(
                     CertificateVerifierError::CertificateChainAVKUnmatch
@@ -265,8 +265,7 @@ impl CertificateVerifier for MithrilCertificateVerifier {
         genesis_verification_key: &ProtocolGenesisVerificationKey,
     ) -> StdResult<Option<Certificate>> {
         debug!(
-            self.logger,
-            "Verifying certificate";
+            self.logger, "Verifying certificate";
             "certificate_hash" => &certificate.hash,
             "certificate_previous_hash" => &certificate.previous_hash,
             "certificate_epoch" => ?certificate.epoch,

mithril-common/src/chain_reader/pallas_chain_reader.rs

@@ -46,7 +46,7 @@ impl PallasChainReader {
     async fn get_client(&mut self) -> StdResult<&mut NodeClient> {
         if self.client.is_none() {
             self.client = Some(self.new_client().await?);
-            debug!(self.logger, "connected to a new client");
+            debug!(self.logger, "Connected to a new client");
         }
 
         self.client
@@ -61,12 +61,12 @@ impl PallasChainReader {
         let chainsync = client.chainsync();
 
         if chainsync.has_agency() {
-            debug!(logger, "has agency, finding intersect point..."; "point" => ?point);
+            debug!(logger, "Has agency, finding intersect point..."; "point" => ?point);
             chainsync
                 .find_intersect(vec![point.to_owned().into()])
                 .await?;
         } else {
-            debug!(logger, "doesn't have agency, no need to find intersect point";);
+            debug!(logger, "Doesn't have agency, no need to find intersect point";);
         }
 
         Ok(())

mithril-common/src/digesters/cardano_immutable_digester.rs

@@ -50,6 +50,7 @@ impl ImmutableDigester for CardanoImmutableDigester {
             .into_iter()
             .filter(|f| f.number <= up_to_file_number)
             .collect::<Vec<_>>();
+        info!(self.logger, ">> compute_digest"; "beacon" => #?beacon, "nb_of_immutables" => immutables.len());
 
         match immutables.last() {
             None => Err(ImmutableDigesterError::NotEnoughImmutable {
@@ -65,16 +66,14 @@ impl ImmutableDigester for CardanoImmutableDigester {
                 })
             }
             Some(_) => {
-                info!(self.logger, "#compute_digest"; "beacon" => #?beacon, "nb_of_immutables" => immutables.len());
-
                 let cached_values = match self.cache_provider.as_ref() {
                     None => BTreeMap::from_iter(immutables.into_iter().map(|i| (i, None))),
                     Some(cache_provider) => match cache_provider.get(immutables.clone()).await {
                         Ok(values) => values,
                         Err(error) => {
                             warn!(
-                                self.logger,
-                                "Error while getting cached immutable files digests: {}", error
+                                self.logger, "Error while getting cached immutable files digests";
+                                "error" => ?error
                             );
                             BTreeMap::from_iter(immutables.into_iter().map(|i| (i, None)))
                         }
@@ -92,13 +91,13 @@ impl ImmutableDigester for CardanoImmutableDigester {
                     .map_err(|e| ImmutableDigesterError::DigestComputationError(e.into()))??;
                 let digest = hex::encode(hash);
 
-                debug!(self.logger, "#computed digest: {:?}", digest);
+                debug!(self.logger, "Computed digest: {digest:?}");
 
                 if let Some(cache_provider) = self.cache_provider.as_ref() {
                     if let Err(error) = cache_provider.store(new_cache_entries).await {
                         warn!(
-                            self.logger,
-                            "Error while storing new immutable files digests to cache: {}", error
+                            self.logger, "Error while storing new immutable files digests to cache";
+                            "error" => ?error
                         );
                     }
                 }
@@ -136,7 +135,7 @@ fn compute_hash(
         };
 
         if progress.report(ix) {
-            info!(logger, "hashing: {}", &progress);
+            info!(logger, "Hashing: {progress}");
         }
     }
 

mithril-common/src/logging.rs

@@ -23,7 +23,14 @@ impl LoggerExtensions for Logger {
 
 fn component_name<T>() -> &'static str {
     let complete_name = std::any::type_name::<T>();
-    complete_name.split("::").last().unwrap_or(complete_name)
+    let without_generic = {
+        if complete_name.contains('<') {
+            complete_name.split('<').next().unwrap_or("")
+        } else {
+            complete_name
+        }
+    };
+    without_generic.split("::").last().unwrap_or(complete_name)
 }
 
 #[cfg(test)]
@@ -38,6 +45,10 @@ mod tests {
     struct TestStructWithLifetime<'a>(&'a str);
     enum TestEnum {}
 
+    struct TestStructWithGeneric<T> {
+        _phantom: std::marker::PhantomData<T>,
+    }
+
     mod test_mod {
         pub struct ScopedTestStruct;
         pub enum ScopedTestEnum {}
@@ -66,6 +77,14 @@ mod tests {
             component_name::<TestStructWithLifetime>(),
             "TestStructWithLifetime"
         );
+        assert_eq!(
+            component_name::<TestStructWithGeneric<test_mod::ScopedTestStruct>>(),
+            "TestStructWithGeneric"
+        );
+        assert_eq!(
+            component_name::<TestStructWithGeneric<&str>>(),
+            "TestStructWithGeneric"
+        );
     }
 
     #[test]

mithril-common/src/messages/cardano_transactions_proof.rs

@@ -322,7 +322,6 @@ mod tests {
             CardanoTransactionsSignableBuilder, MockBlockRangeRootRetriever,
             MockTransactionsImporter, SignableBuilder,
         };
-        use slog::Logger;
         use std::sync::Arc;
 
         use super::*;
@@ -416,7 +415,6 @@ mod tests {
             let cardano_transaction_signable_builder = CardanoTransactionsSignableBuilder::new(
                 Arc::new(transaction_importer),
                 Arc::new(block_range_root_retriever),
-                Logger::root(slog::Discard, slog::o!()),
             );
             cardano_transaction_signable_builder
                 .compute_protocol_message(block_number)

mithril-common/src/signable_builder/cardano_immutable_full_signable_builder.rs

@@ -12,9 +12,9 @@ use crate::{
 };
 use anyhow::Context;
 use async_trait::async_trait;
-use slog::{debug, info, Logger};
+use slog::{info, Logger};
 
-/// This structure is responsible of calculating the message for Cardano immutable files snapshots.
+/// This structure is responsible for calculating the message for Cardano immutable files snapshots.
 pub struct CardanoImmutableFilesFullSignableBuilder {
     immutable_digester: Arc<dyn ImmutableDigester>,
     logger: Logger,
@@ -42,7 +42,6 @@ impl SignableBuilder<CardanoDbBeacon> for CardanoImmutableFilesFullSignableBuild
         &self,
         beacon: CardanoDbBeacon,
     ) -> StdResult<ProtocolMessage> {
-        debug!(self.logger, "compute_signable({beacon:?})");
         let digest = self
             .immutable_digester
             .compute_digest(&self.dirpath, &beacon)
@@ -53,7 +52,7 @@ impl SignableBuilder<CardanoDbBeacon> for CardanoImmutableFilesFullSignableBuild
                     &self.dirpath.display()
                 )
             })?;
-        info!(self.logger, "digest = '{digest}'.");
+        info!(self.logger, "Computed Digest = '{digest}'.");
         let mut protocol_message = ProtocolMessage::new();
         protocol_message.set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest);
 

mithril-common/src/signable_builder/cardano_transactions.rs

@@ -2,12 +2,10 @@ use std::sync::Arc;
 
 use anyhow::Context;
 use async_trait::async_trait;
-use slog::{debug, Logger};
 
 use crate::{
     crypto_helper::{MKMap, MKMapNode, MKTreeNode, MKTreeStorer},
     entities::{BlockNumber, BlockRange, ProtocolMessage, ProtocolMessagePartKey},
-    logging::LoggerExtensions,
     signable_builder::SignableBuilder,
     StdResult,
 };
@@ -53,32 +51,24 @@ pub trait BlockRangeRootRetriever<S: MKTreeStorer>: Send + Sync {
 pub struct CardanoTransactionsSignableBuilder<S: MKTreeStorer> {
     transaction_importer: Arc<dyn TransactionsImporter>,
     block_range_root_retriever: Arc<dyn BlockRangeRootRetriever<S>>,
-    logger: Logger,
 }
 
 impl<S: MKTreeStorer> CardanoTransactionsSignableBuilder<S> {
     /// Constructor
     pub fn new(
         transaction_importer: Arc<dyn TransactionsImporter>,
         block_range_root_retriever: Arc<dyn BlockRangeRootRetriever<S>>,
-        logger: Logger,
     ) -> Self {
         Self {
             transaction_importer,
             block_range_root_retriever,
-            logger: logger.new_with_component_name::<Self>(),
         }
     }
 }
 
 #[async_trait]
 impl<S: MKTreeStorer> SignableBuilder<BlockNumber> for CardanoTransactionsSignableBuilder<S> {
     async fn compute_protocol_message(&self, beacon: BlockNumber) -> StdResult<ProtocolMessage> {
-        debug!(
-            self.logger,
-            "Compute protocol message for CardanoTransactions at block_number: {beacon}"
-        );
-
         self.transaction_importer.import(beacon).await?;
 
         let mk_root = self
@@ -105,9 +95,8 @@ impl<S: MKTreeStorer> SignableBuilder<BlockNumber> for CardanoTransactionsSignab
 mod tests {
 
     use crate::{
-        crypto_helper::MKTreeStoreInMemory,
-        entities::CardanoTransaction,
-        test_utils::{CardanoTransactionsBuilder, TestLogger},
+        crypto_helper::MKTreeStoreInMemory, entities::CardanoTransaction,
+        test_utils::CardanoTransactionsBuilder,
     };
 
     use super::*;
@@ -143,7 +132,6 @@ mod tests {
         let cardano_transactions_signable_builder = CardanoTransactionsSignableBuilder::new(
             Arc::new(transaction_importer),
             Arc::new(block_range_root_retriever),
-            TestLogger::stdout(),
         );
 
         // Action
@@ -177,7 +165,6 @@ mod tests {
         let cardano_transactions_signable_builder = CardanoTransactionsSignableBuilder::new(
             Arc::new(transaction_importer),
             Arc::new(block_range_root_retriever),
-            TestLogger::stdout(),
         );
 
         let result = cardano_transactions_signable_builder