tracing

tracing copied to clipboard

Feature Request

Crates

tracing = "0.1.13" tracing-subscriber = "0.2.3"

Motivation

To use Tracing in web frontend in addition to web backend.

Proposal

Support the build targets wasm32-unknown-unknown and wasm32-wasi.

Alternatives

Using a different tracing package for the frontend would introduce unnecessary complexity.

I believe tracing and tracing-core should compile on these platforms with the std feature disabled (e.g. default-features = false in Cargo.toml). I believe the stdlib's thread_local macro, which is used when the std feature is enabled, is the only feature that tracing and tracing-core use that doesn't exist on wasm targets?

tracing-subscriber is likely going to be a bit more difficult to compile for wasm targets, since it contains machinery for actually logging to stdout. However, it should be possible to use the tracing instrumentation API with an alternative subscriber implementation designed for logging in the browser. Then, the same facade can be used everywhere, and the frontend and backend executables can configure different subscribers. At the very least, this should work as a temporary solution until we can make tracing-subscriber wasm-compatible.

Finally, if we want to officially support these targets, we'd need a CI job that ensures tracing can be cross-compiled for wasm32-unknown-unknown and wasm32-wasi.

If someone who's more familiar with wasm than I am is interested in helping out with this, please let me know — I'm happy to provide guidance on the tracing side!

I believe the stdlib's thread_local macro, which is used when the std feature is enabled, is the only feature that tracing and tracing-core use that doesn't exist on wasm targets?

I don't know about other features that might be an issue but thread_local! is supported for wasm targets. Here's an example that uses it from the electron-sys crate.

If someone who's more familiar with wasm than I am is interested in helping out with this, please let me know — I'm happy to provide guidance on the tracing side!

This is something I would be interested in helping with but realistically it might be awhile (couple weeks or so) before I could do much.

I believe the stdlib's thread_local macro, which is used when the std feature is enabled, is the only feature that tracing and tracing-core use that doesn't exist on wasm targets?

I don't know about other features that might be an issue but thread_local! is supported for wasm targets. Here's an example that uses it from the electron-sys crate.

Oh, cool — I didn't know that! In that case, I think that tracing and tracing-core should probably work out of the box, even with the std feature set. To officially support wasm targets for those crates, all we'd need to do is start cross-compiling on CI.

I did a quick test, and it looks like every crate in the repo builds fine for wasm32-wasi and wasm32-unknown-unknown, which was kind of a pleasant surprise.

We should probably be checking this on CI before we say that wasm targets are officially supported, though.

Hey @hawkw ,

I'd be happy to take a look at adding this to CI if that's the next step? I assume that's just modifying the github actions file to also start targeting' wasm32-unknown-unkwnown, and wasm32-wasi, and running the same steps, is that correct?

@SecurityInsanity Thanks, that would be great! I think just adding the targets to GitHub Actions is a good first step, but there might be more to it than that?

I'm not sure what the Rust testing story for wasm targets is like. My guess (though I'm not too familiar with the wasm ecosystem) is that we can probably build the test executables for wasm targets, but we might need some wasm runtime in order to actually run the tests --- does anyone know if this is the case?

I also suspect that at least some of the tests may not pass on wasm, since they rely on things like thread::spawn. We might need to add conditional compilation attributes to disable some tests that use functionality not relevant to wasm...

Looking at it for another crate, that seems to be the case yeah. Building is easy, executing the tests is currently not the best story: https://github.com/rustwasm/team/issues/173 (seems to be wasm-bindgen-test-runner is the accepted solution?) I'll take a look into this! See if we can get some tests passing.

Great, thank you! Do let me know if you have any questions about stuff on the tracing side --- I'm happy to help out, and excited to get wasm supported officially!

So I've started working on this, and most things truly "just work" with wasm-bindgen-test :tada: ! The two things that don't work that I've noticed in the crates I've ported locally are:

• The few tests that use: thread::spawn() don't work since wasm doesn't have a concept of threads (yet).
• Some formatting tests that try to get the current time: https://github.com/rust-lang/rust/issues/48564 , I feel most of these tests can be rewritten to not include the time in log statements when running in wasm with minimal effort.

