-
Notifications
You must be signed in to change notification settings - Fork 727
Expose API to set log level in embedder #2907
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
Expose API to set log level in embedder #2907
Conversation
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.
LGTM
core/iwasm/include/wasm_export.h
Outdated
| wasm_runtime_full_init(RuntimeInitArgs *init_args); | ||
|
|
||
| /** | ||
| * Set the log level. To be called after the runtime is created. |
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.
Had better change created to initialized?
d043de5 to
0022e87
Compare
core/iwasm/include/wasm_export.h
Outdated
| BH_LOG_LEVEL_ERROR = 1, | ||
| BH_LOG_LEVEL_WARNING = 2, | ||
| BH_LOG_LEVEL_DEBUG = 3, | ||
| BH_LOG_LEVEL_VERBOSE = 4 |
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.
if these are for wasm_runtime_set_log_level, it seems a bit odd to use BH_ prefix. how do you think?
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.
How about changing to WASM_LOG_LEVEL_*? Or is there other suggestion?
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.
How about changing to
WASM_LOG_LEVEL_*?
it sounds better to me.
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.
Good to me too. @eloparco, could you help update the code? Thanks.
0022e87 to
fb80982
Compare
64a4118 to
0b79345
Compare
[build_iwasm ($CLASSIC_INTERP_BUILD_OPTIONS, -DWAMR_BUILD_LIB_PTHREAD=1, macos-latest, darwin)](https://github.com/bytecodealliance/wasm-micro-runtime/actions/runs/7220553955/job/19673804322)
GitHub Actions has encountered an internal error when running your job.I'm amending the commit to restart the job. |
0b79345 to
2979b51
Compare
Follow-up on #2907. The log level is needed in the host embedder to better integrate with the embedder's logger. Allow the developer to customize his bh_log callback with `cmake -DWAMR_BH_LOG=<log_callback>`, and update sample/basic to show the usage.
Expose API `void wasm_runtime_set_log_level(log_level_t level)`.
…#3070) Follow-up on bytecodealliance#2907. The log level is needed in the host embedder to better integrate with the embedder's logger. Allow the developer to customize his bh_log callback with `cmake -DWAMR_BH_LOG=<log_callback>`, and update sample/basic to show the usage.
No description provided.