tom-pytel edited a comment on pull request #2: URL: https://github.com/apache/skywalking-nodejs/pull/2#issuecomment-735255536
> > * Context.capture() and restore() will be made redundant / unnecessary by this? > > Yes I believe they're unnecessary after this PR, feel free to remove them altogether in this PR or in a following PR if you like. Next or following PR, don't want to get too ahead. > Like what I commented, `ContextManagere.withSpan()` seems to be not that useful but just for catching exceptions? I don't want to add another callback level in the users codes. See examples I will send. > Let's tackle Node10 a little bit later after this PR. "using async_hooks.createHook init and destroy to track parent-child relationships" this is my original thought before coming across `AsyncLocalStorage`, but be careful, we need to evaluate the memory usage and performance impact when tracking the parent-child relationship, (does `AsyncLocalStorage` do so behind the scene?) Doesn't really matter what the performance hit is since otherwise SW will not be usable at all on Node 10 without this, so having the option of running slower vs. not running at all is an obvious choice. > I'd consider this as a bug (not sending segment when it exists very quickly). > > Let's create some issues to track those aforementioned that are not to be addressed in this PR ~~Feel free to create the issues so they don't get forgotten about.~~ [EDIT]: Fixed. ---------------------------------------------------------------- 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: us...@infra.apache.org
