Add logging and request id tracking to wasmtime serve#7366
Add logging and request id tracking to wasmtime serve#7366elliottt merged 2 commits intobytecodealliance:mainfrom
wasmtime serve#7366Conversation
elliottt
requested review from
alexcrichton and
fitzgen
and removed request for
a team
| } | ||
|
|
||
| fn check_write(&mut self) -> StreamResult<usize> { | ||
| Ok(1024 * 1024) |
|
|
||
| #[async_trait::async_trait] | ||
| impl preview2::Subscribe for LogStream { | ||
| async fn ready(&mut self) {} |
There was a problem hiding this comment.
It's not clear that there's a reasonable implementation for this here. Perhaps using tokio::io::Stdout instead of a mutex guarded std::io::Stdout would make more sense?
src/commands/serve.rs
Outdated
|
|
||
| builder.stderr(LogStream { | ||
| prefix: format!("stderr [{req_id}] :: "), | ||
| output, |
There was a problem hiding this comment.
I think it would be reasonable to make a separate Stderr handle here. My thought was that since the output is already prefixed with stdout/stderr, using the same output stream would be okay.
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Looks good to me! One thought about whether or not this should be an async write, but otherwise seems fine to me
src/commands/serve.rs
Outdated
| for line in bytes.split(|c| *c == b'\n') { | ||
| if !line.is_empty() { | ||
| msg.extend_from_slice(&self.prefix.as_bytes()); | ||
| msg.extend_from_slice(line); | ||
| msg.push(b'\n'); | ||
| } | ||
| } | ||
|
|
||
| let output = self.output.clone(); | ||
| tokio::task::spawn(async move { | ||
| use std::io::Write; | ||
| let mut output = output.lock().await; | ||
| output.write_all(&msg).expect("writing to stdout"); | ||
| }); |
There was a problem hiding this comment.
What would you think of going ahead and directly doing a blocking write to stdout/stderr here? That'd help applying a bit of backpressure (albeit not in a perfect way) if necessary and additionally would avoid the need to pass around streams much. That'd also perhaps make it a bit easier to write stdout to stdout and stderr to stderr.
There was a problem hiding this comment.
Great suggestion! I've implemented that, and substantially simplified the LogStream::write implementation :)
elliottt
force-pushed
the
trevor/request-ids-serve
branch
from
4a1b335 to
698ec0f
Compare
2 participants
Add some missing log handling to
wasmtime serve:Fixes #7257