Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

k6: group special async function treatment #2863

Closed
wants to merge 3 commits into from
Closed

Conversation

mstoykov
Copy link
Contributor

A group provided with an async function will now:

  1. Treat the whole time it take for the async function promise to finisher the duration of the group.
  2. It will also use goja's AsyncContextTracker to make it so that the code using await within the group will still continue to be tagged with the group after await returns.
  3. Instead of directly calling the async function it schedules it to be called the next time a promise job will work.

The current AsyncContextTracker is only used for this and as such is directly changed in the k6 module. In the future there likely will be API so that multiple modules can use it simultaneously, but that seems way too involved to be included in this change and also currently only group needs this.

fixes #2848 #2728

@mstoykov mstoykov added this to the v0.43.0 milestone Jan 19, 2023
A group provided with an async function will now:
1. Treat the whole time it take for the async function promise to finisher
   the duration of the group.
2. It will also use goja's AsyncContextTracker to make it so that the code
   using `await` within the group will still continue to be tagged with the
   group after `await` returns.
3. Instead of directly calling the async function it schedules it to be
   called the next time a promise job will work.

The current AsyncContextTracker is only used for this and as such is
directly changed in the `k6` module. In the future there likely will be
API so that multiple modules can use it simultaneously, but that seems
way too involved to be included in this change and also currently only
`group` needs this.

fixes #2848 #2728
@mstoykov mstoykov linked an issue Jan 19, 2023 that may be closed by this pull request
@mstoykov
Copy link
Contributor Author

Moving this to a draft in favor of #2875

@mstoykov mstoykov marked this pull request as draft January 25, 2023 15:15
@mstoykov mstoykov removed this from the v0.43.0 milestone Jan 27, 2023
@aakash3771
Copy link

How to get notified when this is fixed?

@mstoykov
Copy link
Contributor Author

For more information you likely should follow #2728 as this PR at this time is very unlikely to be merged as is.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 86.56716% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 76.15%. Comparing base (0b563d4) to head (8101130).
Report is 594 commits behind head on master.

Files Patch % Lines
js/modules/k6/k6.go 83.33% 4 Missing and 1 partial ⚠️
js/modules/k6/asyncGroup.go 89.18% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2863      +/-   ##
==========================================
- Coverage   76.15%   76.15%   -0.01%     
==========================================
  Files         217      218       +1     
  Lines       16604    16669      +65     
==========================================
+ Hits        12645    12694      +49     
- Misses       3185     3196      +11     
- Partials      774      779       +5     
Flag Coverage Δ
ubuntu 75.97% <86.56%> (-0.01%) ⬇️
windows 75.85% <86.56%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mstoykov
Copy link
Contributor Author

I am closing this PR as it is really unlikely we will go down this route

@mstoykov mstoykov closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change how group() calls async functions group doesn't work with async calls well
3 participants