Stream: git-wasmtime

Topic: wasmtime / PR #7366 Add logging and request id tracking t...

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:39):

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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:39):

elliottt requested wasmtime-core-reviewers for a review on PR #7366.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:39):

elliottt requested fitzgen for a review on PR #7366.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:39):

elliottt requested wasmtime-default-reviewers for a review on PR #7366.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:40):

elliottt requested alexcrichton for a review on PR #7366.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:40):

elliottt created PR review comment:

This is arbitrary.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:40):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:41):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:41):

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 guarded std::io::Stdout would make more sense?

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:44):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:44):

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 with stdout/stderr, using the same output stream would be okay.

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:47):

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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:59):

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

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:59):

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

Wasmtime GitHub notifications bot (Oct 25 2023 at 18:59):

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.

Wasmtime GitHub notifications bot (Oct 25 2023 at 22:29):

elliottt updated PR #7366.

Wasmtime GitHub notifications bot (Oct 25 2023 at 22:31):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Oct 25 2023 at 22:31):

elliottt created PR review comment:

Great suggestion! I've implemented that, and substantially simplified the LogStream::write implementation :)

Wasmtime GitHub notifications bot (Oct 25 2023 at 22:32):

elliottt updated PR #7366.

Wasmtime GitHub notifications bot (Oct 26 2023 at 00:11):

elliottt merged PR #7366.

Last updated: Nov 22 2024 at 16:03 UTC