-
Notifications
You must be signed in to change notification settings - Fork 107
ScopeManager for in-process context propagation #112
Comments
It's a bit tough given that there are no thread locals in Javascript. There might be something coming up from https://github.com/nodejs/diagnostics |
Node.js diagnostics are limited to the Node.js runtime. AWS X-Ray uses https://github.com/othiym23/node-continuation-local-storage which in turn is built on https://github.com/othiym23/async-listener. It has built-in support for Node.js APIs. Though I would recommend using https://github.com/angular/zone.js which is robust with a long history. It also has an associated (early stage) TC39 proposal https://github.com/domenic/zones. If zones/thread locals become a part of the ES standard, this is the likely form. Zone.js has built-in support for both web browser and Node.js APIs. |
Good to have domain experts 🙂 Are there minimum Node version limitations in those cls libraries? |
I used node-contination-local-storage in the tutorial a while ago: |
node-cls support was developed in 2012, supported 0.12.x, and is very lower level library, The latest nodejs has async hook |
Here is a summary of the situation, along with the version support information asked above. Node modulesThe following Node modules exist and are popular enough to be mentioned here. async-listenerOverviewThis is the oldest one and probably the most used one for Node applications. It is used by the very popular continuation-local-storage. Node supportPartial Node support is available for Browser supportN/A IssuesThis library has had an unfixed memory leak for over a year. The main reason it’s not fixed yet from my understanding is the maintainers of the library have since moved on to local implementations in their respective projects, meaning they no longer actively use/maintain the library. Another issue is that this is the only library in the list that binds promises on What this means for our purpose is that we would have to maintain a fork of the project that fixes the memory leak, alters promises behavior, and doesn’t attach to the process to preserve compatibility with the non-forked module. async_hooksOverviewThis is the officially supported and built-in solution from Node. It is still marked as "experimental" but the API has been stable for a while now. Node supportIt supports Node Browser supportN/A async-hook-jlOverviewThis is a wrapper around Node supportIt supports Node Browser supportN/A zone.jsOverviewThis is the reference implementation of Zones, which is an attempt to standardize context propagation. Node supportNode support is limited to Node 8, and is incomplete as Browser supportThis is the only project on the list that supports the browser. The project is led by Angular so it makes sense. The following browsers are supported:
Node DomainOverviewThis was one of the many attempts to standardize context propagation in Node. It went directly from unstable to deprecated and never landed proper. Node supportNode IssuesThe feature is deprecated, and was never officially supported outside of being marked as unstable. The behavior of promises binding has also changed over time, and the implementation had some dealbreaker drawbacks. Proposed solutionGiven all of the above, the scope manager should be implemented with the following underlying implementations.
All of the above should be automatically selected based on the environment. The implementation should also be based on continuation passing style as explained in opentracing/specification#126, with the following API: // `activate` could be renamed to something like `run` or `execute`
// if this is considered an acceptable deviation from the spec.
interface ScopeManger {
active(): Span;
activate(Span span, f: () => void): void;
} @yurishkuro A lot of this work has already been done by @pauldraper in #113. How can we get this PR to move forward? On our end, we've implemented the aforementioned |
Thanks for the writeup @rochdev. We can devote some time to looking at this. |
Thanks @rochdev for the summary! That is a good and AFAIK complete summary of JS tracing tech. (1) The most confusing and hard to implement thing is autoclosing scopes. And I'm not sure they really solve a meaningful problem. (2) CPS is a dead-simple API, to understand, to implement, and to use. It is possible that it lacks some important ability, but so far, not examples have been provided. |
Also FYI Node 8 is the earliest version in active LTS, and as of Apr 2019, it's the earliest in maintenance LTS. |
I don't think this is necessary at all with a CPS implementation.
Since internally even CPS usually has some kind of enter/exit logic, this could be exposed eventually if warranted.
Unfortunately, it's not so simple for APM providers. On our end, we have customers that are still using Node 4, and I know some OpenTracing users are still on 0.10. Node 6 is still officially supported at the moment as well. However, this is not really a problem since as you've demonstrated in your PR it's possible to have multiple ScopeManager implementations in very few lines. |
After thinking about this a bit more, right now the current proposal stores a span, but it doesn't use any method from the span, meaning it could be used to store any arbitrary value. What I'm wondering is if we should create the scope manager as a generic external library and simply import it in Then it would in theory be possible to even create a new universal implementation of If Zone.js can support the CLS API as well, then maybe it could even be the other way around: build a universal CLS and then have the ScopeManager use it. The benefit of this approach is that users could also use the CLS for their app and effectively reuse the same context propagation mechanism. Since context propagation is very heavy in JavaScript, this would be a huge performance benefit. See the API here: https://github.com/othiym23/node-continuation-local-storage. Also note that it's missing 2 public methods |
wondering if there was any progress on this issue? |
The JavaScript implementation should follow suite of languages like Java and Python and implement the proposed ScopeManager specification. This would make managing the active span significantly easier.
RFC: https://github.com/opentracing/specification/blob/master/rfc/scope_manager.md
Python: opentracing/opentracing-python#63
Java: opentracing/opentracing-java#188
The text was updated successfully, but these errors were encountered: