From 70a128ff5dca16060ca7c2cff943369ae9999c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= Date: Mon, 8 Feb 2021 17:12:34 +0100 Subject: [PATCH] feat: add support to forward args in context.with (#1883) Co-authored-by: Daniel Dyla --- packages/opentelemetry-api/src/api/context.ts | 12 ++++++---- .../opentelemetry-api/test/api/api.test.ts | 20 ++++++++++++++++ .../src/AbstractAsyncHooksContextManager.ts | 8 ++++--- .../src/AsyncHooksContextManager.ts | 10 ++++---- .../src/AsyncLocalStorageContextManager.ts | 11 +++++---- .../test/AsyncHooksContextManager.test.ts | 24 +++++++++++++++++++ .../src/NoopContextManager.ts | 10 ++++---- .../opentelemetry-context-base/src/types.ts | 10 +++++--- .../test/NoopContextManager.test.ts | 24 +++++++++++++++++++ .../src/ZoneContextManager.ts | 12 ++++++---- .../test/ZoneContextManager.test.ts | 24 +++++++++++++++++++ .../test/export/TestStackContextManager.ts | 10 ++++---- .../src/StackContextManager.ts | 12 ++++++---- .../test/StackContextManager.test.ts | 24 +++++++++++++++++++ 14 files changed, 177 insertions(+), 34 deletions(-) diff --git a/packages/opentelemetry-api/src/api/context.ts b/packages/opentelemetry-api/src/api/context.ts index ac5aff2cdd..fee5f30e49 100644 --- a/packages/opentelemetry-api/src/api/context.ts +++ b/packages/opentelemetry-api/src/api/context.ts @@ -78,12 +78,16 @@ export class ContextAPI { * * @param context context to be active during function execution * @param fn function to execute in a context + * @param thisArg optional receiver to be used for calling fn + * @param args optional arguments forwarded to fn */ - public with ReturnType>( + public with ReturnType>( context: Context, - fn: T - ): ReturnType { - return this._getContextManager().with(context, fn); + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { + return this._getContextManager().with(context, fn, thisArg, ...args); } /** diff --git a/packages/opentelemetry-api/test/api/api.test.ts b/packages/opentelemetry-api/test/api/api.test.ts index bb15fb02e9..b3e7728095 100644 --- a/packages/opentelemetry-api/test/api/api.test.ts +++ b/packages/opentelemetry-api/test/api/api.test.ts @@ -41,6 +41,26 @@ describe('API', () => { assert.strictEqual(typeof tracer, 'object'); }); + describe('Context', () => { + it('with should forward this, arguments and return value', () => { + function fnWithThis(this: string, a: string, b: number): string { + assert.strictEqual(this, 'that'); + assert.strictEqual(arguments.length, 2); + assert.strictEqual(a, 'one'); + assert.strictEqual(b, 2); + return 'done'; + } + + const res = context.with(ROOT_CONTEXT, fnWithThis, 'that', 'one', 2); + assert.strictEqual(res, 'done'); + + assert.strictEqual( + context.with(ROOT_CONTEXT, () => 3.14), + 3.14 + ); + }); + }); + describe('GlobalTracerProvider', () => { const spanContext = { traceId: 'd4cda95b652f4a1592b449d5929fda1b', diff --git a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts index 0198e1f213..85bf9c5dd6 100644 --- a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts @@ -42,10 +42,12 @@ export abstract class AbstractAsyncHooksContextManager implements ContextManager { abstract active(): Context; - abstract with ReturnType>( + abstract with ReturnType>( context: Context, - fn: T - ): ReturnType; + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType; abstract enable(): this; diff --git a/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts index 7b2d79593c..c7cc4d4801 100644 --- a/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts @@ -38,13 +38,15 @@ export class AsyncHooksContextManager extends AbstractAsyncHooksContextManager { return this._stack[this._stack.length - 1] ?? ROOT_CONTEXT; } - with ReturnType>( + with ReturnType>( context: Context, - fn: T - ): ReturnType { + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { this._enterContext(context); try { - return fn(); + return fn.call(thisArg!, ...args); } finally { this._exitContext(); } diff --git a/packages/opentelemetry-context-async-hooks/src/AsyncLocalStorageContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AsyncLocalStorageContextManager.ts index 1626af9dc5..87cd0941f5 100644 --- a/packages/opentelemetry-context-async-hooks/src/AsyncLocalStorageContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AsyncLocalStorageContextManager.ts @@ -30,11 +30,14 @@ export class AsyncLocalStorageContextManager extends AbstractAsyncHooksContextMa return this._asyncLocalStorage.getStore() ?? ROOT_CONTEXT; } - with ReturnType>( + with ReturnType>( context: Context, - fn: T - ): ReturnType { - return this._asyncLocalStorage.run(context, fn) as ReturnType; + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { + const cb = thisArg == null ? fn : fn.bind(thisArg); + return this._asyncLocalStorage.run(context, cb as any, ...args); } enable(): this { diff --git a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts index 4e9c0b19b6..32921f71c2 100644 --- a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts +++ b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts @@ -109,6 +109,30 @@ for (const contextManagerClass of [ return done(); }); + it('should forward this, arguments and return value', () => { + function fnWithThis(this: string, a: string, b: number): string { + assert.strictEqual(this, 'that'); + assert.strictEqual(arguments.length, 2); + assert.strictEqual(a, 'one'); + assert.strictEqual(b, 2); + return 'done'; + } + + const res = contextManager.with( + ROOT_CONTEXT, + fnWithThis, + 'that', + 'one', + 2 + ); + assert.strictEqual(res, 'done'); + + assert.strictEqual( + contextManager.with(ROOT_CONTEXT, () => 3.14), + 3.14 + ); + }); + it('should finally restore an old context', done => { const ctx1 = ROOT_CONTEXT.setValue(key1, 'ctx1'); const ctx2 = ROOT_CONTEXT.setValue(key1, 'ctx2'); diff --git a/packages/opentelemetry-context-base/src/NoopContextManager.ts b/packages/opentelemetry-context-base/src/NoopContextManager.ts index cb7fdd0de6..54589a0624 100644 --- a/packages/opentelemetry-context-base/src/NoopContextManager.ts +++ b/packages/opentelemetry-context-base/src/NoopContextManager.ts @@ -22,11 +22,13 @@ export class NoopContextManager implements types.ContextManager { return ROOT_CONTEXT; } - with ReturnType>( + with ReturnType>( _context: types.Context, - fn: T - ): ReturnType { - return fn(); + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { + return fn.call(thisArg, ...args); } bind(target: T, _context?: types.Context): T { diff --git a/packages/opentelemetry-context-base/src/types.ts b/packages/opentelemetry-context-base/src/types.ts index 20922441a7..af7a1ee15d 100644 --- a/packages/opentelemetry-context-base/src/types.ts +++ b/packages/opentelemetry-context-base/src/types.ts @@ -50,11 +50,15 @@ export interface ContextManager { * Run the fn callback with object set as the current active context * @param context Any object to set as the current active context * @param fn A callback to be immediately run within a specific context + * @param thisArg optional receiver to be used for calling fn + * @param args optional arguments forwarded to fn */ - with ReturnType>( + with ReturnType>( context: Context, - fn: T - ): ReturnType; + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType; /** * Bind an object as the current context (or a specific one) diff --git a/packages/opentelemetry-context-base/test/NoopContextManager.test.ts b/packages/opentelemetry-context-base/test/NoopContextManager.test.ts index 61f9ae7355..5528735adc 100644 --- a/packages/opentelemetry-context-base/test/NoopContextManager.test.ts +++ b/packages/opentelemetry-context-base/test/NoopContextManager.test.ts @@ -70,6 +70,30 @@ describe('NoopContextManager', () => { return done(); }); }); + + it('should forward this, arguments and return value', () => { + function fnWithThis(this: string, a: string, b: number): string { + assert.strictEqual(this, 'that'); + assert.strictEqual(arguments.length, 2); + assert.strictEqual(a, 'one'); + assert.strictEqual(b, 2); + return 'done'; + } + + const res = contextManager.with( + ROOT_CONTEXT, + fnWithThis, + 'that', + 'one', + 2 + ); + assert.strictEqual(res, 'done'); + + assert.strictEqual( + contextManager.with(ROOT_CONTEXT, () => 3.14), + 3.14 + ); + }); }); describe('.active()', () => { diff --git a/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts b/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts index d9bbe7f5d3..b04a27247a 100644 --- a/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts +++ b/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts @@ -244,15 +244,19 @@ export class ZoneContextManager implements ContextManager { * The context will be set as active * @param context A context (span) to be called with provided callback * @param fn Callback function + * @param thisArg optional receiver to be used for calling fn + * @param args optional arguments forwarded to fn */ - with ReturnType>( + with ReturnType>( context: Context | null, - fn: () => ReturnType - ): ReturnType { + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { const zoneName = this._createZoneName(); const newZone = this._createZone(zoneName, context); - return newZone.run(fn, context); + return newZone.run(fn, thisArg, args); } } diff --git a/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts b/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts index 4a21678999..b1f053ccdd 100644 --- a/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts +++ b/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts @@ -105,6 +105,30 @@ describe('ZoneContextManager', () => { return done(); }); + it('should forward this, arguments and return value', () => { + function fnWithThis(this: string, a: string, b: number): string { + assert.strictEqual(this, 'that'); + assert.strictEqual(arguments.length, 2); + assert.strictEqual(a, 'one'); + assert.strictEqual(b, 2); + return 'done'; + } + + const res = contextManager.with( + ROOT_CONTEXT, + fnWithThis, + 'that', + 'one', + 2 + ); + assert.strictEqual(res, 'done'); + + assert.strictEqual( + contextManager.with(ROOT_CONTEXT, () => 3.14), + 3.14 + ); + }); + it('should finally restore an old context, including the async task', done => { const ctx1 = ROOT_CONTEXT.setValue(key1, 'ctx1'); const ctx2 = ROOT_CONTEXT.setValue(key1, 'ctx2'); diff --git a/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts index 6612a34381..00c86d18c4 100644 --- a/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts +++ b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts @@ -33,13 +33,15 @@ export class TestStackContextManager implements ContextManager { return this._contextStack[this._contextStack.length - 1] ?? ROOT_CONTEXT; } - with ReturnType>( + with ReturnType>( context: Context, - fn: T - ): ReturnType { + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { this._contextStack.push(context); try { - return fn(); + return fn.call(thisArg, ...args); } finally { this._contextStack.pop(); } diff --git a/packages/opentelemetry-web/src/StackContextManager.ts b/packages/opentelemetry-web/src/StackContextManager.ts index a49609c0c8..b63598dff0 100644 --- a/packages/opentelemetry-web/src/StackContextManager.ts +++ b/packages/opentelemetry-web/src/StackContextManager.ts @@ -103,16 +103,20 @@ export class StackContextManager implements ContextManager { * The context will be set as active * @param context * @param fn Callback function + * @param thisArg optional receiver to be used for calling fn + * @param args optional arguments forwarded to fn */ - with ReturnType>( + with ReturnType>( context: Context | null, - fn: () => ReturnType - ): ReturnType { + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { const previousContext = this._currentContext; this._currentContext = context || ROOT_CONTEXT; try { - return fn(); + return fn.call(thisArg, ...args); } finally { this._currentContext = previousContext; } diff --git a/packages/opentelemetry-web/test/StackContextManager.test.ts b/packages/opentelemetry-web/test/StackContextManager.test.ts index 7a5742bab7..2753260b86 100644 --- a/packages/opentelemetry-web/test/StackContextManager.test.ts +++ b/packages/opentelemetry-web/test/StackContextManager.test.ts @@ -132,6 +132,30 @@ describe('StackContextManager', () => { }); assert.strictEqual(contextManager.active(), window); }); + + it('should forward this, arguments and return value', () => { + function fnWithThis(this: string, a: string, b: number): string { + assert.strictEqual(this, 'that'); + assert.strictEqual(arguments.length, 2); + assert.strictEqual(a, 'one'); + assert.strictEqual(b, 2); + return 'done'; + } + + const res = contextManager.with( + ROOT_CONTEXT, + fnWithThis, + 'that', + 'one', + 2 + ); + assert.strictEqual(res, 'done'); + + assert.strictEqual( + contextManager.with(ROOT_CONTEXT, () => 3.14), + 3.14 + ); + }); }); describe('.bind(function)', () => {