-
Notifications
You must be signed in to change notification settings - Fork 16
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
The case for _not_ binding around awaits #83
Comments
That is not correct: in the spec text as it currently stands, calling |
Ah...well then that is wrong too. 😅 My understanding from prior conversations here was that we had went for following resolve path rather than promise construction path. We want context to flow from the point a promise is resolved to the point where a continuation from that resolution would run. |
Do you want If both relationships should be kept it's not about choosing a path - both are needed. |
This is starting to touch on the problem I tried to convey last year: a promise reaction is mainly a join of 2 paths in the execution graph: the resolving path and the subscribing path. This is actually something most telemetry tools I've seen don't have a way to represent (otel supports links, but they are rarely/never displayed this way by otel tools which represent the execution mostly as a tree). This is particularly apparent with One could argue there is a 3rd context: the promise creation time, but that is just the point where the execution graph "forked", and as such is usually recorded by any "call" instrumentation. It is also most of the time the same as the registration context. Both resolving and subscription contexts are necessary for building distributed debuggers allowing to visualize the execution graph. One such example is Causeway. |
So this "third" point you mention is more the difference between the scope of the With context management we basically always want to capture where scheduling happens and resume where continuation happens. In a callback-receiving function this is fairly straightforward that we bind the callback to return to the context captured from the original call. It's a bit fuzzier with promises and especially async/await, but we generally want the resolution of a promise to flow the context at that point to the continuation which runs from that resolution. It should flow in that way both with If we have a way to escape the bound context back to that scheduling path then it doesn't matter if it follows this path, but we do need the tools to get to that path. |
I want to build an example from your OP code: const ctx = new AsyncContext.Variable();
function readFile(path) {
const span = new Span();
return ctx.run(span, () => {
const fd = await openFile(path);
// This is the core ask, that awaiting can change the context.
assert.notEqual(ctx.get(), span);
// …
return data;
});
} But this is essentially the same as mutating a context in a child call and expecting it to be present in the parent execution: const ctx = new AsyncContext.Variable();
function readFile(path) {
const span = new Span();
return ctx.run(span, () => {
const open = openFile(path);
// Calling `openFile` was allowed to change my context.
assert.notEqual(ctx.get(), span);
const fd = await open;
// …
return data;
});
} I think making this change is a mistake. Rather than allowing
This was an important design goal so that we didn't violate dynamic scoping. The same as the discussion we're having in tc39/proposal-using-enforcement#1, I think allowing a child call to replace the context is a mistake.
Under all circumstances, |
My understanding of @Qard's proposal is that This would not be the function's context changing arbitrarily, it would only happen at |
It's not just a mistake, it completely breaks the feature for anything other than tracing. For scheduling outer task will suddenly start gaining the priority of tasks that were scheduled (even arbitrarily deeply) inside other tasks, for things like dependency injection this would mean inner injections would bleed out to the upper context. In order to repair this, you would have to wrap every await scheduler.runSubTask(...); on every I don't see that any solution for the tracing use case other than just providing Something I am wary though of the fact that any form of Like for termination tracking, termination can't happen until the parent context ends as well: const task = await scheduler.measureTask(theTask, (totalTime) => {
console.log(`Took: `, totalTime);
});
// This following code is *still* within the measurement lifetime because
// AsyncContext.callingContexts() could possibly be called
for (let i = 0; i < 100000000; i+=1) {
doHeavyComputation();
} |
Perhaps there should be second kind of variable (or option to const tracingVariable = new AsyncContext.TracingVariable();
tracingVariable.run("call", async function() {
console.log(tracingVariable.get()); // "call"
console.log(tracingVariable.callingContexts()); // []
await Promise.all([
new Promise((resolve) => {
tracingVariable.run("promise-1", () => {
resolve();
});
}),
new Promise((reject) => {
tracingVariable.run("promise-2", () => {
resolve();
});
}),
]);
console.log(tracingVariable.get()); // "call"
console.log(tracingVariable.callingContexts()); // ["promise-1", "promise-2"]
}); |
Yes, this is what I mean. Context should definitely not leak out of a context scope, but it should flow through a continuation path, which means that if a promise resolves with a different context then that different context should be what comes out on the other side of the await. If no context change happened before it reached the resolve than it would use the same context as before without needing to bind around the await, it would just gain the additional capability of flowing context values through continuation paths. Context is meant to model execution flow, so when execution flow is merging back then the context flow should merge back too. |
This is exactly what should be happening. If you want to orphan a sub-graph by binding around it then it probably should be explicit. I think generally speaking though that context is meant to flow to logically continuing execution. In callback-based code a call which schedules an async task would capture a snapshot and restore that snapshot around the execution of the callback. The exact same behaviour should be expected of a promise, both with It's important that the innermost path be followed and that parts of the graph do not get stepped over when propagating the context. It's acceptable for context values to flow to a point and then have a bind occur to swap that value out, but that context should flow to that point and should be primarily propagating around the points where tasks are scheduled not where continuations like callbacks are received as the receipt point often has further things within which need context for tracing, but also there could be multiple points at which tasks could be "scheduled" for a thing which is a single operation from the perspective of making a call, so each scheduling should be capturing the context at its appropriate point to be correct. |
No, this is completely unusable for all the non-tracing use cases, the whole point of
No, it's only important for the tracing use case, for all other use cases this is an anti-feature. |
Just to give an example of why the proposed behaviour is completely unusable, consider for example the dependency injection use case: // In practice the logger would be a wrapper around this, but this is fine for the example
const logger = new AsyncContext.Variable({ default: consoleLogger });
async function subTask() {
// At the end of the subtask
return await logger.run(fileLogger("subTask.log", async function() {
logger.get().print("In the subtask");
});
}
await logger.run(fileLogger("main.log"), async function() {
logger.get().print("Starting the subtask");
await subTask();
logger.get().print("Ending the subtask");
}); In this example, your proposed behaviour would have |
Yes, I am in agreement with this, and I do think that the ideal state is that context has a most reasonable default flow which avoids needing these manual binds. But there is no path which is universal, even excluding the tracing cases. We need to have mechanisms of getting the context values from other execution paths. A default can and should be chosen in the general case, but we need flexibility or the feature will be useless even for the basic application developer use case as there will be many cases where it is needed to provide some manual signals about path preference.
This is absolutely false. Flowing through the inner paths are required for correct functioning or users will most certainly try to access context in internal flows of complex interactions they are creating and have it not work the way they expected because we didn't flow the context through all execution. For example, say you have a database client connection object and make a query call which takes a callback, but as part of the connection you have some session management step that can be defined as another function, or perhaps an event--if you don't flow through the internal path to where that session management step is executed and instead step over from call to callback the user could try to read the context expecting a value to be present because it logically flows from the call and be frustrated and confused when they instead get an empty context. Context can not function correctly unless the innermost path is followed. We (APMs) have done extensive research on this subject to prove that this is the case and I have a whole bunch of documentation on this subject which I will be sharing soon when I have had the time to clean it up.
That actually usually has been the expected behaviour with AsyncLocalStorage in Node.js. It currently doesn't function this way (due to limitations of the implementation) and we have received many bug reports over the years from people expecting for context to flow to logically continuing code. Certainly in this specific example it can be a bit more ambiguous which flow is correct, and this is why I keep pushing that we need control of context flow. I will repeat this as many times as is necessary for people to finally acknowledge it: There is no single perfect context flow, we need per-store context flowing capabilities, and in the most user-friendly way possible. |
I agree that there should be a way to get other contexts, but I completely believe that the current design is the only coherent behaviour for There is no way to keep the registration, i.e. the (Users are typically not going to the ones calling these functions, so the fact
Developers often want magic that isn't coherent beyond a narrow particular usage. In particular we return to the fact that there is arbitrarily many possible contexts when Also from looking at the issues on
This is yet another case that needs to be made coherent under the existence of many contexts (i.e. |
Yes, so I'm saying as long as that capability exists then our needs are satisfied. However, I'm not seeing how exactly one could do that in a way which could result in having the different context after the await, even with a wrapper. If the await expression is ignoring whatever it receives from the promise it is yielding the result of then how would wrapping that result be any sort of solution? At the least, I think we may need to specify some additional machinery to ensure the promise continuation context can actually be available somehow, because as it is now I don't see how it would be. 🤔
Yes, and that is something we want to be able to flow through. The final task should be the default value, but we should have the tools to get the other paths to express a trace graph where execution branched and merged.
Another
Yes, I agree with this. This is another case where tools to get the other contexts are necessary. As I said earlier, I don't think the default behaviour necessarily needs to change so long as we have a way to reach the other relevant context paths. APMs are more than happy to do some additional work to make sure we're getting the right contexts at the right time, and I'm working on a Universal Diagnostics Channel proposal to manage this connection with context management. I'll be working on the docs for that next week so hopefully can share that by the end of the month. |
Do APMs need to be able to access the contexts for all of the promises resolved with Edit: Also, how would multiple calling contexts flow through the data flow graph? |
My point is that this is
Why isn't it possible to cooperatively mutate the shared object in your examples? I think the way that you're approaching this is backwards. Learning context variables as if they are function params is exactly the right mental model so that anyone could understand them. Once you enter the function body, the param references and context is set. You can mutate a shared object, but you can't replace the object value itself. |
Honestly, I personally view multi-context merges as a bit of a stretch goal. But to implement OpenTelemetry correctly, according to its own spec, it does indeed require all contexts from a multi-branch merge as it is supposed to include follows-from links to each.
I'm not sure I understand what you are asking. What do you mean by "cooperatively mutate"?
Trying to map context to existing language constructs is a mistake. It doesn't fit that model. If it did, we wouldn't need another model. Context is meant to follow intent of execution, and there are many cases where the "flow" of intended execution is pull-based such as my aws-sdk example. In cases such as these the context needs to flow back along merge paths just as the data flows. From the user perspective, not having the task pull take the context defined within the function flow into the continuation would be a failure of context propagation. Now perhaps the specific expression of how I have described this problem so far has been insufficient or imperfect from a usability perspective. But I'm trying to make clear that this form of data flow is a very real user expectation that by not supporting would be a significant source of confusion to many users. To put it as plainly as I can: context is a reflection of the flow of data within an application. |
This is a core language feature, the design likely can't be changed later so getting it right the first time is a must.
The exact same reasoning can be applied to the alternate proposal here, having values flow out of inner calls is exactly the same model as if the things had just returned the associated values. Like the example in the explainer could just be: const { headers } = await client.send(command);
// use headers somewhere Neither approach unlocks anything fundamental that can't be done today. |
Have the functions mutate the context object. Ie, there's a
I completely disagree with this. I think breaking from this would be the source of confusion. If you want additional data out of a promise, then the promise needs to return that data.
It breaks each of the README examples, and anything that would make use of nested scoping. |
That's how we create memory leaks. That is exactly what almost every APM does now and every one of them suffers severely from memory problems as a result. They're stuffing all their data into span objects and then stuffing each into one continuous trace object which they hold for the entire life of the request rather than storing only the specific bits of data needed for the given path. We shouldn't even be storing spans, we should be storing correlation IDs to minimize risk of things being held alive unexpectedly. If everything is getting dumped into a Context is a thing we have always been able to do in userland code, it was just terrible because of unreliable flow and cleanup. This is why we pushed for including it in the Node.js runtime, and now the language, so the VM can actually have full control over data lifetimes and expire things at appropriate times. But if we're not going to take advantage of that then I feel like we're missing the point of putting it in the language. What we want from context is to be able to progressively define points in the execution graph alongside the execution and flow through those points as execution flows. We specifically do not want to be holding that whole graph in-memory. The point of context is that the runtime manages what flows where and until when.
You can disagree all you want, but you're ignoring the feedback of the person maintaining the single most used context management system (AsyncLocalStorage) currently existing in JS. The existing tools sort of solve some of the needs of context management, but they are all incomplete and frequently have context breakage issues because they simply lack a fully-formed model. These existing systems have all needed to exist in this way because the more advanced needs are simply not possible without runtime assistance or tremendous complexity and overhead to achieve externally. I have provided what seems to me like a very clear case where user expectation would be that context does flow through the promise and out the await. Now I'm not saying it should always work in that way as it seems like you have some use case in mind for it to work the opposite way, which I have not seen explained so far, and I do in fact have some use for that myself, but I am saying we need to have some way to access context from that internal path for the API to be useful in many very common cases. Without a way to get internal paths like this we are standardizing broken context management. The fact that what we have now is broken should not be taken as validation of the status quo. You're talking to someone whose core focus of their entire career for near a decade has been figuring out how to make context management work. I was hired by Datadog in the first place to fix performance and reliability issues with prior context management technologies and to define what those could look like in the future, and before that it was a big part of my work at Elastic and AppNeta previously. I had my part in building the core parts of what became AsyncLocalStorage, but we had limited ability to make it work the way it actually needs to work, partly because we had not yet learned many of these behaviours I am talking about now, and partly because it was just too difficult to do outside of the runtime. I'm trying to engage here to ensure that what we land on with AsyncContext is a good API and not just pushing through something half-baked, but I'm feeling that my expertise in this space is being ignored and my examples dismissed as edge cases when I've seen quite extensively that these scenarios occur all the time in real-world code. I'll be the first to question if edge case behaviours are worth additional complexity, but these are not edge cases at all, these are actually quite common patterns for which we would be breaking. |
One question I have is, if this is such a problem for the tracing use case, why wasn't it caught earlier? The behavior across async continuations in the proposed spec text is the same as for ALS and CPED, and my understanding (and apparently the understanding of everyone in yesterday's AsyncContext meeting) was that everyone was clear on that. The examples in the readme also show that this was the intended behavior. And as far as I'm aware, before this issue, no one had raised the model you're proposing. Yet you seemed to assume that your model was the way AsyncContext would work, and not only that, but you framed the OP as if the proposal had that model for |
It was my understanding that we were following the resolve path. That was an error on my part. There was a bunch of discussion previously, particularly from James Snell, that suggested the resolve path was the more correct path and it seemed to me at least at the time that people were in agreement on that. I have only had the time to fairly passively follow the progress of the proposal until recently, so I definitely missed a lot of details and conversations. I have not been invited to any of the meetings, so I've definitely missed all those conversations, and I only joined the Matrix chat somewhat recently as I was unaware there was a channel there for this. I have unfortunately not had the time to invest too deeply in the AsyncContext discussion until more recently, which is why I had not brought this up before. I was under the impression it was still very early in development yet more recently I've had several people reach out to me telling me it's pushing rapidly toward Stage 3 and asking me to provide my own input, thus I started to dig in to make sure the model made sense before it gets too far along to propose changes. And no, I was not assuming it worked the way I'm describing, I didn't yet know all the details of how it worked. I knew of the general object model, but had not taken the time yet to look really closely at the details of how the flow works. As for the multi-path thing, that's something I'm only more recently trying to design solutions for (I have a bunch of docs to share on that soon) so I wasn't yet sure if that was a thing which could be solved reasonably in only the single-path way. My original intuition was that it could be, but only if we followed the internal path by default and allowed binding around it. But in more recent iteration on the ideas in my own experimentation recently I'm seeing that we can have a default path that follows user behaviour as much as possible, but we need some additional tools to be able to achieve that, like having a way to escape a bind to the old path (calling context) or having a way to flow through awaits (not yet sure what that should look like) and probably other cases. What I'm trying to do here is just to shine a light on some areas which I don't feel are sufficiently solved yet so we don't push to the next stage too quickly without consideration for how to handle those cases. If AsyncContext was to land in a state that it is unable to satisfy the majority of use cases then different systems will surely continue to exist and in particular Observability products may have to continue doing things their own way, which I feel would be a major failure as that will continue to leave a major performance impact on the table without any resolution. |
Can you explain the memory leak here? As far as I understand, both my proposed mutations and the resolve-time continuations would keep the parent chain alive. Especially if you're building a
That seems like it could apply with either decision. I assume we'd still need a parent ID so that we can build the relationship after the current execution is completed? In which case, my mutations suggestion would to store a new active ID in
My reason for proposing this in the language was to use ALS in a web environment, with considerably better performance. I feel like the proposal is successful at that, and so I'm hesitant to break with precedent.
It's not my intention to ignore problem, and I've proposed how I would solve the OP with the current semantics. I'm interested in figuring out why that approach doesn't work. I need more examples to understand why the memory leak exists in one situation and not the other. My disagreement here is specifically the "by not supporting would be a significant source of confusion to many users" statement. The way that I reason about AC (and everyone in the meeting yesterday), and the way I've explained it to the committee is with the current semantics. Specifically, building from a faulty
From the meeting yesterday, I think we're interested in exploring this. You, @mhofman, and Shay (I don't know their GH handle) have raised the need to receive a promise's resolving context in some senarios. This seems analogous to
I hope I haven't made you feel like I'm questioning your expertise here. I completely recognize that you have a better understanding of the problem space. I think this is the only proposal of yours that I disagree with, and I'm approaching from the standpoint of how to explain the changes to the committee on why we're breaking precedent, and does it break the use cases that I have in mind in the README examples.
This is a mistake on our part, I thought you were include in the invite list. Please message me on Matrix with your email!
The docs here would be very useful. We're currently trying to reach Stage 2.7 (signal to implementers that they should begin implementing), and the last remaining piece is defining the expected HTML integration. I think based on this discussion and the |
You keep stating this in various ways, but haven't provided compelling points that this change is good for any of the use cases other than tracing. Just to be clear I do think everyone here believes that for tracing, the current design has problems, but introducing problems (like your own suggestion that guards should wrap every Something you keep stating that developers expect context to flow out, but you also don't really address why this shouldn't apply equally so to synchronous code and why Like in your example: // The job received here has distributed tracing headers passed from another service.
// We want to create a span when client.send(...) receives the result of the command
// and finds distributed tracing headers. This span should then be stored in the context
// so subsequent execution can connect to that span.
const command = new ReceiveMessageCommand(input)
const response = await client.send(command)
// Do some processing... the fact that Further you still need to address how
what happens if we have: await Promise.all([
oneTaskThatStartsASession(),
anotherTaskThatStartsASession(),
]);
// What is the session here? does this create two sessions and one arbitrarily wins? Are all sessions available as an array? Do the two tasks somehow coordinate to avoid creating redundant sessions, how
Your suggested change here produces the same problem but the other way around, now if you set a value it leaks out: const asyncVar = new AsyncContext.Variable();
async function foo() {
return asyncVar.run(someLargeThing, () => {
});
}
// The large thing has leaked into the outer scope
await foo(); Now this thread could go in circles forever arguing one approach over the other, however I think the real solution is just to have two kinds of "async variables". I would like to explain what I think is really the solution here that works for all use cases, but first lets revisit As everyone probably knows here, when function calls occur it's parameters are essentially pushed on the call stack and the function is given access to that part of the start which it sees as parameters. However this behaviour isn't fundamental in the synchronous case, one could simulate parameters by simply having your own stack on objects: class Param {
#stack = [];
run(value, fn) {
try {
this.#stack.push(value);
return fn();
} finally {
this.#stack.pop();
}
}
get() {
return this.#stack.at(-1);
}
}
const params = new Param();
function add() {
const [x, y] = params.get();
return x + y;
}
const v = params.run([1, 2], add); This explains, however call stacks also deal with returns (and exceptions) which are similar to parameters but they live on the parent part of the stack. Like in this example: function foo() {
const v = add(3, 4);
} the parameters Now what is being called However it's clear tracing (and some other use cases I'm sure) needs the asynchronous equivalent of returns, i.e. the value is received from the resolve context (i.e. the return context). Some use cases probably even require mixes between asynchronous parameters and asynchronous returns in which case they would need both behaviours. As such as a specific proposal I'd suggest we simply have a second kind of variable which behaves in the way like return does: // With existing variable being renamed to ParameterVariable
const v = new AsyncContext.ReturnVariable();
async function openFile() {
v.set("openFile");
return ...;
}
async function foo() {
v.set("foo");
}
async function bar() {
v.set("bar");
}
async function main() {
const fd = await openFile(...);
v.get(); // openFile
await Promise.all([foo(), bar()]);
v.get(); // ["foo", "bar"]
} In particular the async context mapping would consist of two parts, the parameters context (same as today) and the return context(s), when resuming an The snapshots should probably even be split as well, i.e. you could capture the return and parameter snapshots separately: class EventsLike {
#listeners = [];
addListener(listener) {
this.#listeners.push({
listener,
parameterCtx: new AsyncContext.ParameterSnapshot(),
});
}
notify(v) {
for (const { listener, parameterCtx } of this.#listeners) {
const returnCtx = new AsyncContext.ReturnSnapshot();
queueMicrotask(() => {
AsyncContext.run(listener, parameterCtx, returnCtx);
});
}
}
}
const e = new EventsLike();
const p = new AsyncContext.ParameterVariable();
const r = new AsyncContext.ReturnVariable();
p.run("init", () => {
e.addListener(() => {
console.log(p.get()); // init
console.log(r.get()); // notify
});
});
r.run("notify", () => e.notify()); In terms of an API shape (and naming) I don't know what the best option here is, we could either have partitioned APIs: namespace AsyncContext {
export class ParameterVariable<T> { ... }
export class ReturnVariable<T> { ... }
export class ParameterSnapshot<T> { ... }
export class ReturnSnapshot<T> { ... }
} or we could have options: namespace AsyncContext {
export class Variable<T> {
constructor(opts: { kind: "parameter" | "return", ... });
}
export class Snapshot<T> {
// The return-only part of the snapshot
get returnSnapshot(): Snapshot<T> { ... }
// The parameters-only part of the snapshot
get parametersSnapshot(): Snapshot<T> { ... }
}
} |
Yes, both keep the data alive, but pushing spans into other spans keeps more data alive, which significantly increases memory pressure. It's not a "leak" in the traditional sense, but users always call it a leak because requests suddenly take several times more memory space than usual. Observability, by design, captures data everywhere and so that adds up quickly unless we're very careful about flowing the minimum amount of data possible. This is especially a concern when dealing with highly asynchronous environments like Node.js--10k concurrent requests means 10k concurrent traces, each with potentially 1k+ spans.
We need to hold only the ID representing the "span" data itself until we reach the continuation (callback, promise resolution, etc.) at which point we send an event with that ID marking the end of that span and then any new span created within that continuation or any nested execution would consider that its parent and swap the value for its part of the async execution tree. So you actually don't need separate stores for current and parent IDs, the current always becomes the parent when a switch would happen. We want to send the data out of the process immediately when a span ends, not hold it in an in-memory graph, because it's too expensive to store an in-memory graph.
Yes, I agree that it achieves mostly the same as what ALS does now, but with better performance. However, you only benefit from that better performance if you don't need to go to significant lengths to work around it not flowing in a path that is useful to your use case, which is why I believe we need more tools to be able to influence the flow. This is why I proposed instance-scoped bind previously, and why I was supportive of the calling context idea, because those can be built with pretty good performance at the runtime level, whereas building them externally is very expensive yet we quite often need those capabilities. Control over data flow and lifetimes is a big part of how performance can be achieved in these scenarios, thus my skepticism that there's actually much benefit putting this in the language if it doesn't actually enable us to gain from these supposed advantages.
As I stated above, your solution involves holding a lot of data to build the graph in-memory. This is too expensive. It's not uncommon for an app to have 10k-50k concurrent requests, which means 10k-50k traces, which would typically amount to about 10m+ spans. This typically amounts to 1gb+ of memory being occupied by spans, which makes users unhappy about the overhead. We can trim this down somewhat and store reduced representations, which is essentially just an act in pursuit of just tracking IDs, but we still have not dealt with the cpu cost of working around that the context flow doesn't deliver what our users expect and so we need to do additional work to hack around these expectations. This is work APM vendors are doing right now on top of ALS because it doesn't deliver the exact model users are expecting in some cases, so we are currently trading overhead for correctness from our users perspectives. If we had a model with more flexibility we would not have this problem.
Yes, I am aware that is how it has been communicated so far. But, to be frank, there is zero representation in the committee from people building production observability tools, so you all simply lack awareness of the more advanced needs. That's fine, that's why I'm here trying to share my experience on that topic and to get people thinking about solutions for these more demanding needs. Also, as I expressed before, I don't disagree with having a default path that is the most obvious to the average user. What I disagree with is dismissing the other perfectly valid paths entirely by not considering tools to enable other such paths. There are very valid use cases for different flows and we should be considering how we can enable them, even if it involves a little extra effort to reach those other paths.
Yes, agreed that we want something like that, though I think the
I don't view it as a break so much as an addition. Sorry if I didn't communicate the original message well but I'm definitely not trying to say flowing out of the await is the only valid interpretation, just that it is a valid interpretation and there are definite scenarios where users would expect that so I think there needs to be ways to express both paths.
Not that I've ever seen, no. I'll message you about on Matrix soon. 🙂
Yes, agreed. And I don't think it's necessarily a significant change, but I want to share my docs soon (hopefully end of this week) which will explain some of our own ideation and our reasoning for why we think things should be a bit different.
Because nothing can happen between one piece of sync code and another directly following it, so we can just use global variables at that point. With async code there can be interleaved execution between logically continuing steps so we need a tool to be able to retain data across to those continuations. ALS and other JS-based context management systems are only build to follow one direction of the application data, but other languages which have context management implementations often follow both directions.
It's not preserving of the headers that matters, it's extracting of the headers to produce a span in which operation on the task occurs. To do so requires being able to flow the span out of the await as that's the only valid place to transform those headers into a span as the rest is user code which is unknowable.
I don't yet have fully-formed intuition on exactly what that should look like, but OpenTelemetry does specify that we should should link all of those branches as follow-from when a span is created in the continuation after the I don't have all the answers here, but I do have a collection of ideas which have been discussed within the tracing space. For example, one that has come up is a fork and join model where scheduling of a promise forks the context and await joins it back into another context.
Yes, that's kind of intentional, actually. Everything after the await is a continuation of that promise resolving, so it is logically descending in some sense. If you wrote the same code with callbacks it would be obvious that the context should flow into it as it would be a callback passed into
I partly agree, though I'm not entirely convinced it should be two different variables, I have been leaning to two different flows expressed within the same variable. I'm not firm on that though. We just need something to express the other flow.
Yes, exactly. To model the "follows-from" graph which OpenTelemetry describes we need to be able to be able to retain the parent in a return flow graph when promises resolve to where the continuation does more work which could produce a new span.
Yep, so this is where I'm unsure that a two-variable model would work. There are cases where the logical flow is going inward into the calls and terminating, but there are cases such as my job processor example in the original post where it's also logical that the context flows out to the parent where it was awaited. Perhaps some sort of indicator on the part of that inner run that it should flow out? Like what if we had something like: async function foo() {
return store.runAndReturn('foo', () => {
return Promise.resolve()
})
}
await foo()
// Should be 'foo' after because we've explicitly stated the context should flow out of the return It's also worth noting that, at least in the case of tracing, I think we'd almost always want promises to flow through this return path. |
@Qard You mentioned (in private communication, I think) that you have been looking at context propagation across multiple languages. So is there any language or framework that has something like |
Yep, working on a cross-language context management design. Yet to be seen how well it applies universally, but an RFC for it exists and is getting a lot of engagement internally. This is part of the docs I plan to share soon. In most languages APMs have invented their own context management system. This works for languages like Java and .NET where they have access to rewriting bytecode, but without that power we can't really do the same externally, thus the limitations of ALS as it is presently. This does also mean the shape of context in those languages is quite different at the moment. I'll ask around within my team and see what can be shared from our other languages. Most of the team is still asleep at the moment though, so I'll get back to you on that... I did get pointed to this related issue for .NET with AsyncLocal though: dotnet/runtime#99153 |
I've been looking into Python's implementation due to this thread and due to the fact that JS's generator support is basically the same as Python's, and it's largely similar to this proposal but offers some additional tools for the inner path (but probably not in a way that would be useful for tracing). Now Python has a library called contextvars, this contains two main things that are essentially We have from contextvars import *
c_var = ContextVar("cVar") in Python, contexts are fully map-like so you can get/set at any point once you've created the variable. c_var.set(3)
print(c_var.get()) Like c_var = ContextVar("cVar")
def foo():
c_var.set("in-foo")
c_var.set("in-main")
ctx = copy_context()
ctx.run(foo)
print(c_var.get()) # Prints in-main Python doesn't have is the equivalent of priority_var = ContextVar("var")
def the_real_thing_to_run():
print(priority_var.get()) # "high"
def run_with_priority():
the_real_thing_to_run()
ctx = copy_context()
ctx.run(lambda: priority_var.set("high"))
ctx.run(run_with_priority) (Functionally this all Now you might've noticed Python's thing is just called So how does this integrate with Python's async-await, well first you need to know that coroutines in Python aren't eager, calling the async function doesn't start the coroutine and coroutines don't have As a consequence this does nothing whatsoever: c_var = ContextVar("var")
async def foo():
c_var.set("foo")
ctx = copy_context()
ctx.run(foo)
print(c_var.get()) # LookupError However if you are running in an event loop, you can use context vars with c_var = ContextVar("var")
async def foo():
c_var.set("foo")
async def main():
c_var.set("main")
await foo()
print(c_var.get()) # prints foo
asyncio.run(main()) Before you think this is special, nothing particularly interesting is happening here, it's just like if we had a mutable asyncVar: const cVar = new AsyncContext.Variable();
async function foo() {
cVar.get().value = "foo";
}
async function main() {
cVar.get().value = "main";
await foo();
console.log(cVar.get().value); // print foo
}
cVar.run({ value: null }, main); Now where things do actually get special is with how Python deals with context containment in coroutines. Because coroutines are basically just generators by a different name (well it makes it easy to distinguish Instead Python also has a type similar to Promise called Future, this has .then by another name (and without any flatmapping), this type has another subclass called Task which actually wraps a coroutine into a future. In order to give a task a separate context, instead of When you create a task this way, it automatically is like c_var = ContextVar("var")
async def foo():
c_var.set("foo")
async def main():
c_var.set("main")
await asyncio.create_task(foo())
print(c_var.get()) # Now prints main
asyncio.run(main()) Now this looks like the semantics of this current proposal (restoring the context after await). However as of the most recent version of Python tasks also expose their context, which means it's possible to view the context from the task itself (you can also pass in a custom context, rather than defaulting to a new one, which is analagous to c_var = ContextVar("var")
async def foo():
c_var.set("foo")
async def main():
c_var.set("main")
task = asyncio.create_task(foo())
await task
print(c_var.get()) # main
print(task.get_context().get(c_var)) # foo
asyncio.run(main()) This means that yes contexts do live as long as their "promises", however you only retain as long as you actually have a first class reference to the task. So the inner path is available, but it's not really available to the sorts of things that would care because you need to explictly pass the task around to get at it's context. Some general thoughts I have about Python's design here and how it relates to this proposal:
|
Yeah, basically every other language which has implemented context management has used implicit scopes and a simple setter/getter pattern rather than our
It actually is special, and an example of what I'm asking for. While the shape of the API and the linearity of the code example makes it look like a simple mutable object, it's not just a plain assignment. It manages the flow into continuations and you can see it doing the same thing even when executing concurrently, meaning it's not just patching that value away there. Calling Consider this code: from contextvars import ContextVar
import asyncio
c_var = ContextVar("var")
async def foo(name):
c_var.set(name)
async def bar(name, n):
c_var.set("bar")
await foo(name)
print("adjusted", name)
await asyncio.sleep(n)
print(name, c_var.get()) # prints name
async def main():
task1 = asyncio.create_task(bar("first", 0))
task2 = asyncio.create_task(bar("second", 0.1))
await task1
await task2
asyncio.run(main()) It will run the two tasks concurrently and will have the expected values reach the final print of each task because of the task isolation. It's certainly a bit different from our model, but it's definitely not the same as a mutable object as if it was the two branches would then be mutating and would not get the correct values due to the interleaving. I'm not sure what insight there is to be gain from this exactly as the asynchrony model of Python itself is different and so layering context management on top of it will of course look different. 🤔 |
I think the source of my confusion with the resolve path thing with promises was a combination of #16 and nodejs/node#46262. |
maybe, in both cases the agreement was to stick on capture at call time except for Regarding Some languages have constructs like try with resources to get again a defined, exception safe enter/exit behavior but without a callback. A bit like explicit resource management proposal. I don't know the python implementation, but at the time I worked on the What is really special in the .NET implementation is that it provides a hook to get notified whenever the content of the variable changes. |
I don't advocate for set/get over run, that's just what most other systems seemed to do before
Yep, most context implementations I've seen do a copy-on-write thing, either at the instance level or with a shared underlying representation like
I've been working on a language-agnostic proposal for context concepts which includes basically synchronous "windows" around parts of the execution which context applies to and integrates with a language-agnostic diagnostics channel concept which would emit before and after events around each window. I'll be sharing more very soon, just revising a few things in the doc and figuring out how to share it as our corporate Google Docs policy forbids public docs. 😅 |
I am confused by the statement that it is the design of the context propagation caused the memory pressure. I would find that the OpenTelemetry SpanContext which would be saved in a ContextVariable prevents a span from alive indefinitely, whilst preserves the ability to formulate corelations between spans. And this can be built on top of proposed |
OpenTelemetry is actually a perfect example of the problem--the JS version has a context manager which stores all pieces of data in one giant map which needs to be copied and modified every time any context data would change rather than using a different store instance per context key. This means everything needs to flow together and be accessed together. It means storing large objects and doing this extra work of navigating these objects every time context is accessed rather than splitting into many variables. Context systems need to be multi-tenant because there will undoubtedly be multiple users within most complex applications. But then if individual users go and only use one store instance for all their data then they are doing one level of lookup to get their store among all the stores and then another lookup to pull their key out of the map they've stuff into that store whereas if there was just one level with many stores that directly represented individual pieces of data then it'd be faster, simpler, and spend substantially less time doing map copies at every change and needing to navigate all the GC reference complexity that adds. If you're just directly pointer-copying most things and only changing the stores that actually need changing then it's just a whole lot better from both a memory and computation cost perspective. |
@andreubotella (from way upthread)
Computed signals can logically have multiple calling contexts: const sig1 = new Signal.State(1);
const sig2 = new Signal.State(2);
const sum = new Signal.Computed(() => sig1.get() + sig2.get() + var1.get());
var1.run(3, () => sig1.set(4));
var1.run(5, () => sig2.set(6)); Computed signals are evaluated lazily, so it logically has two causes, and those contexts would presumably need to be merged somehow (exactly how probably depends on the specific variable). Reading through this thread, it seems like we've been talking past each other for most of it. Interestingly, I'm understanding @Qard to be saying that preserving the context mapping across In fact, tracing is one of my goals for |
Even multiple calling contexts aside, this example is a good reason why "return-like" variables should be able to flow up the stack even in synchronous cases. I went ahead and wrote up some possible semantics for the two-kinds-of-flow suggestion I made above which should help for arguing about precise semantics for how these flows actually work. Here, "parameter variables" work the same way the proposal already does today and have the same methods. "Return variables" work more like is being proposed here in the OP, essentially the context at (Also obviously I can't change the builtin Example usages in async await, event integration, and namespace AsyncContext {
type ParameterMapping = Map<Variable<any>, any>;
export type GetReturn<T> =
| { kind: "multiple"; values: Array<GetReturn<T>> }
| { kind: "one"; value: T | undefined };
class ReturnMappings {
static ofMappings(childMappings: ReadonlyArray<ReturnMappings>): ReturnMappings {
return new ReturnMappings(new Map(), childMappings);
}
readonly #mappings: Map<Variable<any>, any>;
readonly #childMappings: ReadonlyArray<ReturnMappings>;
constructor(
mappings = new Map<Variable<any>, any>(),
childMappings: ReadonlyArray<ReturnMappings> = [],
) {
this.#mappings = mappings;
this.#childMappings = childMappings;
}
#get(variable: Variable<any>): GetReturn<any> {
if (this.#mappings.has(variable)) {
return { kind: "one", value: this.#mappings.get(variable) };
}
return {
kind: "multiple",
values: this.#childMappings.map((mapping) => {
return mapping.#get(variable);
}),
};
}
get(variable: Variable<any>): GetReturn<any> {
return this.#get(variable);
}
#without(variable: Variable<any>): ReturnMappings {
const newMapping = new Map([...this.#mappings]);
newMapping.delete(variable);
return new ReturnMappings(
newMapping,
this.#childMappings.map((mapping) => {
return mapping.#without(variable);
}),
);
}
without(variable: Variable<any>): ReturnMappings {
return this.#without(variable);
}
withSet(variable: Variable<any>, value: any): ReturnMappings {
return new ReturnMappings(
new Map([...this.#mappings, [variable, value]]),
// We clean the inner sets to avoid holding onto dead values, in practice this
// data structure is just not very good requiring this much enumeration to set
// values
this.#childMappings.map((mapping) => {
return mapping.#without(variable);
}),
);
}
}
export let activeParameterMapping: ParameterMapping = new Map();
export let activeReturnMappings: ReturnMappings = new ReturnMappings();
export type VariableOptions<T> = {
name?: string | undefined;
defaultValue?: T;
};
export class Variable<T> {
readonly #name: string;
readonly #defaultValue: T | undefined;
constructor({ name = "", defaultValue }: VariableOptions<T> = {}) {
this.#name = name;
this.#defaultValue = defaultValue;
}
get name(): string {
return this.#name;
}
get(): T | undefined {
if (activeParameterMapping.has(this)) {
return activeParameterMapping.get(this);
}
return this.#defaultValue;
}
getReturn(): GetReturn<T> {
return activeReturnMappings.get(this);
}
run<Args extends ReadonlyArray<any>, Return>(
value: T,
cb: (...args: Args) => Return,
...args: Args
): Return {
const previousParameterMapping = activeParameterMapping;
try {
activeParameterMapping = new Map([...activeParameterMapping, [this, value]]);
return cb(...args);
} finally {
activeParameterMapping = previousParameterMapping;
}
}
setReturn(value: T): void {
activeReturnMappings = activeReturnMappings.withSet(this, value);
}
deleteReturn(): void {
activeReturnMappings = activeReturnMappings.without(this);
}
setReturnIn<Args extends ReadonlyArray<any>, R>(
value: T,
fn: (...args: Args) => R,
...args: Args
): R {
const previousReturnMappings = activeReturnMappings;
try {
activeReturnMappings = activeReturnMappings.withSet(this, value);
return fn(...args);
} finally {
activeReturnMappings = previousReturnMappings;
}
}
}
export type SnapshotOptions = {
captureInVariables?: boolean | undefined;
captureOutVariables?: boolean | undefined;
};
export function setReturnAll(snapshots: ReadonlyArray<Snapshot>): void {
activeReturnMappings = ReturnMappings.ofMappings(
snapshots
.map((snapshot) => {
return getReturnMappings(snapshot);
})
.filter((mapping): mapping is ReturnMappings => mapping !== null),
);
}
let getReturnMappings: (snapshot: Snapshot) => ReturnMappings | null;
export class Snapshot {
static wrap<This, Args extends ReadonlyArray<any>, Return>(
fn: (this: This, ...args: Args) => Return,
options: SnapshotOptions = {},
): (this: This, ...args: Args) => Return {
const snapshot = new Snapshot(options);
return function wrapper(this: This, ...args: Args): Return {
return snapshot.run(() => {
return fn.call(this, ...args);
});
};
}
static {
getReturnMappings = (snapshot) => snapshot.#returnMappings;
}
readonly #parameterMapping: ParameterMapping | null;
readonly #returnMappings: ReturnMappings | null;
constructor({
captureInVariables = false,
captureOutVariables = false,
}: SnapshotOptions = {}) {
if (!captureInVariables && !captureOutVariables) {
throw new RangeError(`at least one type of async variable should be captured`);
}
this.#parameterMapping = captureInVariables ? activeParameterMapping : null;
this.#returnMappings = captureOutVariables ? activeReturnMappings : null;
}
run<Args extends ReadonlyArray<any>, Return>(
cb: (...args: Args) => Return,
...args: Args
): Return {
const previousParameterMapping = activeParameterMapping;
const previousReturnMappings = activeReturnMappings;
try {
activeParameterMapping = this.#parameterMapping ?? activeParameterMapping;
activeReturnMappings = this.#returnMappings ?? activeReturnMappings;
return cb(...args);
} finally {
activeParameterMapping = previousParameterMapping;
activeReturnMappings = previousReturnMappings;
}
}
}
type PromisePendingState<T> = {
kind: "pending";
fulfilledCallbacks: Array<(value: T) => void>;
rejectedCallbacks: Array<(error: any) => void>;
};
type PromiseFulfilledState<T> = {
kind: "fulfilled";
resolveContext: Snapshot;
value: T;
};
type PromiseRejectedState = {
kind: "rejected";
resolveContext: Snapshot;
error: any;
};
type PromiseState<T> = PromiseFulfilledState<T> | PromisePendingState<T> | PromiseRejectedState;
export class Promise<T> {
static #isPromise(value: any): value is Promise<any> {
return value != null && #status in value;
}
static #resolve<T>(value: Promise<T> | T): Promise<T> {
if (Promise.#isPromise(value)) {
return value;
}
return new Promise<T>((resolve, reject) => {
resolve(value as T);
});
}
static #try<Args extends ReadonlyArray<any>, Return>(
fn: (...args: Args) => Promise<Return> | Return,
...args: Args
): Promise<Return> {
return new Promise((resolve, reject) => {
const p = Promise.#resolve(fn(...args));
p.#subscribe(resolve, reject);
});
}
static all<T extends ReadonlyArray<unknown> | []>(
values: T,
): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }> {
return new Promise((resolve, reject) => {
let pendingResults = 0;
const results: Array<any> = [];
const resolveContexts: Array<Snapshot | null> = [];
for (const [index, value] of values.entries()) {
pendingResults += 1;
results.push(null);
resolveContexts.push(null as any);
Promise.resolve(value).then((value) => {
resolveContexts[index] = new Snapshot({ captureOutVariables: true });
pendingResults -= 1;
results[index] = value;
if (pendingResults === 0) {
AsyncContext.setReturnAll(resolveContexts as any);
resolve(results as any);
}
}, reject);
}
});
}
static resolve<T>(value: Promise<T> | T): Promise<T> {
return Promise.#resolve(value);
}
static reject<T = never>(error: any): Promise<T> {
return new Promise((resolve, reject) => {
reject(error);
});
}
#status: PromiseState<T> = {
kind: "pending",
fulfilledCallbacks: [],
rejectedCallbacks: [],
};
constructor(init: (resolve: (value: T) => void, reject: (error: any) => void) => void) {
try {
init(
(value) => {
if (this.#status.kind !== "pending") {
return;
}
const { fulfilledCallbacks } = this.#status;
// NEW: Store the active return context along with the resolved value to
// be called with on future calls of .then
const resolveContext = new Snapshot({ captureOutVariables: true });
this.#status = { kind: "fulfilled", value, resolveContext };
for (const onFulfilled of fulfilledCallbacks) {
globalThis.queueMicrotask(() => {
resolveContext.run(onFulfilled, value);
});
}
},
(error) => {
if (this.#status.kind !== "pending") {
return;
}
const { rejectedCallbacks } = this.#status;
// NEW: Same as in resolve
const resolveContext = new Snapshot({ captureOutVariables: true });
this.#status = { kind: "rejected", error, resolveContext };
for (const onRejected of rejectedCallbacks) {
globalThis.queueMicrotask(() => {
resolveContext.run(onRejected, error);
});
}
},
);
} catch (error: unknown) {
if (this.#status.kind === "pending") {
// Capture the context for synchronous rejection
this.#status = {
kind: "rejected",
error,
resolveContext: new Snapshot({ captureOutVariables: true }),
};
}
}
}
#subscribe(
onFulfilled: ((value: T) => void) | null,
onRejected: ((error: any) => void) | null,
): void {
if (this.#status.kind === "fulfilled") {
if (onFulfilled) {
const { value, resolveContext } = this.#status;
globalThis.queueMicrotask(() => {
resolveContext.run(onFulfilled, value);
});
}
} else if (this.#status.kind === "rejected") {
if (onRejected) {
const { error, resolveContext } = this.#status;
globalThis.queueMicrotask(() => {
resolveContext.run(onRejected, error);
});
}
} else {
if (onFulfilled) {
this.#status.fulfilledCallbacks.push(onFulfilled);
}
if (onRejected) {
this.#status.rejectedCallbacks.push(onRejected);
}
}
}
then<TResult1 = T, TResult2 = never>(
onFulfilled?: ((value: T) => Promise<TResult1> | TResult1) | null | undefined,
onRejected?: ((reason: any) => Promise<TResult2> | TResult2) | null | undefined,
): Promise<TResult1 | TResult2> {
onFulfilled =
onFulfilled ?
Snapshot.wrap(onFulfilled, { captureInVariables: true })
: onFulfilled;
onRejected =
onRejected ? Snapshot.wrap(onRejected, { captureInVariables: true }) : onRejected;
return new Promise((resolve, reject) => {
this.#subscribe(
(value) => {
if (onFulfilled) {
const p = Promise.#try(onFulfilled, value);
p.#subscribe(resolve, reject);
} else {
resolve(value as unknown as TResult1);
}
},
(error) => {
if (onRejected) {
const p = Promise.#try(onRejected, error);
p.#subscribe(resolve, reject);
} else {
reject(error);
}
},
);
});
}
}
// NOTE: There is actually nothing in this implementation that cares about AsyncContext
// all behaviour falls out as a consequence of how Promise.then is defined above
export function async<This, Args extends ReadonlyArray<any>, Return>(
genFunc: (this: This, ...args: Args) => Generator<any, Return, any>,
): (this: This, ...args: Args) => Promise<Return> {
return function asyncWrapper(...args: Args): Promise<Return> {
return new Promise<Return>((resolve, reject) => {
const gen = genFunc.call(this, ...args);
function continueAsync(value: any, isError: boolean): void {
try {
const iterResult = isError ? gen.throw(value) : gen.next(value);
if (iterResult.done) {
resolve(iterResult.value);
} else {
Promise.resolve(iterResult.value).then(
(value) => {
continueAsync(value, false);
},
(error) => {
continueAsync(error, true);
},
);
}
} catch (error: unknown) {
reject(error);
}
}
continueAsync(undefined, false);
});
};
}
export function queueMicrotask(cb: () => void): void {
globalThis.queueMicrotask(
Snapshot.wrap(cb, { captureInVariables: true, captureOutVariables: true }),
);
}
export function setTimeout(cb: () => void, time: number): any {
return globalThis.setTimeout(
Snapshot.wrap(cb, { captureInVariables: true, captureOutVariables: true }),
time,
);
}
export function setInterval(cb: () => void, time: number): any {
return globalThis.setInterval(
Snapshot.wrap(cb, { captureInVariables: true, captureOutVariables: true }),
time,
);
}
}
export default AsyncContext;
// Example with an event-like system
{
class EventsLike<T> {
readonly #listeners: Array<(v: T) => void> = [];
addListener(listener: (v: T) => void): void {
this.#listeners.push(
AsyncContext.Snapshot.wrap(listener, { captureInVariables: true }),
);
}
notify(v: T): void {
for (const listener of this.#listeners) {
queueMicrotask(
AsyncContext.Snapshot.wrap(() => void listener(v), {
captureOutVariables: true,
}),
);
}
}
}
const e = new EventsLike<number>();
const paramVar = new AsyncContext.Variable<string>();
const returnVar = new AsyncContext.Variable<string>();
paramVar.run("init", () => {
e.addListener((v) => {
console.log(`paramVar: `, paramVar.get());
console.log(`returnVar: `, returnVar.getReturn());
});
});
returnVar.setReturnIn("notify", () => {
e.notify(10);
});
}
// Example with pure promises
{
const paramVar = new AsyncContext.Variable<string>();
const returnVar = new AsyncContext.Variable<string>();
const promise = new AsyncContext.Promise<number>((resolve) => {
globalThis.setTimeout(() => {
returnVar.setReturnIn("resolve", () => {
resolve(10);
});
}, 10);
});
await paramVar.run("init", () => {
return promise.then((v) => {
console.log(`paramVar: `, paramVar.get());
console.log(`returnVar: `, returnVar.getReturn());
});
});
}
// Example with "async await"
{
const returnVar = new AsyncContext.Variable<string>();
const paramVar = new AsyncContext.Variable<string>();
const mixedVar = new AsyncContext.Variable<string>();
const f = AsyncContext.async(function* f(): Generator<any, void, any> {
yield new AsyncContext.Promise((resolve) => {
globalThis.setTimeout(() => {
mixedVar.setReturnIn("resolve mixed", () => {
returnVar.setReturnIn("resolve", () => {
resolve(null);
});
});
});
});
console.log(`paramVar: `, paramVar.get());
console.log(`returnVar: `, returnVar.getReturn());
console.log(`mixedVar param: `, mixedVar.get());
console.log(`mixedVar return: `, mixedVar.getReturn());
});
await paramVar.run("init", () => {
return mixedVar.run("mixed init", () => {
return f();
});
});
}
// Example with Promise.all
{
const returnVar = new AsyncContext.Variable<string>();
const paramVar = new AsyncContext.Variable<string>();
await paramVar.run("init", () => {
return AsyncContext.Promise.all([
new AsyncContext.Promise((resolve) => {
returnVar.setReturnIn("foo", () => {
resolve("foofoo");
});
}),
(async () => {
returnVar.setReturn("bar");
return "barbar";
})(),
]).then(() => {
console.log(`paramVar: `, paramVar.get());
console.log(`returnVar: `, returnVar.getReturn());
returnVar.setReturn("new return");
});
});
} |
Mutable global are insufficient because between when the promise-returning function resolves internally and when the await resumes there could be other things that happen, so we specifically need a solution to bind between that promise resolve and its continuation. There currently are no solutions to this, not even patching the world, because await will insert microtasks we can't patch. |
I had a call with @andreubotella the other day explaining some of what APM vendors want from context management that we don't have now, explaining the use cases, how we are limited presently, and how some aspects of the proposal at present somewhat limit us too. One of the main issues discussed was that capturing context when an await happens and then restoring at that point cuts off a branch of the graph which we are interested in.
Some examples:
In this scenario we want the execution branches which merge back through an await to also merge back their context. If we instead just relied on context flowing through the promise resolve we would get this automatically and, if no context adjustment was made within openFile, we would still get the same flow expected by binding at awaits.
Another real-world scenario is that we instrument aws-sdk to capture SQS jobs being processed and linking them into a distributed trace. A job processor would look something like this:
In the above example we want each
await client.send(...)
to flow the span created within out to the async function it's being called within. Subsequent execution within that function until the nextawait client.send(...)
should be considered as continuations of that context. Because of promises flowing context through the resolve path, if this were to be rewritten withpromise.then(...)
nesting instead of async/await this would behave as expected, which leads me to believe that binding around awaits is actually incorrect. What we should be doing is just letting the context flow into the functions being called and merged back out of whatever the resolution of the awaited promises are. Context is meant to follow execution behaviour, so if execution is merging back from elsewhere so too should context.If we have a way to access the calling context rather than the bound context then this would be a non-issue, but if we're aiming for a single blessed path then this would make the current design of context flow completely unusable for observability products.
As it is presently, context within an async function is effectively no different from local variables. The stack restores in the same way as the context binding over an await does, so they are functionally the same at that point. The whole point of adding context management to the language is that it can flow values through paths which existing code can not.
The text was updated successfully, but these errors were encountered: