search

containerd / rust-extensions

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

Proposal: a more customizable logger #260

Open Mossaka opened 4 months ago

Mossaka commented 4 months ago

Motivation:

Runwasi shims would like to filter out crate-level logs such as the wasmtime cranelift compiler logs which contain large number of compiler related logs that pollute the containerd logs, but there isn't an option to configure the FifoLogger at the moment.

Alternatives:

One alternative is to use env_logger crate that provides powerful customization ability to filter out crates logs.

We could integrate env_logger with pipes to offer same level of functionality or use other name here to avoid confusion - users might expect same level of customization, which we don't offer.

_Originally posted by @mxpv in https://github.com/containerd/rust-extensions/pull/247#discussion_r1501257170_

Another alternative is to use tracing_log which is based on the tracing crate. I prefer this alternative more since we are discussing adding OpenTelemetry supports in Runwasi where the tracing crate is at the center of enabling OpenTelemetry OLTP collection. Tracing could also be used as a logger.

Adding tracing_log could potentially unblock the OpenTelemetry work on the SpinKube shim project as well: https://github.com/spinkube/containerd-shim-spin/issues/61

mxpv commented 4 months ago

I'd love to have RUST_LOG supported, so env_logger sounds like a good start here. Though it still needs file as a target AFAIU.

We also can introduce tracing adapter as an optional feature for ppl who want tracing functionality.

WDT?

calebschoepp commented 4 months ago

If possible I'd like us to land on using tracing. Downstream in containerd-shim-spin I'm trying to integrate some observability work and it is failing b/c we're trying to globally initialize a logger twice (way more detail in the issue here). Using tracing would prevent this.

mxpv commented 4 months ago

That's totally fair, using just tracing crate would be more convenient and easier. However also I'd want to keep rust-extensions generic and unopinionated when possible. We don't want tracing to be enforced when its not required by a user, but rather I'd hide it behind a feature toggle.

Mossaka commented 4 months ago

We don't want tracing to be enforced when its not required by a user, but rather I'd hide it behind a feature toggle.

Hey @mxpv, are you suggesting to add a conditional compilation feature in Rust for the tracing crate? If the feature is not toggled on, does it go back to env_logger?

calebschoepp commented 4 months ago

That's totally fair, using just tracing crate would be more convenient and easier. However also I'd want to keep rust-extensions generic and unopinionated when possible. We don't want tracing to be enforced when its not required by a user, but rather I'd hide it behind a feature toggle.

That's fair. Can you say more about instances where downstream libraries (what I assume you mean by user) would prefer/need env_logger rather than tracing?

From my perspective if we think tracing is more convenient and easier than I'm not sure why we would want to even keep the option of env_logger around?

Looking to learn here 👍 b/c I'm totally just parachuting into this project.