Init RuntimeLogger automatically for each runtime api call
#8128
Conversation
This pr change the runtime api in such a way to always and automatically enable the `RuntimeLogger`. This enables the user to use `log` or `tracing` from inside the runtime to create log messages. As logging introduces some extra code and especially increases the size of the wasm blob. It is advised to disable all logging completely with `sp-api/disable-logging` when doing the wasm builds for the on-chain wasm runtime. Besides these changes, the pr also brings most of the logging found in frame to the same format "runtime::*".
bkchr
added
A0-please_review
|
Maybe we can do something like this to also standardize this |
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
There was a problem hiding this comment.
If I understand correctly the feature "max_level_off" propagate to all crate thus the wasm binary compiled with feature "disable-logging" compiles all the crate with "log/max_level_off", isn't it?
I'm not familiar with how log removes the logging code.
Or maybe what you mean by:
As logging introduces some extra code and especially increases the size of the wasm
blob.
Is just about the initialization of logger in wasm not the logging in pallets themself.
Another note: I think it should better to have the feature disable-logging in node-runtime as well so people can compile wasm binary with cargo build --feature disable-logging -p node-runtime (or I don't know what it the command documented to build the wasm binary)
Yes that is correct.
The logging is just a macro that is essentially wrapped in some This feature sets |
Yeah in the end that will come. However, I'm not sure if we require this for the Substrate node. In the end I want a proper tutorial that explains this to people. :) |
|
So this will need the polkadot companion for adding features to the runtimes, But otherwise looks good to me |
|
Yeah. I want to go some 2 way documentation way. For polkadot I will communicate this internal. For Substrate in general I want to have this on the dev hub |
| ] | ||
| nightly = [] |
There was a problem hiding this comment.
-
I couldn't find this in the wasm-builder: can we have a test to compare the wasm size when the feature is on or off?
-
As recommended elsewhere, I think can autogenerate the
const LOG_TARGET: ...for all pallets in the macro, instead of assuming that they all prefix with runtime. We can also generate one helper macro for logging withLOG_TARGET(what I have done in staking). Could be an easy follow up.
|
And, if we had changelogs, this def. needs to go into one as I think it breaks many downstream projects. |
For Kusama the difference is 700 bytes :D So, not that much xD
Yeah, my plan ;) |
See paritytech/substrate#8128 for more info.
* Bump Substrate to version used by Polkadot (`5f056830`) * Use `log` crate for runtime logging See paritytech/substrate#8128 for more info. * Stop using return value from `execute_block` * Update test weight
|
Are the released runtimes compiled with |
|
They are disabled in the Polkadot/Kusama runtimes |
This pr change the runtime api in such a way to always and automatically
enable the
RuntimeLogger. This enables the user to uselogortracingfrom inside the runtime to create log messages. As loggingintroduces some extra code and especially increases the size of the wasm
blob. It is advised to disable all logging completely with
sp-api/disable-loggingwhen doing the wasm builds for the on-chainwasm runtime.
Besides these changes, the pr also brings most of the logging found in
frame to the same format "runtime::*".
Fixes: #7927
Should fix: #8107
polkadot companion: paritytech/polkadot#2522