Unfortunately there is one snag, I'm particularly not too fond of, and would like an idea of what would be the easiest for the project long term. So testing currently requires tagging tests with: #[wasm_bindgen_test].

So for example:

#[test]
fn default_name_test() {
    let (subscriber, handle) = subscriber::mock()
        .new_span(span::mock().named("default_name"))
        .enter(span::mock().named("default_name"))
        .exit(span::mock().named("default_name"))
        .done()
        .run_with_handle();

    with_default(subscriber, || {
        default_name();
    });

    handle.assert_finished();
}

Becomes:

#[test]
#[wasm_bindgen_test]
fn default_name_test() {
    let (subscriber, handle) = subscriber::mock()
        .new_span(span::mock().named("default_name"))
        .enter(span::mock().named("default_name"))
        .exit(span::mock().named("default_name"))
        .done()
        .run_with_handle();

    with_default(subscriber, || {
        default_name();
    });

    handle.assert_finished();
}

In this case testing for wasm is "opt in". There is currently no way to just have: #[test] macro work. AFAICT anything we do here will result in a giant PR that most people will need to rebase against to start adding: #[wasm_bindgen_test] to tests as well except in the very few cases where that test isn't compatible with wasm. Not to mention tests going forward have to be tagged like this which adds overhead for contributors.

TBH I can't think of a solution that looks really good here, adding a custom test macro that by default runs test && wasm-bindgen-test, and force everyone to rebase? Introduce a linter where new tests added that are tagged test, without wasm_bindgen_test have warnings thrown, and I do it in a wave of PRs that minimize rebases? Just introduce wasm_bindgen_test for the "most important" crates to start out, and do them bit by bit, so hopefully it minimizes pain? Something else?

I'd appreciate some insight as what to would be easier for y'all, I'd be happy to really take any solution, just wanna make sure it's the easiest for everyone involved. Thanks!

Hmm...in order to include the wasm_bindgen_test attribute, is there also an external dependency that's required?

I think just making the change crate-by-crate is probably the best approach, but I'm not sure. It would be good to get input from other contributors on that.

Why don't we start with a PR that just builds the crates for wasm targets on CI< and then add support for actually testing them in a follow-up?

Hmm...in order to include the wasm_bindgen_test attribute, is there also an external dependency that's required?

Yeah, there's a dev-dependency on wasm-bindgen-test crate. This includes both the underlying runner, and the test glue.

Why don't we start with a PR that just builds the crates for wasm targets on CI< and then add support for actually testing them in a follow-up?

Yeah, we can do that. I'll give it a bit to give others a chance to chime in, but will prepare it on my side 😄 .

Hmm...in order to include the wasm_bindgen_test attribute, is there also an external dependency that's required?

Yeah, there's a dev-dependency on wasm-bindgen-test crate. This includes both the underlying runner, and the test glue.

Okay, that sounds fine. Let's make sure that this dependency is in

[target.'cfg(target_arch = "wasm32)'.dev-dependencies]

and similar, so that it doesn't need to be downloaded when running the test for other targets?

Thanks for working on this!

Yep, I've got that locally to ensure it's good!

:wave: Hi @SecurityInsanity, any updates on this? I've noticed that a handful of subscriber implementations for emitting diagnostics from tracing to various browser APIs are starting to show up, like tracing-wasm and wgpu-subscriber, so it would be nice to be actually building and testing tracing for wasm targets on CI.

Let me know if there are any blockers?

Ah sorry I just forgot to come back to this! I can push up the building Pr later today as I assume most people have had a chance to respond back by now. 😄

Great! No real rush, I just wanted to see where you were at and if you were stuck on anything. Thanks again!

Just a note that:

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]

is not enough, since wasm_bindgen_test does not support WASI.

For WASI (wasm32-wasi target), there's no need for an additional attribute. Just the normal test.

So it should be:

#[cfg_attr(all(target_arch = "wasm32", not(target_os = "wasi")), wasm_bindgen_test::wasm_bindgen_test)]

I don't know if this is still a desired feature, but to make tracing-subscriber at least work in the browser with external subscribers (events work, spans don't), e.g. tracing-web, I made #2758.

Though I'm happy to go further and actually add compatibility to tracing_subscriber::fmt::Layer.