Stream: git-wasmtime

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


view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2020 at 18:04):

pchickey requested kubkon for a review on PR #2139.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2020 at 18:31):

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

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2020 at 18:01):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow submitted PR Review.

view this post on Zulip 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?

view this post on Zulip 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?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:04):

iximeow created PR Review Comment:

:heart_eyes_cat:

view this post on Zulip 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...

view this post on Zulip 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"
        );

view this post on Zulip 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?)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:15):

pchickey submitted PR Review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

pchickey submitted PR Review.

view this post on Zulip 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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

iximeow submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:16):

iximeow created PR Review Comment:

:pray: thank you wiggle

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:23):

iximeow submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:23):

iximeow created PR Review Comment:

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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 18:41):

iximeow submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 25 2020 at 22:09):

pchickey merged PR #2139.


Last updated: Nov 22 2024 at 16:03 UTC