Skip to content

Commit

Permalink
fix(logging): avoid logging large messages (#3081)
Browse files Browse the repository at this point in the history
Currently we log some large messages in network when they get dropped and it makes the log very hard to read on the debug level. This PR removes logging the full `RoutedMessage` by implementing `Debug` trait for it.
  • Loading branch information
bowenwang1996 authored Aug 7, 2020
1 parent 040cb15 commit c846cd1
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 9 deletions.
16 changes: 8 additions & 8 deletions chain/network/src/peer_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ impl PeerManagerActor {
ctx.spawn(async move {
while let Some(response) = requests.next().await {
if let Err(e) = response {
error!(target: "network", "Failed sending broadcast message(query_active_peers): {}", e);
debug!(target: "network", "Failed sending broadcast message(query_active_peers): {}", e);
}
}
}.into_actor(self));
Expand Down Expand Up @@ -742,7 +742,7 @@ impl PeerManagerActor {
ctx.spawn(async move {
while let Some(response) = requests.next().await {
if let Err(e) = response {
error!(target: "network", "Failed sending broadcast message(query_active_peers): {}", e);
debug!(target: "network", "Failed sending broadcast message(broadcast_message): {}", e);
}
}
}.into_actor(self));
Expand Down Expand Up @@ -770,7 +770,7 @@ impl PeerManagerActor {
message: PeerMessage,
) -> bool {
if let Some(active_peer) = self.active_peers.get(&peer_id) {
let msg_kind = format!("{}", message);
let msg_kind = message.msg_variant().to_string();
trace!(target: "network", "Send message: {}", msg_kind);
active_peer
.addr
Expand All @@ -789,9 +789,9 @@ impl PeerManagerActor {
true
} else {
debug!(target: "network",
"Sending message to: {} (which is not an active peer) Active Peers: {:?}\n{:?}",
"Sending message to: {} (which is not an active peer) Num active Peers: {}\n{}",
peer_id,
self.active_peers.keys(),
self.active_peers.len(),
message
);
false
Expand Down Expand Up @@ -845,12 +845,12 @@ impl PeerManagerActor {
.as_str(),
);

debug!(target: "network", "{:?} Drop signed message to {:?} Reason {:?}. Known peers: {:?} Message {:?}",
debug!(target: "network", "{:?} Drop signed message to {:?} Reason {:?}. Num known peers: {} Message {:?}",
self.config.account_id,
msg.target,
find_route_error,
self.routing_table.peer_forwarding.keys(),
msg,
self.routing_table.peer_forwarding.len(),
msg.body,
);
false
}
Expand Down
52 changes: 51 additions & 1 deletion chain/network/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ use crate::peer::Peer;
#[cfg(feature = "metric_recorder")]
use crate::recorder::MetricRecorder;
use crate::routing::{Edge, EdgeInfo, RoutingTableInfo};
use serde::export::fmt::Error;
use serde::export::Formatter;
use std::fmt::Debug;

/// Number of hops a message is allowed to travel before being dropped.
/// This is used to avoid infinite loop because of inconsistent view of the network
Expand Down Expand Up @@ -221,7 +224,6 @@ pub struct Pong {
PartialEq,
Eq,
Clone,
Debug,
strum::AsStaticStr,
strum::EnumVariantNames,
)]
Expand Down Expand Up @@ -254,6 +256,54 @@ pub enum RoutedMessageBody {
Pong(Pong),
}

impl Debug for RoutedMessageBody {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
match self {
RoutedMessageBody::BlockApproval(approval) => write!(
f,
"Approval({}, {}, {:?})",
approval.target_height, approval.account_id, approval.inner
),
RoutedMessageBody::ForwardTx(tx) => write!(f, "tx {}", tx.get_hash()),
RoutedMessageBody::TxStatusRequest(account_id, hash) => {
write!(f, "TxStatusRequest({}, {})", account_id, hash)
}
RoutedMessageBody::TxStatusResponse(response) => {
write!(f, "TxStatusResponse({})", response.transaction.hash)
}
RoutedMessageBody::QueryRequest { .. } => write!(f, "QueryRequest"),
RoutedMessageBody::QueryResponse { .. } => write!(f, "QueryResponse"),
RoutedMessageBody::ReceiptOutcomeRequest(hash) => write!(f, "ReceiptRequest({})", hash),
RoutedMessageBody::ReceiptOutComeResponse(response) => {
write!(f, "ReceiptResponse({})", response.outcome_with_id.id)
}
RoutedMessageBody::StateRequestHeader(shard_id, sync_hash) => {
write!(f, "StateRequestHeader({}, {})", shard_id, sync_hash)
}
RoutedMessageBody::StateRequestPart(shard_id, sync_hash, part_id) => {
write!(f, "StateRequestPart({}, {}, {})", shard_id, sync_hash, part_id)
}
RoutedMessageBody::StateResponse(response) => {
write!(f, "StateResponse({}, {})", response.shard_id, response.sync_hash)
}
RoutedMessageBody::PartialEncodedChunkRequest(request) => {
write!(f, "PartialChunkRequest({:?}, {:?})", request.chunk_hash, request.part_ords)
}
RoutedMessageBody::PartialEncodedChunkResponse(response) => write!(
f,
"PartialChunkResponse({:?}, {:?})",
response.chunk_hash,
response.parts.iter().map(|p| p.part_ord).collect::<Vec<_>>()
),
RoutedMessageBody::PartialEncodedChunk(chunk) => {
write!(f, "PartialChunk({:?})", chunk.header.hash)
}
RoutedMessageBody::Ping(_) => write!(f, "Ping"),
RoutedMessageBody::Pong(_) => write!(f, "Pong"),
}
}
}

#[derive(BorshSerialize, BorshDeserialize, Serialize, PartialEq, Eq, Clone, Debug)]
pub enum PeerIdOrHash {
PeerId(PeerId),
Expand Down

0 comments on commit c846cd1

Please sign in to comment.