elliottt opened PR #7366 from elliottt:trevor/request-ids-serve
to bytecodealliance:main
:
Add some missing log handling to
wasmtime serve
:
- Log the listening address and port before starting to accept incoming connections
- Track individual requests with unique ids
- Prefix stdout and stderr from the guest with the request id
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt requested wasmtime-core-reviewers for a review on PR #7366.
elliottt requested fitzgen for a review on PR #7366.
elliottt requested wasmtime-default-reviewers for a review on PR #7366.
elliottt requested alexcrichton for a review on PR #7366.
elliottt created PR review comment:
This is arbitrary.
elliottt submitted PR review.
elliottt submitted PR review.
elliottt created PR review comment:
It's not clear that there's a reasonable implementation for this here. Perhaps using
tokio::io::Stdout
instead of a mutex guardedstd::io::Stdout
would make more sense?
elliottt submitted PR review.
elliottt created PR review comment:
I think it would be reasonable to make a separate
Stderr
handle here. My thought was that since the output is already prefixed withstdout
/stderr
, using the same output stream would be okay.
elliottt edited PR #7366:
Add some missing log handling to
wasmtime serve
:
- Log the listening address and port before starting to accept incoming connections
- Track individual requests with unique ids
- Prefix stdout and stderr from the guest with the request id
Fixes #7257
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton submitted PR review:
Looks good to me! One thought about whether or not this should be an async write, but otherwise seems fine to me
alexcrichton submitted PR review:
Looks good to me! One thought about whether or not this should be an async write, but otherwise seems fine to me
alexcrichton created PR review 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.
elliottt updated PR #7366.
elliottt submitted PR review.
elliottt created PR review comment:
Great suggestion! I've implemented that, and substantially simplified the
LogStream::write
implementation :)
elliottt updated PR #7366.
elliottt merged PR #7366.
Last updated: Dec 23 2024 at 13:07 UTC