-
Notifications
You must be signed in to change notification settings - Fork 143
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
Do not release application insights context and continuation-local-storage can't build e2e trace #592
Comments
For node.js 8.2+, we use async_hooks which was very new from your node.js version's perspective and so it might be buggy/unstable at that point. You might also have some dangling references to those contexts that are preventing garbage collection from happening. I'd recommend either using a more recent node.js version and see if it resolves the issue, or force the SDK to use
|
Thx @markwolff , we'll change to use continuation-local-storage instead, per check its code, there are also no internal _contexts map. In fact, we just integrate applicationInsights with auto-collection functions, so there should no dangling references to those cls_hooks namespace's internal field of _contexts map. |
Hi @markwolff , after i invoked |
Is it related to Memory leak in |
It could be if you are referring to the AI context in your own code at all. I'm unable to repro a leak with a simple express demo on node 8.3.0. Otherwise, I would expect each generated context to be remain on the heap until it has been GC'd, so you may see some contexts depending on your service's RPS when you profiled it. Can you clarify what you mean by not building a trace (screenshot?). I've tested this on my demo as well and it seems to be working as intended. |
We have multiple applications/services which build a whole system, I mean once we changed to call |
Did you set process.env.DEBUG_CLS_HOOKED to check the _contexts map? |
Setting forceCls should have no effect on incoming request telemetry correlation. It reads the http header the same way, regardless of this configuration. What do you see on I did not use DEBUG_CLS_HOOKED. Does it do something other than log async id information to console? |
Yes, i can see there are still old context in _contexts map after the request. |
You should import and start the SDK before you import in any HTTP modules. It seems the patching required for CLS-mode requires us to not only be imported before express/requests, but also started before as well. |
Thx @markwolff , after the code change, it worked with |
We integrated application insights with a service (apigee microgateway) , this service used nodejs 8.3.0, applicationInsights-nodejs 1.7.0. we found that the service's memory is growing as time goes on. We dumped the heap, found that there are many application insights context in memory.
And our service used cluster mode , forked threads per CPU, so there are 40M * number of CPU context data.
Per check the code, this context related code is in applicationinsights\out\AutoCollection\CorrelationContextManager.js and HttpRequest.js

Any clue?
The text was updated successfully, but these errors were encountered: