Stream: git-wasmtime

Topic: wasmtime / PR #9263 Use `instrument` in generated `call_`...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:37):

elliottt opened PR #9263 from elliottt:trevor/instrument-call-functions to bytecodealliance:main:

Following on from #9217, generate call_* functions that use tracing::Instrument instead of Span::enter.

To test this change I added an additional set of generated files that corresponds to the combination of async: true and tracing: true. This way we can verify that the changes in #9217 and this PR remove uses of Span::enter from the code generated by the bindgen! macro.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:37):

elliottt requested alexcrichton for a review on PR #9263.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:37):

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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:38):

elliottt requested pchickey for a review on PR #9263.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:39):

elliottt edited PR #9263:

Following on from #9217, generate call_* functions that use tracing::Instrument instead of Span::enter.

To test this change I added an additional set of generated files that corresponds to the combination of async: true and tracing: true. This way we can verify that the changes in #9217 and this PR remove uses of Span::enter from the code generated by the bindgen! macro.

The additional exp files add an awful lot to this diff, and I think it would be reasonable to remove them before merging. The meat of the changes is in the middle commit, 4c1a4f0998b8f3ebca9e6873a781be69a9121a82.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 03:55):

elliottt edited PR #9263:

Following on from #9217, generate call_* functions that use tracing::Instrument instead of Span::enter.

To test this change I added an additional set of generated files that corresponds to the combination of async: true and tracing: true. This way we can verify that the changes in #9217 and this PR remove uses of Span::enter from the code generated by the bindgen! macro.

The additional exp files add an awful lot to this diff, and I think it would be reasonable to remove them before merging. The meat of the changes is in the middle commit, 4c1a4f0998b8f3ebca9e6873a781be69a9121a82, and the final commit shows the differences to the generated code that the middle commit introduces. I believe this catches all outstanding uses of enter for the combination of async: true and tracing: true.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 04:00):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 04:00):

elliottt created PR review comment:

This all feels a bit verbose, but it seemed slightly easier to read than the version where I inlined the conditional into the args to the format string. Happy to add the inlined version if that's preferred.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 04:30):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 17 2024 at 05:19):

elliottt merged PR #9263.

Last updated: Oct 23 2024 at 20:03 UTC