Stream: git-wasmtime

Topic: wasmtime / Issue #1796 wiggle: support logging via `traci...

Wasmtime GitHub notifications bot (May 31 2020 at 02:18):

pchickey commented on Issue #1796:

Currently a very sketchy WIP. I made an example inside test-helpers (because it was most convenient to reuse the test-helpers HostMemory and WasiCtx) that demonstrates that tracing integration works at all, but I'm not sure where the best place to stick that in the long run is. In order to provide an example we have to take on tracing and tracing_subscriber as a dev-dependency, which are not especially heavyweight.

Wasmtime GitHub notifications bot (May 31 2020 at 02:20):

pchickey edited a comment on Issue #1796:

Currently a very sketchy WIP, just wanted to show I could factor it out to provide either logging setup as a config option to the macro. Need to still finish factoring out all the log statements into helper funcs, and also setup tracing spans per function.

I made an example inside test-helpers (because it was most convenient to reuse the test-helpers HostMemory and WasiCtx) that demonstrates that tracing integration works at all, but I'm not sure where the best place to stick that in the long run is. In order to provide an example we have to take on tracing and tracing_subscriber as a dev-dependency, which are not especially heavyweight.

Wasmtime GitHub notifications bot (May 31 2020 at 02:39):

github-actions[bot] commented on Issue #1796:

Subscribe to Label Action

cc @kubkon

<details>
This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

• kubkon: wasi

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jun 01 2020 at 07:40):

kubkon commented on Issue #1796:

This looks interesting, thanks Pat! I haven’t looked over the code properly yet, however, one observation about removing log altogether is that this will probably force every user of libwasmtime to rely on tracing crate which i don’t think is a good idea. I think if I was a developer making use of the library, I’d rather have the choice or at the very least expect log support by default which seems to be the “industry” standard.

Wasmtime GitHub notifications bot (Jun 01 2020 at 15:50):

alexcrichton commented on Issue #1796:

This seems reasonable to me to add as a feature, but I'm also hesitant about deleting log. It looks like tracing is a pretty hefty dependency, and I'm not sure all users of wasmtime will want to take that on.

(stable) support for structured fields

FWIW this seems like a good reason to poke upstream about the support here, I know it's been in limbo for awhile but upstream may have thoughts about stabilizing.

tracing statements that are disabled (marked as uninteresting, essentially) by the backend are very low overhead - its one atomic load to determine whether a hook is disabled.

I know this is also true for log, so I don't think there should be any speed difference.

Wasmtime GitHub notifications bot (Jun 01 2020 at 18:38):

pchickey commented on Issue #1796:

Cool, thanks for the input and the context. I'll move forward with this patch which will make the logging choice an either/or log/tracing. I'm not going to pull on the thread of structured fields in log for lack of time to do so, but I'd welcome any help with that.

Wasmtime GitHub notifications bot (Jun 01 2020 at 23:49):

pchickey commented on Issue #1796:

I thought about this more and got some feedback from @hawkw and @fitzgen -

• taking a dep on tracing implies tracing-core and tracing-attributes, which in turn pull in lazy_static, cfg-if, and the standard proc macro crates (proc-macro2, syn, quote). wasi-common already has all of those deps transitively, so, this is not so heavyweight. tracing-subscriber is the heavyweight crate, just like the backend crates in the log ecosystem.
• when the log feature is enabled for tracing, the log system is used as the backend for the tracing macros. This allows the existing users who just want log in their system to use wasi-common just as they did before. Since the tracing dep isn't very heavy, this seems preferable than having a switch throughout wiggle codegen to emit one kind of log or another
• If we want to completely disable any code footprint from tracing or log, Wiggle can emit macros that point to wiggle::tracing_impl::(macro), and use a feature flag in wiggle/src/tracing_impl.rs to substitute between pub use tracing::* and a module that defines no-op versions of each macro. This is suitable for use cases that are very sensitive about vendoring in additional deps, or where the code size of always-runtime-disabled macros is an issue.

Wasmtime GitHub notifications bot (Jun 02 2020 at 22:59):

pchickey commented on Issue #1796:

Unfortunately, we can't switch between the tracing macros and no-op versions, while keeping that api re-exported inside a module scope like wiggle::tracing (and its a fairly big API, so i think we really need to), without defining a whole other tracing_noop crate, because much of the API is in terms of macro_rules which can only be exported at the top level of a crate. So, Rust visibility rules got us here.

At some point, the cranelift project will need to implement a tracing_noop crate in order to turn off tracing deps for use inside spidermonkey. Until then, we'll do a mandatory dep on tracing, with just a feature to set it in log mode.

Wasmtime GitHub notifications bot (Jun 03 2020 at 01:24):

pchickey commented on Issue #1796:

The failures in wasi-common are due to a fixed bug in tracing that will be part of the 0.1.15 release, which will happen shortly since we need it to merge this.

Wasmtime GitHub notifications bot (Jun 03 2020 at 01:46):

hawkw commented on Issue #1796:

The failures in wasi-common are due to a fixed bug in tracing that will be part of the 0.1.15 release, which will happen shortly since we need it to merge this.

Hot off the presses: https://crates.io/crates/tracing/0.1.15

Sorry for any inconvenience! :)

Wasmtime GitHub notifications bot (Jun 03 2020 at 17:56):

pchickey commented on Issue #1796:

I believe this is ready for review.

The example program at crates/wiggle/test-helpers/examples/tracing.rs demonstrates the use of tracing-subscriber and log as backends to the tracing statements emitted by wiggle. If you set the RUST_LOG environment variable, itll use env_logger to output, otherwise it'll use tracing-subscriber.

Last updated: Nov 22 2024 at 17:03 UTC