-
Notifications
You must be signed in to change notification settings - Fork 324
quic_trace: full client support #6352
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
Conversation
a34728e
to
5cae487
Compare
@@ -355,11 +348,11 @@ fd_quic_trace_1rtt( fd_quic_trace_ctx_t * trace_ctx, | |||
"\"flow\": \"%s\", " | |||
"\"trace_time\": \"%s\", " | |||
"\"src_ip_addr\": \"" FD_IP4_ADDR_FMT "\", " | |||
"\"src_udp_port\": %u, " | |||
"\"src_udp_port\": \"%u\", " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are numbers. Why wrap them in quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to make this lowly err msg consistent with the presumably 'correct' version in fd_quic_pretty_print_quic_pkt
do you think I should change them both to leave off the quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize another was different. This is fine. I might take a look to make everything follow the same policy. But for now, at least this makes them consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a couple of comments
5cae487
to
cc894a3
Compare
This PR extends quic_trace for the full client (particularly send_tile). It simplifies the rebasing logic and fixes various quic_trace bugs.