[GitHub] [skywalking-nodejs] tom-pytel opened a new pull request #10: Mostly cleanups

[GitBox <gi...@apache.org>]

tom-pytel opened a new pull request #10:
URL: https://github.com/apache/skywalking-nodejs/pull/10


   * Incorporated changes from 'cleanup' branch - that is removed `Context.capture()` and `.restore()` since they are now redundant and wouldn't even work correctly with the new method of async tracking.
   * Resolved issue of `Span.inject()` in `Context.newExitSpan()`, doesn't need to be there.
   * Defined behavior a little more clearly for `HttpPlugin` which will be a nested base for most plugins - sets `peer` and `httpURL` only if not already set (by higher level plugin maybe) and sets `component` and `layer` only if it is sole plugin. All this because the exit portion will be executed after the exit code of any overriding plugin.
   
   So basically the main problem before was how to deal with Javascript asynchronicity and I think that is mostly solved. This agent should actually work now without weirdness due to async. I've tested it on my side with several async requests to several servers themselves executing async requests and it works as expected.
   
   What is left for the code here is to add tests and a component ID for `axios` and maybe `request` modules? I haven't done a plugin for `request` since it just works straight through `http`, but if someone wants to identify it specifically as a `request` component then that will need to be added. And of course plugins for any other modules that need support, but the main thing is that the underlying system should now be able to handle any type of plugin (callback, promise, async, etc...).


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #10: Mostly cleanups

[GitBox <gi...@apache.org>]

tom-pytel commented on pull request #10:
URL: https://github.com/apache/skywalking-nodejs/pull/10#issuecomment-741937448


   To be clear, what I meant is that this is ready for merge, those other things I mention are TODOs for the future.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [skywalking-nodejs] kezhenxu94 merged pull request #10: Mostly cleanups

[GitBox <gi...@apache.org>]

kezhenxu94 merged pull request #10:
URL: https://github.com/apache/skywalking-nodejs/pull/10


   


----------------------------------------------------------------
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:
users@infra.apache.org