Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #10
Mossaka opened this issue May 6, 2024 · 1 comment
Closed
Tracked by #10

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

Mossaka opened this issue May 6, 2024 · 1 comment

Comments

@Mossaka
Copy link
Member

Mossaka commented May 6, 2024

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
}
  1. Either the shim implementation or shim crate parses the -trace flag and set the context to the tracing by
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
Copy link
Member Author

Mossaka commented May 20, 2024

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.

@Mossaka Mossaka closed this as completed May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant