chore: log message id if send queue full instead of full raw message#6263
chore: log message id if send queue full instead of full raw message#6263mergify[bot] merged 4 commits intolibp2p:masterfrom
Conversation
014e74c to
d55a35e
Compare
jxs
left a comment
There was a problem hiding this comment.
Hi, thanks for this, but this is un-required, we can just manually impl Display for RpcOut with just the message type and message id/topic hash
Makes sense, I will make the required changes |
|
Hi have been needing this on a separate feature, this is what I'd suggest: impl std::fmt::Display for RpcOut {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
RpcOut::Publish { message_id, .. } => write!(f, "Rpc Out Publish id: {}", message_id),
RpcOut::Forward { message_id, .. } => write!(f, "Rpc Out Forward id: {}", message_id),
RpcOut::Subscribe {
topic,
requests_partial,
supports_partial,
} => write!(
f,
"Rpc Out Subscribe topic: {}, supports_partial: {}, requests_partial: {}",
topic, supports_partial, requests_partial
),
RpcOut::Unsubscribe(topic_hash) => {
write!(f, "Rpc Out Unsubscribe topic: {}", topic_hash)
}
RpcOut::Graft(graft) => write!(f, "Rpc Out Graft topic: {}", graft.topic_hash),
RpcOut::Prune(prune) => write!(f, "Rpc Out Prune topic: {}", prune.topic_hash),
RpcOut::IHave(ihave) => write!(
f,
"Rpc Out IHAVE topic: {}, message ids: {:?}",
ihave.topic_hash, ihave.message_ids
),
RpcOut::IWant(iwant) => write!(f, "Rpc Out IWANT message ids: {:?}", iwant.message_ids),
RpcOut::IDontWant(idontwant) => write!(
f,
"Rpc Out IDONTWANT message ids: {:?}",
idontwant.message_ids
),
RpcOut::Extensions(extensions) => {
write!(
f,
"Rpc Out Extensions message test_extension: {}, partial_messages: {}",
extensions.test_extension.unwrap_or_default(),
extensions.partial_messages.unwrap_or_default()
)
}
RpcOut::TestExtension => todo!(),
RpcOut::PartialMessage(partial_message) => write!(
f,
"Rpc Out Partial Message topic: {}, group id : {:?}",
partial_message.topic_hash, partial_message.group_id
),
}
}
} |
23c2523 to
f177647
Compare
|
@jxs To reduce log size and avoid printing large message data, I implemented a custom Display trait for the RpcOut enum in types.rs as directed by you. In behaviour.rs, I removed the use of the non-existent RpcLog wrapper and updated the send queue full warning to use the new Display implementation, changing the log line to print only concise RpcOut details. I also removed the RpcLog import. As a result, all relevant logs now output only minimal, relevant information, preventing large byte arrays from being written to logs, while maintaining all necessary debugging context. |
jxs
left a comment
There was a problem hiding this comment.
@jxs To reduce log size and avoid printing large message data, I implemented a custom Display trait for the RpcOut enum in types.rs as directed by you. In behaviour.rs, I removed the use of the non-existent RpcLog wrapper and updated the send queue full warning to use the new Display implementation, changing the log line to print only concise RpcOut details. I also removed the RpcLog import. As a result, all relevant logs now output only minimal, relevant information, preventing large byte arrays from being written to logs, while maintaining all necessary debugging context.
ok sorry I was just updating the PR as we can simplify further, we can impl a custom Debug for RawMessage where we just count the bytes in the message data
impl std::fmt::Debug for RawMessage {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("RawMessage")
.field("source", &self.source)
.field("data length", &self.data.len())
.field("sequence_number", &self.sequence_number)
.field("topic", &self.topic)
.field("signature", &self.signature)
.field("key", &self.key)
.field("validated", &self.validated)
.finish()
}
}thanks!
|
If I am not wrong, the debug approach needs to be implemented right (as done before)? @jxs |
you just need to remove the |
9116a04 to
46b0d19
Compare
|
@jxs does it look good now?? |
jxs
left a comment
There was a problem hiding this comment.
looks good, only two suggestions left.
Can you also add a CHANGELOG.md entry please?
thanks!
|
done as requested @jxs |
Merge Queue StatusRule:
This pull request spent 5 seconds in the queue, with no time running CI. Required conditions to merge
|
|
thank you for your guidance on this @jxs |
Description
Fixes #6252
Reduce log size for "Send Queue full" warnings by avoiding printing large message data.
Previously, when the send queue was full, the warning log would print the entire
RpcOutusing itsDebugimplementation, which included the completeRawMessagewith potentially massive byte arrays (data,signature,keyfields). In high-traffic scenarios, this resulted in extremely large log files and unnecessary disk I/O.This PR introduces a
RpcLogwrapper struct with a customDebugimplementation that:PublishandForwardvariants: logs only the essential debugging information (message_id,topic,source,sequence_number,validated) while omitting the large byte arrays (data,signature,key, andtimeout)Subscribe,Unsubscribe,Graft,Prune,IHave,IWant,IDontWant): logs normally as they don't contain large dataThis change significantly reduces log file sizes in high-load scenarios while maintaining all useful debugging information for troubleshooting.
Notes & open questions
The
timeoutfield (aDelaytype) is also omitted from the log output forPublishandForwardvariants. While this field could be useful for debugging, theDelaytype doesn't have a human-readable debug representation that shows the deadline, so including it wouldn't add much value to the logs.All existing tests pass with these changes.
Change checklist