tom-pytel commented on pull request #20: URL: https://github.com/apache/skywalking-nodejs/pull/20#issuecomment-753318812
> @tom-pytel I think the tests all passed, and I run your tests locally, which seems to work well too, can you please take a look at the codes and verify the aforementioned tests if needed. Thanks > > BTW, Happy New Year !! Thx, you too. Checked on my side with my tests and is ok. I have one issue though, why did you remove `span.async()` and `config.span.resync()` in AxiosPlugin? Were you getting a problem? Because I put them back and my test still works fine. The purpose of `async()` and `resync()` is as follows (since it is probably a little different from what these function names did originally when I started with this codebase) - The list of spans is now kept as an async task local variable so that spans get the correct parent hierarchy when created and multiple children can share the same parent (if they are running concurrently). In the case here what happens is that the axios request is about to return synchronously so it calls `span.async()` to remove itself from the current async task span list so that other spans which are created in this task do not have it as a parent since the real processing for the request will happen in another async context. That new async context is the function passed to `axios.interceptors.request.use()` and so when that function gets control `span.resync()` is called to re-add the span to the new task span list (which will be empty). If this mechanism is removed then if there is something that creates a span between when `axios.Axios.prototype.request()` returns and when the `axios.interceptors.request.use()` handler gets control then that span will have this request as its parent, which is wrong. This something can be synchronous code that runs after the return from `request()` or something in the event loop scheduled before the new request handler function, high probability of something like this happening (like when creating multiple requests one after another) and so the `async()` and `resync()`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
