-
Notifications
You must be signed in to change notification settings - Fork 177
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
feat(logging): add a tracing span
per JSON-RPC call
#722
Conversation
tracing span
per calltracing span
per call
27391f2
to
5fbdcbf
Compare
tracing span
per calltracing span
per call
|
||
/// Find the next char boundary to truncate at. | ||
fn truncate_at_char_boundary(s: &str, max: u32) -> &str { | ||
match s.char_indices().nth(max as usize) { |
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.
This is quite an expensive thing to do relatively speaking, but I guess we only do it for TRACE level logs which are only used in exceptional cases!
A quick optimisation might be to check s.len() < max
first, and if that is true, there is def no need to truncate?
|
||
/// Find the next char boundary to truncate at. | ||
fn truncate_at_char_boundary(s: &str, max: u32) -> &str { | ||
match s.char_indices().nth(max as usize) { |
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.
This is quite an expensive thing to do relatively speaking, but I guess we only do it for TRACE level logs which are only used in exceptional cases!
A quick optimisation might be to check s.len() < max
first, and if that is true, there is def no need to truncate?
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.
Assuming you still want to get this merged, looks good to me, and the tracing stuff sounds very useful!
serde_json::from_slice::<serde_json::Value>(&raw) | ||
); | ||
return Err(Error::Custom("Unparsable response".into())); | ||
let json = serde_json::from_slice::<serde_json::Value>(&raw); |
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.
the connection is killed here, we can propagate the entire response in the error then.
tracing span
per calltracing span
per JSON-RPC call
tracing span
per JSON-RPC calltracing span
per JSON-RPC call
To include the bugfix for `tracing::enabled!` when `log` is enabled. Follow up on #722
To include the bugfix for `tracing::enabled!` when `log` is enabled. Follow up on #722
/// | ||
/// To enable this you need to call `RpcTracing::method_call("some_method").span().enable()`. | ||
pub fn method_call(method: &str) -> Self { | ||
Self(tracing::span!(tracing::Level::DEBUG, "method_call", %method)) |
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.
^
Essentially this PR downgrades to only print request and response bodies when
trace!
is enabled.In addition I have added a tracing span per call so it's possible to just trace specific method calls which could be quite useful I reckon.
For batches it's possible to just
log/trace
by the targetbatch
because the span is calledbatch
so not possible to trace the individual calls inside it.The main motivation for tracing spans is for the servers where this can be super useful.
Example WS
Example HTTP