opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

[WIP] Remove Send bounds on wasm32

Open OliverEvans96 opened this issue 1 year ago • 1 comments

Fixes #910

Also related: #1235

Changes

Attempting to compile to WASM produces a number of errors regarding Send bounds, as reported in #1235. This is because wasm_bindgen::JsValue is not Send, since it's intended to run in a single-threaded JS environment.

This issue was successfully addressed in the rspotify crate in https://github.com/ramsayleung/rspotify/pull/458, although I think opentelemetry-rust requires a somewhat more complex solution.

I've managed to fix these errors (see other WASM errors in #1472 and #2047) by conditionally removing Send bounds for target_arch="wasm32".

I've done this in three ways:

  • conditionally change #[async_trait] to #[async_trait(?Send)] based on target_arch
  • Replace BoxFuture with MaybeSendBoxFuture, which conditionally removes Send bounds on wasm
  • Replace Arc<dyn InstrumentProvider + Send + Sync> with type ArcInstrumentProvider = Arc<dyn InstrumentProvider> on WASM and type ArcInstrumentProvider = Arc<dyn InstrumentProvider + Send + Sync> otherwise.

I suspect that this PR is not yet complete. Initially, I only made enough changes to get my application to compile, using the following feature flags:

opentelemetry = { version = "0.24.0" }
opentelemetry_sdk = { version = "0.24.1", features=["logs"] }
opentelemetry-semantic-conventions = "0.16.0"
opentelemetry-otlp = { version = "0.17.0", default-features = false, features = ["logs", "trace", "http", "http-json", "reqwest-client"] }
opentelemetry-http = { version = "0.13.0", default-features = false }

I then continued by replacing all instances of #[async_trait] that seemed relevant. But I assume that there are other crates or feature flags that would require similar changes.

I'm also happy to hear your feedback on this approach, and make changes as necessary.

Thanks, Oliver

Merge requirement checklist

  • [x] CONTRIBUTING guidelines followed
  • [ ] Unit tests added/updated (if applicable)
  • [ ] Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • [ ] Changes in public API reviewed (if applicable)

OliverEvans96 avatar Sep 02 '24 15:09 OliverEvans96

I'm also wondering whether it's really wise to assume that all wasm32 targets are single-threaded. Are there or will there be multi-threaded WASM environments??

OliverEvans96 avatar Sep 02 '24 15:09 OliverEvans96

I think we need to start tracking all issues blocking OpenTelemetry in wasm using a label, and see where we stand. I won't be surprised if OpenTelemetry Rust is completely unusable in wasm :( , but something we can make some efforts to invest into.

I

cijothomas avatar Dec 11 '24 03:12 cijothomas

Closing inactive PRs. The linked issue can be used to track the work of supporting wasm in general.

cijothomas avatar Dec 18 '24 18:12 cijothomas