containerd / rust-extensions

Rust crates to extend containerd
https://containerd.io
Apache License 2.0
176 stars 69 forks source link

Proposal(shim): preserve tracing context in `shim::spawn` #268

Closed Mossaka closed 5 months ago

Mossaka commented 5 months ago

Problem

I am working on a feature to add opentelemetry support to runwasi shims, building on top of the shim crate from this repo. However, I was blocked from this major problem:

One major block point is there are some missing parent spans for functions like task_wait().

And later I discovered that the shim process being spawned by the parent process needs to be able to preserve the opentelemetry context (read more at Otel's Context Propogation).

Proposal

  1. Adding a conditional compilation feature on the shim crate to include tracing,opentelemetry and tracing_opentelemetry dependencies on the shim crate
  2. Adding a new flag pub trace: String to the Flags struct
  3. Before spawning the child shim process, invoke the following extract_tracing_context() function, and append the context to the -trace flag
    
    fn extract_tracing_context() -> String {
    let mut injector: HashMap<String, String> = HashMap::new();
    global::get_text_map_propagator(|propagator| {
        // retrieve the context from `tracing`
        propagator.inject_context(&Span::current().context(), &mut injector);
    });
    let trace_context = serde_json::to_string(&injector).unwrap();
    trace_context
    }
4. Either the shim implementation or `shim` crate parses the `-trace` flag and set the context to the tracing by
```rust
if !flags.trace.is_empty() {
    log::info!("Setting parent span from trace context: {}", flags.trace);
    let extractor: HashMap<String, String> =
        serde_json::from_str(&flags.trace).unwrap();
    let context =
        global::get_text_map_propagator(|propagator| propagator.extract(&extractor));
    Span::current().set_parent(context);
}

Discussions

Although runwasi shims use tracing crate to collect spans and emit to OpenTelemetry-compitable tracing systems for processing, I don't think it's a good idea to be opinionated about what crates to be used for collecting Otel tracings. Thus, maybe we could add functions to the Shim trait to get / set the current span context. By abstracting the context to the shim implementation level, the shim crate could stay un-opinionated about tracing collectors.

Implementing this feature will foster better observability within containerd shims. I would like to discuss an approach that ensures compatibility with various tracing solutions while maintaining flexibility and modularity in the shim architecture.

Mossaka commented 5 months ago

Will close this issue since we are going to use env var (e.g. TRACECONTEXT) to pass around the tracing context in the shim implementation itself.