Skip to content

Commit 12e6246

Browse files
authored
fix(asynchooks-scope): fix context loss using .with() open-telemetry#1101 (open-telemetry#1099)
This addresses open-telemetry#1101 open-telemetry#752, and open-telemetry#1013 - Fix context loss when used in some async cases
1 parent 2987534 commit 12e6246

File tree

2 files changed

+107
-225
lines changed

2 files changed

+107
-225
lines changed

packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts

+47-62
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,6 @@ type PatchedEventEmitter = {
2929
__ot_listeners?: { [name: string]: WeakMap<Func<void>, Func<void>> };
3030
} & EventEmitter;
3131

32-
class Reference<T> {
33-
constructor(private _value: T) {}
34-
35-
set(value: T) {
36-
this._value = value;
37-
return this;
38-
}
39-
40-
get() {
41-
return this._value;
42-
}
43-
}
44-
4532
const ADD_LISTENER_METHODS = [
4633
'addListener' as 'addListener',
4734
'on' as 'on',
@@ -52,72 +39,36 @@ const ADD_LISTENER_METHODS = [
5239

5340
export class AsyncHooksContextManager implements ContextManager {
5441
private _asyncHook: asyncHooks.AsyncHook;
55-
private _contextRefs: Map<number, Reference<Context> | undefined> = new Map();
42+
private _contexts: Map<number, Context | undefined> = new Map();
43+
private _stack: Array<Context | undefined> = [];
5644

5745
constructor() {
5846
this._asyncHook = asyncHooks.createHook({
5947
init: this._init.bind(this),
48+
before: this._before.bind(this),
49+
after: this._after.bind(this),
6050
destroy: this._destroy.bind(this),
6151
promiseResolve: this._destroy.bind(this),
6252
});
6353
}
6454

6555
active(): Context {
66-
const ref = this._contextRefs.get(asyncHooks.executionAsyncId());
67-
return ref === undefined ? Context.ROOT_CONTEXT : ref.get();
56+
return this._stack[this._stack.length - 1] ?? Context.ROOT_CONTEXT;
6857
}
6958

7059
with<T extends (...args: unknown[]) => ReturnType<T>>(
7160
context: Context,
7261
fn: T
7362
): ReturnType<T> {
74-
const uid = asyncHooks.executionAsyncId();
75-
let ref = this._contextRefs.get(uid);
76-
let oldContext: Context | undefined = undefined;
77-
if (ref === undefined) {
78-
ref = new Reference(context);
79-
this._contextRefs.set(uid, ref);
80-
} else {
81-
oldContext = ref.get();
82-
ref.set(context);
83-
}
63+
this._enterContext(context);
8464
try {
8565
return fn();
8666
} finally {
87-
if (oldContext === undefined) {
88-
this._destroy(uid);
89-
} else {
90-
ref.set(oldContext);
91-
}
92-
}
93-
}
94-
95-
async withAsync<T extends Promise<any>, U extends (...args: unknown[]) => T>(
96-
context: Context,
97-
fn: U
98-
): Promise<T> {
99-
const uid = asyncHooks.executionAsyncId();
100-
let ref = this._contextRefs.get(uid);
101-
let oldContext: Context | undefined = undefined;
102-
if (ref === undefined) {
103-
ref = new Reference(context);
104-
this._contextRefs.set(uid, ref);
105-
} else {
106-
oldContext = ref.get();
107-
ref.set(context);
108-
}
109-
try {
110-
return await fn();
111-
} finally {
112-
if (oldContext === undefined) {
113-
this._destroy(uid);
114-
} else {
115-
ref.set(oldContext);
116-
}
67+
this._exitContext();
11768
}
11869
}
11970

120-
bind<T>(target: T, context: Context): T {
71+
bind<T>(target: T, context?: Context): T {
12172
// if no specific context to propagate is given, we use the current one
12273
if (context === undefined) {
12374
context = this.active();
@@ -137,7 +88,8 @@ export class AsyncHooksContextManager implements ContextManager {
13788

13889
disable(): this {
13990
this._asyncHook.disable();
140-
this._contextRefs.clear();
91+
this._contexts.clear();
92+
this._stack = [];
14193
return this;
14294
}
14395

@@ -156,6 +108,7 @@ export class AsyncHooksContextManager implements ContextManager {
156108
* It isn't possible to tell Typescript that contextWrapper is the same as T
157109
* so we forced to cast as any here.
158110
*/
111+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
159112
return contextWrapper as any;
160113
}
161114

@@ -271,9 +224,9 @@ export class AsyncHooksContextManager implements ContextManager {
271224
* @param uid id of the async context
272225
*/
273226
private _init(uid: number) {
274-
const ref = this._contextRefs.get(asyncHooks.executionAsyncId());
275-
if (ref !== undefined) {
276-
this._contextRefs.set(uid, ref);
227+
const context = this._stack[this._stack.length - 1];
228+
if (context !== undefined) {
229+
this._contexts.set(uid, context);
277230
}
278231
}
279232

@@ -283,6 +236,38 @@ export class AsyncHooksContextManager implements ContextManager {
283236
* @param uid uid of the async context
284237
*/
285238
private _destroy(uid: number) {
286-
this._contextRefs.delete(uid);
239+
this._contexts.delete(uid);
240+
}
241+
242+
/**
243+
* Before hook is called just beforing executing a async context.
244+
* @param uid uid of the async context
245+
*/
246+
private _before(uid: number) {
247+
const context = this._contexts.get(uid);
248+
if (context !== undefined) {
249+
this._enterContext(context);
250+
}
251+
}
252+
253+
/**
254+
* After hook is called just after completing the execution of a async context.
255+
*/
256+
private _after() {
257+
this._exitContext();
258+
}
259+
260+
/**
261+
* Set the given context as active
262+
*/
263+
private _enterContext(context: Context) {
264+
this._stack.push(context);
265+
}
266+
267+
/**
268+
* Remove the context at the root of the stack
269+
*/
270+
private _exitContext() {
271+
this._stack.pop();
287272
}
288273
}

0 commit comments

Comments
 (0)