Repository navigation
Identify async_hooks use cases beyond AsyncLocalStorage #437
Description
It seems to be the general consensus of this working group that async_hooks should not be directly made stable due to it exposing internals. The AsyncLocalStorage API provides a higher-level solution to many of the use cases of async_hooks, however some use cases still remain. I would like to identify what those use cases are so we can introduce safer APIs solving those problems and eventually move toward deprecating direct use of async_hooks or perhaps just the unsafe aspects of it.
Known use cases:
- long stack traces
- Shouldn't make any use of the resource objects, so should be reasonably safe.
- tracking handle lifecycle
- The subsystem type, id, and event timings can be used to track high-level lifecycle safely
- Deeper awareness may involve inspecting the resource object, which could be unsafe
- measure time spent in blocking code
- Mainly just need timing between before/after and maybe type for extra context, resource not required
What other use cases are there? I know clinic uses it for some things, perhaps @mcollina has some insight on that?
Activity
naugtur commentedon Oct 16, 2020
https://github.com/naugtur/blocked-at - I used async_hooks to detect the event loop blocking function.
They can also be used to observe asynchronous flow execution, count number of async resources of certain type being created.
https://github.com/naugtur/debugging-aid
None of these use cases really need access to the resource reference itself. The most valuable information is timing and stack trace form init callback in these cases.
Qard commentedon Oct 17, 2020
Added to the list. 👍
Needs we have so far:
Selectively capture stack trace at init and make available in descending async contexts
I believe this could be achieved through
AsyncLocalStoragewith the addition of an init trigger that could be used to capture the stack trace at that point and store it in the storage. Retrieving is just a matter of getting the value from the storage.We could even have something like
executionAsyncStackTrace()to do the same as whatexecutionAsyncResource()does for resources, but for stack traces. Though, for performance reasons, we'd probably need to make it so it has to be turned on explicitly. 🤔Record event timings
We can already capture general timings with
perf_hooks.monitorEventLoopDelay(...). Could expand on that with resource type scoped histograms also, if individual timings for each run are not as important. Correlating with stack traces would be harder in that case though.Another option is that this too could probably be layered on top of
AsyncLocalStorage, storing an hrtime in an init trigger. It would need some additional system of computing and emitting the diff hrtimes in the other async_hooks events though, and I'm not sure exactly what that would look like or what the performance impact would be.Do we need individual timings here, or could we reuse the Histogram interface from perf_hooks event loop monitoring but split per resource type and event type combination?
mcollina commentedon Oct 17, 2020
A few of our clients uses cla-hooked to keep the request/response/user settings/logger and a lot of other critical data. The lose of context would be extremely problematic.
Qard commentedon Oct 17, 2020
Could we not just update cls-hooked to use AsyncLocalStorage internally? And probably even encourage users of it to just switch to using AsyncLocalStorage directly? They're functionally the same.
Also, to be clear: I'm definitely not proposing deprecating async_hooks right away. I'm just working on clarifying the path towards being able to do that someday as it's not great that we have this unsafe API exposing internals. I want to gradually migrate all our use cases for async_hooks to safer APIs so we can someday deprecate it.
mcollina commentedon Oct 17, 2020
In Bubbleprof we use async_hooks to keep track of all the states of an async activity.
Qard commentedon Oct 17, 2020
"All the states" being the async_hooks event transitions, or do you do some deeper inspection of the resource objects? If you interact with the handles directly, why? Is it possible to encapsulate any handle access needs into a new API that doesn't expose the internal handle directly?
Flarna commentedon Oct 17, 2020
One feature I miss in
AsyncLocalStorageis to decide if context passing should happen or not based on the resource type. Technically it's correct to pass context viasetIntervalbut in real life it's not wanted in each case. This doesn't require the resource itself.A slightly different approach would be to configure some timeout when a context should be no longer propagated.
The usecase where this is helpful are lazy initalizations of e.g. a logger, database connection,... within the first HTTP request as they stay assoziated with this request forever.
I have also an idea in mind to sum up CPU and wall clock time for an async tree. This doesn't require the resource but before/after hooks.
Qard commentedon Oct 17, 2020
That propagation decision is potentially something that could be achieved with the more limited init hook that'd also be needed by the stack trace capturing and event timing needs mentioned above, if we design it right. I see something emerging there.
naugtur commentedon Oct 17, 2020
@Flarna the decision about context passing (or not) is made based in resource type, not reference to the resource itself, right?
Flarna commentedon Oct 17, 2020
For timers the timout would be helpful. some people use
setTimeout(0)as replacement fornextTickwhich is interesting to track. But timers for minutes are usually not of interest for a tracing usecase.mcollina commentedon Oct 17, 2020
We just inspect the type of the handle.
AndreasMadsen commentedon Oct 17, 2020
@mcollina @Qard I don't work for NearForm/clinic anymore. But one obscure usage is to detect
http.Serverinstances and what address+port they are listening on. This is to automatically benchmark them, without the user specifying the port and address. You can look at https://github.com/mafintosh/on-net-listen.I can understand replacing the resource object. As it is now, it is unsafe. However, I think there is value in attaching metadata to a resource. I don't get deprecating all of async_hooks, and providing higher-level APIs. I think it has always been the philosophy of node.js to provide low-level APIs to enable users to write their own modules. Deprecating async_hooks would conflict with that philosophy.
Qard commentedon Oct 17, 2020
The use case of detecting a net server is one of the use cases I see for diagnostics_channel, so I think we have a solution incoming for that.
As for the low-level APIs comment: I don't think this is a low-level versus high-level thing, this is a safe versus unsafe thing. Yes, there are many other low-level APIs, but they're generally all much safer and are not exposing undocumented internals. My goal here is to find the path forward to removing unsafety, whether that be through deprecating parts of the async_hooks API or deprecating the API as a whole. This is a sketchpad/brainstorming session to figure out what the possibilities are to achieve the results we want in a better way.
My personal perspective on async_hooks is that it conflates several unrelated things because one specific use case had multiple problems to solve--the context propagation need required barrier tracking and there are multiple forms of async barrier that needs to be propagated through. For mostly any other use case, the distinction between promises, timers, and libuv handles/requests is important and should probably not be presented in an aggregate form which then has to be filtered back to the form we actually want because that's just an unnecessary expense and complicates the internals. The problem with async_hooks in it's current form is actually not that it's too low-level, it's that it's an arbitrary mix of too much abstraction where none is wanted and not enough where some is needed.
My ultimate goal is to raise AsyncLocalStorage to a deeper level of integration, bypassing much of the unnecessary bits of async_hooks imposed by using the user-facing API. In addition, I want to create some more purpose-focused APIs to solve the other needs of async_hooks, which can be built with a more appropriate scope. By constructing more intentionally designed APIs that only operate on what needs to be operated on, many of these scenarios can achieve much better performance, rather than just having one global firehose of every async thing ever. There's a reason async_hooks performance is still bad in many scenarios: it does too much. 😬
16 remaining items
Qard commentedon Aug 8, 2021
It's turned off by default. You should only see it if something in your app is using it.
leodutra commentedon Aug 20, 2021
Sequelize (ORM) uses CLS-hooked for managed transactions. They had some had time with CLS + async in the past, but it looks like they could not move from it for some reason, IDK (I'm a consumer).
There's a good explanation in the cls-hooked package adventures, that may be valuable:
https://www.npmjs.com/package/cls-hooked#a-little-history-of-asyncwrapasync_hooks-and-its-incarnations
leodutra commentedon Aug 20, 2021
@papb I wonder if this is of interest.
Qard commentedon Aug 20, 2021
It should be straightforward to switch from cls-hooked to AsyncLocalStorage.
leodutra commentedon Aug 20, 2021
As I said, I'm just a consumer. Hopefully, someone more involved will bring more value than me.
Thanks for the info @Qard
cjihrig commentedon Feb 28, 2022
Just another data point: I've been working on a test runner, and have been using async hooks to associate resources with individual tests so that I can detect async operations that live beyond the lifetime of the test itself.
github-actions commentedon Jul 20, 2022
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.
github-actions commentedon Nov 2, 2022
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.