Stream: git-wasmtime

Topic: wasmtime / PR #2139 wasi-common: switch all logs from `lo...

Wasmtime GitHub notifications bot (Aug 18 2020 at 17:39):

pchickey opened PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 18 2020 at 17:46):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 18 2020 at 18:04):

pchickey requested kubkon for a review on PR #2139.

Wasmtime GitHub notifications bot (Aug 18 2020 at 18:31):

pchickey requested kubkon and sunfishcode for a review on PR #2139.

Wasmtime GitHub notifications bot (Aug 18 2020 at 22:01):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 18 2020 at 23:03):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 24 2020 at 18:01):

pchickey requested kubkon, sunfishcode and iximeow for a review on PR #2139.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

while it was ad-hoc, the | ... thing was kind of handy to know what was happening in a single hostcall. might be good to replace the idea by having spans for each hostcall?

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

assuming this is the case but just want to be sure: the lines in snapshot_0 don't have the cool fd = tracing::field::display(fd) treatment because they're slated to be deleted and polyfilled, right?

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

:heart_eyes_cat:

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

        tracing::debug!(
            host_fd = tracing::field::display(file.as_raw_fd()),
            "Host fd is a block device"
        );

I don't recall off the top of my head where else we'd want to draw the distinction between host/wasi fds, so this probably extends elsewhere...

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

        tracing::debug!(
            poll_fd = tracing::field::display(poll_fd),
            poll_event = tracing::field::debug(fd_event), // or display? are either impl'd?
            "poll_oneoff handle_fd_event"
        );

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

        tracing::debug!(
            old_path = tracing::field::display(&old_path),
            "Following symlinks"
        );

(I'm less inclined to suggest that concatenate should get the old_path = ... treatment because I wonder if that's actually useful logging?)

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:15):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:15):

pchickey created PR Review Comment:

Correct. Just laziness. Hopefully those lines are all deleted by labor day.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

pchickey created PR Review Comment:

There are spans in each hostcall now thanks to wiggle! Spans rule

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

iximeow created PR Review Comment:

:pray: thank you wiggle

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:17):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:23):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:23):

iximeow created PR Review Comment:

oh this should probably be old_path.display(), sorry

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:32):

pchickey updated PR #2139 from pch/wasi_common_tracing to main:

tracing is already the dep that wiggle uses, so this is just pruning the log dep.

For users that still want to consume the logs with a log backend, the trace_log feature
on this crate still does that trick.

I used tracing structured arguments wherever I could, but I skipped over
it in all of the snapshot_0 code, because I'm going to delete that code
and replace it with wiggle-based stuff real soon.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 25 2020 at 18:41):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Aug 25 2020 at 22:09):

pchickey merged PR #2139.

Last updated: Dec 23 2024 at 12:05 UTC