Use instrument in generated call_ functions
#9263
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elliottt
requested review from
alexcrichton and
pchickey
and removed request for
a team
elliottt
commented
Sep 17, 2024
Comment on lines
+2847
to
+2861
| let instrument = if is_async && self.gen.opts.tracing { | ||
| ".instrument(span.clone())" | ||
| } else { | ||
| "" | ||
| }; | ||
| uwriteln!(self.src, ")){instrument}{await_}?;"); | ||
|
|
||
| let instrument = if is_async && self.gen.opts.tracing { | ||
| ".instrument(span)" | ||
| } else { | ||
| "" | ||
| }; | ||
| uwriteln!( | ||
| self.src, | ||
| "callee.post_return{async__}(store.as_context_mut()){await_}?;" | ||
| "callee.post_return{async__}(store.as_context_mut()){instrument}{await_}?;" |
There was a problem hiding this 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.
alexcrichton
approved these changes
Sep 17, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
elliottt
added a commit
to elliottt/wasmtime
that referenced
this pull request
Sep 17, 2024
* Add async tracing versions of the component-macro tests * Use instrument when generating async call functions with tracing * Update tests
alexcrichton
pushed a commit
that referenced
this pull request
Sep 17, 2024
* Use `instrument` in generated `call_` functions (#9263) * Add async tracing versions of the component-macro tests * Use instrument when generating async call functions with tracing * Update tests * Update release notes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following on from #9217, generate
call_*functions that usetracing::Instrumentinstead ofSpan::enter.To test this change I added an additional set of generated files that corresponds to the combination of
async: trueandtracing: true. This way we can verify that the changes in #9217 and this PR remove uses ofSpan::enterfrom the code generated by thebindgen!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, 4c1a4f0, and the final commit shows the differences to the generated code that the middle commit introduces. I believe this catches all outstanding uses of
enterfor the combination ofasync: trueandtracing: true.