-
Notifications
You must be signed in to change notification settings - Fork 36
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
[Bug]: Instana context loss when cls-hooked is also used #438
Comments
Applies a patch to the cls-hooked module (https://github.com/Jeff-Lewis/cls-hooked/). This patch fixes a breakage that occurs in the following scenario: * The application under monitoring uses the cls-hooked package (in addition to @instana/core), * the application under monitoring binds the incoming http request object (`IncomingMessage`, which is an event emitter), via cls-hooked/bindEmitter, and * the incoming request has a payload. See also: * #438 * https://github.com/jonathansamines/instana-context-loss fixes #438
Hey Jonathan, thanks a ton for this excellent report and the reproducer. ❤️ This made it really easy to troubleshoot the problem. A fix is underway and expected to be released later this week. We will keep you posted. Note that this is actually a bug in Going on a slight tangent: You might want to take a look at AsyncLocalStorage at some point, which should cover all (or almost all) use cases of The intention of the Node.js core team is to enable userland modules that use the async_hooks API directly (as the We have already prepared to use AsyncLocalStorage for our own requirements and will probably switch to that sometime soon for newer Node.js versions. It was pretty straight forward to replace our own version of cls-hooked with AsyncLocalStorage. |
Thanks for the quick response and remediation @basti1302.
Absolutely, can't wait to replace our own usage of |
Applies a patch to the cls-hooked module (https://github.com/Jeff-Lewis/cls-hooked/). This patch fixes a breakage that occurs in the following scenario: * The application under monitoring uses the cls-hooked package (in addition to @instana/core), * the application under monitoring binds the incoming http request object (`IncomingMessage`, which is an event emitter), via cls-hooked/bindEmitter, and * the incoming request has a payload. See also: * #438 * https://github.com/jonathansamines/instana-context-loss fixes #438
Applies a patch to the cls-hooked module (https://github.com/Jeff-Lewis/cls-hooked/). This patch fixes a breakage that occurs in the following scenario: * The application under monitoring uses the cls-hooked package (in addition to @instana/core), * the application under monitoring binds the incoming http request object (`IncomingMessage`, which is an event emitter), via cls-hooked/bindEmitter, and * the incoming request has a payload. See also: * #438 * https://github.com/jonathansamines/instana-context-loss fixes #438
@jonathansamines the fix has landed in release 1.137.3. |
Many thanks @basti1302. I verified locally and everything works now. Will take us some time to upgrade our systems but hopefully this will fix our context loss issues. |
Problem Description
When the @instana/collector is loaded into a Node.js application, but that application also uses cls-hooked to propagate and preserve it's own context, then they both seem to conflict and only one context is propagated, furthermore
upon closer inspection, this problem only seems to happen when:
Short, Self Contained Example
Please see instana-context-loss for a complete, self contained reproduction of the problem.
Node.js Version
Node.js 14.18.2
package.json
package-lock.json
The text was updated successfully, but these errors were encountered: