Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dump network traffic at *.dump:trace level #5688

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@moneromooo-monero
Copy link
Contributor

commented Jun 23, 2019

Useful for debugging

moneromooo-monero added some commits Jun 22, 2019

Properly format multiline logs
As a side effect, colouring on Windows should now work
regardless of version

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:ml branch from 1949993 to 1c2208a Jun 25, 2019

@@ -364,6 +364,7 @@ PRAGMA_WARNING_DISABLE_VS(4355)
context.m_last_recv = time(NULL);
context.m_recv_cnt += bytes_transferred;
m_ready_to_close = false;
MDUMP(buffer_.data(), bytes_transferred, epee::net_utils::print_connection_context_short(context) + " <");

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jul 4, 2019

Contributor

The logging system has to acquire a lock and then scan/search a string before it can determine that the message is going to be skipped. So this has the effect of serializing all TCP read/writes with all log messages, slowing things further than they are already are due to the debug/trace messages sprayed everywhere.

And the benefit is that we get a dump of arbitrary TCP data ... I don't see how this is a net gain. Some of the logs are going to be massive dumps.

At least drop some of the other log messages in these functions, the write function below in particular now has basically wasted trace log.

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jul 5, 2019

Author Contributor

I have a patch that does away with the lock actually.
And it's a gain when debugging.

This comment has been minimized.

Copy link
@moneromooo-monero

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jul 6, 2019

Contributor

That patch does not do away with the lock, it only skips the lock in some contexts. There are still INFO log statements that have to go through the full lock + filtering frequently since the default settings have INFO logging in some categories. Removing the lock in all scenarios requires "fixed" (enum) categories (I think), so that the set_level call would compute the levels once "up-front" instead of "once-per-log-call".

This is dumping all P2P and RPC traffic in a series of text walls, right? Do you actually find that large information dump useful? The DEBUG logs already spew so much data that I typically avoid using it. I'll look at the patch again later ...

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jul 7, 2019

Author Contributor

The patch does not bypass the lock all the time, yes. It does bypass it for trace stuff (like this one in this patch) unless you've set things up to have at least one trace active, but in that case a lock isn't going to be your problem speedwise. I do not intend to remove the lock altogether.
This is dumping all network traffic, yes. I found this useful. It is not intended to be enabled unless you know you want to check what's going on in traffic (ie, timings of what's exchanged, what is exchanged, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.