WASM (wasm32-unknown-unknown) support?

[dotansimha]

It seems like minitrace at the moment is using minstant (https://github.com/tikv/minitrace-rust/blob/master/minitrace/Cargo.toml#L21), and this crate cannot be compiled to WASM. It fails with an error:

error[E0425]: cannot find function `current_cycle` in module `coarse_now`
  --> /.../minstant-0.1.4/src/lib.rs:72:21
   |
72 |         coarse_now::current_cycle()
   |                     ^^^^^^^^^^^^^ not fo

Maybe it's possible to migrate to instant (https://crates.io/crates/instant)?

Also, I noticed that some pieces of the core are using threads, maybe it's possible to provide an external processor to the runtime?

My current use-case uses WASM to build a binary to a CloudFlare Worker (fetch event handler). Ideally, the tracing solution for that runtime would need to collect events during the execution pipeline, store the events in memory, and then flush it with a special handler (wait_until, see https://developers.cloudflare.com/workers/runtime-apis/handlers/fetch/#contextwaituntil).

Using a processor instead of just passing the batching config to set_reporter could look like that:

trait SpanProcessor {
  fn process(&mut self, );
  fn flush(&mut self);
}

// replaces the existing `Config`
struct BatchSpanProcessor {
    pub(crate) max_spans_per_trace: Option<usize>,
    pub(crate) batch_report_interval: Duration,
    pub(crate) batch_report_max_spans: Option<usize>,
}

impl SpanProcessor for BatchSpanProcessor { ... }

// This one can just collect spans until the user decides to flush
struct InMemorySpanProcessor;

impl SpanProcessor for InMemorySpanProcessor { ... }

// ... and this becomes:
pub fn set_reporter(reporter: impl Reporter, impl SpanProcessor) {

(opentelemetry-rust has a similar idea called SpanProcessor)

[andylokandy]

It seems like minitrace at the moment is using minstant (https://github.com/tikv/minitrace-rust/blob/master/minitrace/Cargo.toml#L21), and this crate cannot be compiled to WASM. It fails with an error:

minstant is designed to work on WASM. Seems like a bug! @zhongzc

[dotansimha]

It seems like minitrace at the moment is using minstant (master/minitrace/Cargo.toml#L21), and this crate cannot be compiled to WASM. It fails with an error:

minstant is designed to work on WASM. Seems like a bug! @zhongzc

Yeah, that seems to be a bug 🐛

@andylokandy regarding the option to implement some kind of a Span processor, does this sound like a viable solution to you? I can start a PR :)

[andylokandy]

The collector is very complex. I don't believe it'll be a good idea to make it public customizable. You may see https://github.com/tikv/minitrace-rust/blob/master/minitrace/src/collector/global_collector.rs

[dotansimha]

The collector is very complex. I don't believe it'll be a good idea to make it public customizable. You may see master/minitrace/src/collector/global_collector.rs

Yeah, that's what I just figured. We have a completely separate entry point for the WASM runtime, so I think ideally, I can create a custom collector and avoid using the global one (using the library without enabled and implement some kind of collector fitted to the runtime of the WASM env).

I'm keeping open because I still think we need to get the minstant issue fixed, as it's a blocker for compilation of minitrace.

[dotansimha]

@andylokandy I noticed the complexity you mentioned. I was wondering if it makes sense to separate the GlobalCollector and allow it to provide it to the global init method? something like set_reporter(GlobalCollect, reporter, config) (this call will also trigger the .start(), instead of the global call we have now).

This way I thin I can implement different Collector my project a that matches my needs 🤔 (collect and flush manually)

[andylokandy]

If only manual flushing is considered, there is a workaround: you could set batch_report_interval to an insantly large value to avoid auto reporting, and then call minitrace::flush manually.

[zhongzc]

minstant is designed to work on WASM. Seems like a bug! @zhongzc

Let me take a look.

[dotansimha]

If only manual flushing is considered, there is a workaround: you could set batch_report_interval to an insantly large value to avoid auto reporting, and then call minitrace::flush manually.

Yeah that's one option, but the main thing is the implementation of the current collector. It's using threads and I think it's currently not supported (it passes compilation, but fails at runtime as far as I remember).

Also, I noticed that minitrace itself also fails when trying to compile to WASM with cargo build --target wasm32-unknown-unknown:

error: the wasm*-unknown-unknown targets are not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> /Users/dotansimha/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.12/src/lib.rs:291:9
    |
291 | /         compile_error!("the wasm*-unknown-unknown targets are not supported by \
292 | |                         default, you may need to enable the \"js\" feature. \
293 | |                         For more information see: \
294 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |________________________________________________________________________^

   Compiling parking_lot_core v0.9.9
error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /Users/dotansimha/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.12/src/lib.rs:347:9
    |
347 |         imp::getrandom_inner(dest)?;
    |         ^^^ use of undeclared crate or module `imp`

For more information about this error, try `rustc --explain E0433`.

So it's not only the compilation error, we might also need alternative runtime? The opentelemetry-rust project is allowing developers to pass async runtime and implement custom SpanCollector, to achieve full support for all envs.

[andylokandy]

Well. If threads are not available, then minitrace is hardly useless, because we heavily use optimized thread synchronization technique internally to achieve its performance.

[dotansimha]

Well. If threads are not available, then minitrace is hardly useless, because we heavily use optimized thread synchronization technique internally to achieve its performance.

Yeah, that makes perfect sense when compiling and running on a binary with a long-living session.
In our use case, we are building WASM for CloudFlare Worker, where every incoming request has a short-living session. So tracing should start when a request is initiated, and flush the traces when request handling is done.

We can't really use threads, so we want to collect spans in memory, and flush them at the end of the request.

[dotansimha]

Updated my lockfile and seems like the fix in minstant worked! Thanks @zhongzc ! 🎉

I noticed that some other packages are not fully supported:

error[E0433]: failed to resolve: could not find `blocking` in `reqwest`
  --> /Users/dotansimha/.cargo/registry/src/index.crates.io-6f17d22bba15001f/minitrace-datadog-0.6.2/src/lib.rs:73:31
   |
73 |         let client = reqwest::blocking::Client::new();
   |                               ^^^^^^^^ could not find `blocking` in `reqwest`
   |

[andylokandy]

Fixed by tikv/minstant#32. I'm going to move non-related discussion to a new issue.