per-transaction trace log specification#447
Conversation
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
4c9a7cc to
c572397
Compare
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
|
@julio4 , note: I shifted the log level from info to debug, because even not provinding So now at debug level, any of the following options would generate these logs |
@yashvardhan-kukreja Did you test on dev or release build? In release builds trace are not compiled. |
Yep @julio4, as per my knowledge this is a release build (given I am building it directly from the dockerfile with no flag/arg overrides).
Semantically speaking, I agree and kinda want to include these logs at trace level and have the user really opt-in into them via tx_trace=trace RUST_LOG setting.
Pardon my lack of production rust knowledge, is there a way and a negative consequence of enabling this specific trace log? Otherwise, we can just have a dedicated env var (ENABLE_TXN_TRACE_DEBUG_LOG) which when set to true will cause these logs to generate (at debug level) keeping backwards compatibility the max. Wdyt of these ideas? |
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
| match event { | ||
| FullTransactionEvent::Pending(hash) => { | ||
| debug!( | ||
| target: "tx_trace", |
There was a problem hiding this comment.
this is a redundant log with the log below, if you like to filter by target = tx_trace, could consider updating all the target=monitoring tx logs to target=tx_trace? discarded tx logs would also be helpful here I think, and reduces redundant logs
There was a problem hiding this comment.
yeah I thought of that but didn't want my changes to be intrusive. Happy to make that change if you re ok with that
There was a problem hiding this comment.
Just got done. Note: I got rid of kind tag in favor of stage
|
Wondering if adding these to a trace span would be better here |
Hmm yeah but wouldn't that require a tracing setup?
Although yeah even trace spans can be logged though they don't look very pretty |
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
We have tracing setup for span under the telemetry flag, but I can understand why you prefer logs for these which are fine as well. It should be possible to enable trace with a feature like tx-trace-logs = ["tracing/release_max_level_trace"] or changing reth features to use min-trace-logs instead of min-debug-logs (but this would requires to opt-out of reth default-features). With a env filter like RUST_LOG=debug,tx_trace=trace, the runtime overhead should be negligible. |
Thanks @julio4 . So just to confirm, with
|
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
|
@julio4 unfortunately, the min_trace_logs approach didn't work out. After building with And running it with Here's the suspected analysis by Claude
(I confirmed from reth's cargo.toml, it does seem that it has min-debug-logs as its default features) If this issue is true, then it can be a problem to manage this. The whole approach of overriding defaults of reth crate in rbuilder cargo.toml minus But I am still surprised how is this true (if it's true)? Like any crate in the dependency tree can impose restrictions on its dependent like this? Would you mind validating this? |
I thought it would be additive in choosing the less restrictive level, not the most (so trace and not debug). But because feature is defined by dependency it might be different. We can move ahead and keep it at debug level and I'll look into this, so you're not blocked on this. |
|
Thanks @julio4 , I appreciate it. I'll just double-check with you if you really want to go ahead with |
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
Yes I think env gated would be best as a temporary solution if possible! |
|
Awesome, thanks! Will proceed with it |
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
|
Hi @julio4 , just committed an env-level gate and tested it in a live-devnet. It's working as expected. |
📝 Summary
Helps trace a transaction from being included in a mempool to getting publishing it over a flashblock.
Doesn't interfere with the existing log levels of rbuilder.
Needs
RUST_LOG=tx_trace=info. Even env vars likeRUST_LOG=info,tx_trace=info(info logs for reth, but info logs for the txn trace) orRUST_LOG=debug,tx_trace=info(debug logs for reth, but info logs for the txn trace) work.For example:
(upon setting
RUST_LOG=info,tx_trace=info)With
RUST_LOG=warn,tx_trace=info, it would have looked like💡 Motivation and Context
✅ I have completed the following steps:
make lintmake test