From 4b2554b58bb907bc115d6f3ae16e3e468e8139b2 Mon Sep 17 00:00:00 2001 From: Rauno Viskus Date: Wed, 2 Jun 2021 12:59:29 +0300 Subject: [PATCH] feat: unify the signatures of bind and with --- .../src/AbstractAsyncHooksContextManager.ts | 16 ++++---- .../test/AsyncHooksContextManager.test.ts | 40 +++++++++---------- .../src/ZoneContextManager.ts | 18 ++++----- .../test/ZoneContextManager.test.ts | 18 ++++----- .../src/grpc-js/clientUtils.ts | 4 +- .../src/grpc-js/serverUtils.ts | 4 +- .../src/grpc/clientUtils.ts | 4 +- .../src/grpc/serverUtils.ts | 4 +- .../src/http.ts | 14 +++---- .../test/NodeTracerProvider.test.ts | 2 +- .../test/BasicTracerProvider.test.ts | 2 +- .../test/export/TestStackContextManager.ts | 2 +- .../src/StackContextManager.ts | 12 +++--- .../test/StackContextManager.test.ts | 14 +++---- 14 files changed, 77 insertions(+), 77 deletions(-) diff --git a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts index 0f540b62bb8..0b9a7d0bd1b 100644 --- a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts @@ -51,18 +51,18 @@ export abstract class AbstractAsyncHooksContextManager abstract disable(): this; - bind(target: T, context: Context = this.active()): T { + bind(context: Context, target: T): T { if (target instanceof EventEmitter) { - return this._bindEventEmitter(target, context); + return this._bindEventEmitter(context, target); } if (typeof target === 'function') { - return this._bindFunction(target, context); + return this._bindFunction(context, target); } return target; } - private _bindFunction(target: T, context: Context): T { + private _bindFunction(context: Context, target: T): T { const manager = this; const contextWrapper = function (this: never, ...args: unknown[]) { return manager.with(context, () => target.apply(this, args)); @@ -85,12 +85,12 @@ export abstract class AbstractAsyncHooksContextManager * By default, EventEmitter call their callback with their context, which we do * not want, instead we will bind a specific context to all callbacks that * go through it. - * @param ee EventEmitter an instance of EventEmitter to patch * @param context the context we want to bind + * @param ee EventEmitter an instance of EventEmitter to patch */ private _bindEventEmitter( - ee: T, - context: Context + context: Context, + ee: T ): T { const map = this._getPatchMap(ee); if (map !== undefined) return ee; @@ -180,7 +180,7 @@ export abstract class AbstractAsyncHooksContextManager listeners = new WeakMap(); map[event] = listeners; } - const patchedListener = contextManager.bind(listener, context); + const patchedListener = contextManager.bind(context, listener); // store a weak reference of the user listener to ours listeners.set(listener, patchedListener); return original.call(this, event, patchedListener); diff --git a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts index ba0a169e915..0b8e4868919 100644 --- a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts +++ b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts @@ -299,26 +299,26 @@ for (const contextManagerClass of [ describe('.bind(function)', () => { it('should return the same target (when enabled)', () => { const test = { a: 1 }; - assert.deepStrictEqual(contextManager.bind(test, ROOT_CONTEXT), test); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, test), test); }); it('should return the same target (when disabled)', () => { contextManager.disable(); const test = { a: 1 }; - assert.deepStrictEqual(contextManager.bind(test, ROOT_CONTEXT), test); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, test), test); contextManager.enable(); }); it('should return current context (when enabled)', done => { const context = ROOT_CONTEXT.setValue(key1, 1); - const fn = contextManager.bind(() => { + const fn = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), context, 'should have context' ); return done(); - }, context); + }); fn(); }); @@ -329,20 +329,20 @@ for (const contextManagerClass of [ it('should return current context (when disabled)', done => { contextManager.disable(); const context = ROOT_CONTEXT.setValue(key1, 1); - const fn = contextManager.bind(() => { + const fn = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), context, 'should have context' ); return done(); - }, context); + }); fn(); }); it('should fail to return current context with async op', done => { const context = ROOT_CONTEXT.setValue(key1, 1); - const fn = contextManager.bind(() => { + const fn = contextManager.bind(context, () => { assert.strictEqual(contextManager.active(), context); setTimeout(() => { assert.strictEqual( @@ -352,7 +352,7 @@ for (const contextManagerClass of [ ); return done(); }, 100); - }, context); + }); fn(); }); @@ -363,11 +363,11 @@ for (const contextManagerClass of [ const context = ROOT_CONTEXT.setValue(key1, 2); const otherContext = ROOT_CONTEXT.setValue(key1, 3); const fn = otherContextManager.bind( - contextManager.bind(() => { + otherContext, + contextManager.bind(context, () => { assert.strictEqual(contextManager.active(), context); assert.strictEqual(otherContextManager.active(), otherContext); - }, context), - otherContext + }) ); fn(); }); @@ -376,19 +376,19 @@ for (const contextManagerClass of [ describe('.bind(event-emitter)', () => { it('should return the same target (when enabled)', () => { const ee = new EventEmitter(); - assert.deepStrictEqual(contextManager.bind(ee, ROOT_CONTEXT), ee); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, ee), ee); }); it('should return the same target (when disabled)', () => { const ee = new EventEmitter(); contextManager.disable(); - assert.deepStrictEqual(contextManager.bind(ee, ROOT_CONTEXT), ee); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, ee), ee); }); it('should return current context and removeListener (when enabled)', done => { const ee = new EventEmitter(); const context = ROOT_CONTEXT.setValue(key1, 1); - const patchedEE = contextManager.bind(ee, context); + const patchedEE = contextManager.bind(context, ee); const handler = () => { assert.deepStrictEqual(contextManager.active(), context); patchedEE.removeListener('test', handler); @@ -403,7 +403,7 @@ for (const contextManagerClass of [ it('should return current context and removeAllListener (when enabled)', done => { const ee = new EventEmitter(); const context = ROOT_CONTEXT.setValue(key1, 1); - const patchedEE = contextManager.bind(ee, context); + const patchedEE = contextManager.bind(context, ee); const handler = () => { assert.deepStrictEqual(contextManager.active(), context); patchedEE.removeAllListeners('test'); @@ -418,7 +418,7 @@ for (const contextManagerClass of [ it('should return current context and removeAllListeners (when enabled)', done => { const ee = new EventEmitter(); const context = ROOT_CONTEXT.setValue(key1, 1); - const patchedEE = contextManager.bind(ee, context); + const patchedEE = contextManager.bind(context, ee); const handler = () => { assert.deepStrictEqual(contextManager.active(), context); patchedEE.removeAllListeners(); @@ -441,7 +441,7 @@ for (const contextManagerClass of [ contextManager.disable(); const ee = new EventEmitter(); const context = ROOT_CONTEXT.setValue(key1, 1); - const patchedEE = contextManager.bind(ee, context); + const patchedEE = contextManager.bind(context, ee); const handler = () => { assert.deepStrictEqual(contextManager.active(), context); patchedEE.removeListener('test', handler); @@ -456,7 +456,7 @@ for (const contextManagerClass of [ it('should not return current context with async op', done => { const ee = new EventEmitter(); const context = ROOT_CONTEXT.setValue(key1, 1); - const patchedEE = contextManager.bind(ee, context); + const patchedEE = contextManager.bind(context, ee); const handler = () => { assert.deepStrictEqual(contextManager.active(), context); setImmediate(() => { @@ -479,8 +479,8 @@ for (const contextManagerClass of [ const context = ROOT_CONTEXT.setValue(key1, 2); const otherContext = ROOT_CONTEXT.setValue(key1, 3); const patchedEE = otherContextManager.bind( - contextManager.bind(ee, context), - otherContext + otherContext, + contextManager.bind(context, ee), ); const handler = () => { assert.strictEqual(contextManager.active(), context); diff --git a/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts b/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts index edd5a0719e5..1cdaef951d8 100644 --- a/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts +++ b/packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts @@ -50,10 +50,10 @@ export class ZoneContextManager implements ContextManager { } /** - * @param target Function to be executed within the context * @param context A context (span) to be executed within target function + * @param target Function to be executed within the context */ - private _bindFunction(target: T, context: Context): T { + private _bindFunction(context: Context, target: T): T { const manager = this; const contextWrapper = function (this: any, ...args: unknown[]) { return manager.with(context, () => target.apply(this, args)); @@ -68,10 +68,10 @@ export class ZoneContextManager implements ContextManager { } /** - * @param obj target object on which the listeners will be patched * @param context A context (span) to be bind to target + * @param obj target object on which the listeners will be patched */ - private _bindListener(obj: T, context: Context): T { + private _bindListener(context: Context, obj: T): T { const target = (obj as unknown) as TargetWithEvents; if (target.__ot_listeners !== undefined) { return obj; @@ -153,7 +153,7 @@ export class ZoneContextManager implements ContextManager { listeners = new WeakMap(); target.__ot_listeners[event] = listeners; } - const patchedListener = contextManager.bind(listener, context); + const patchedListener = contextManager.bind(context, listener); // store a weak reference of the user listener to ours listeners.set(listener, patchedListener); return original.call(this, event, patchedListener, opts); @@ -202,18 +202,18 @@ export class ZoneContextManager implements ContextManager { /** * Binds a the certain context or the active one to the target function and then returns the target - * @param target * @param context A context (span) to be bind to target + * @param target */ - bind(target: T | TargetWithEvents, context: Context): T { + bind(context: Context, target: T | TargetWithEvents): T { // if no specific context to propagate is given, we use the current one if (context === undefined) { context = this.active(); } if (typeof target === 'function') { - return this._bindFunction(target, context); + return this._bindFunction(context, target); } else if (isListenerObject(target)) { - this._bindListener(target, context); + this._bindListener(context, target); } return (target as unknown) as T; } 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 43aba94e175..aea0dacb278 100644 --- a/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts +++ b/packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts @@ -251,46 +251,46 @@ describe('ZoneContextManager', () => { const ctx = ROOT_CONTEXT.setValue(key1, obj1); obj1.title = 'a2'; const obj2 = new Obj('b1'); - const wrapper: any = contextManager.bind(obj2.getTitle, ctx); + const wrapper: any = contextManager.bind(ctx, obj2.getTitle); assert.ok(wrapper(), 'a2'); }); it('should return the same target (when enabled)', () => { const test = { a: 1 }; - assert.deepStrictEqual(contextManager.bind(test, ROOT_CONTEXT), test); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, test), test); }); it('should return the same target (when disabled)', () => { contextManager.disable(); const test = { a: 1 }; - assert.deepStrictEqual(contextManager.bind(test, ROOT_CONTEXT), test); + assert.deepStrictEqual(contextManager.bind(ROOT_CONTEXT, test), test); contextManager.enable(); }); it('should return current context (when enabled)', done => { const context = ROOT_CONTEXT.setValue(key1, { a: 1 }); - const fn: any = contextManager.bind(() => { + const fn: any = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), context, 'should have context' ); return done(); - }, context); + }); fn(); }); it('should return root context (when disabled)', done => { contextManager.disable(); const context = ROOT_CONTEXT.setValue(key1, { a: 1 }); - const fn: any = contextManager.bind(() => { + const fn: any = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), ROOT_CONTEXT, 'should have context' ); return done(); - }, context); + }); fn(); }); @@ -298,7 +298,7 @@ describe('ZoneContextManager', () => { const ctx1 = ROOT_CONTEXT.setValue(key1, 1); const element = document.createElement('div'); - contextManager.bind(element, ctx1); + contextManager.bind(ctx1, element); element.addEventListener('click', () => { assert.strictEqual(contextManager.active(), ctx1); @@ -322,7 +322,7 @@ describe('ZoneContextManager', () => { const ctx1 = ROOT_CONTEXT.setValue(key1, 1); const element = document.createElement('div'); - contextManager.bind(element, ctx1); + contextManager.bind(ctx1, element); element.addEventListener('click', () => { assert.strictEqual(contextManager.active(), ctx1); diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts index 26906ad5b99..b2810cd1047 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts @@ -110,7 +110,7 @@ export function makeGrpcClientRemoteCall( span.end(); callback(err, res); }; - return context.bind(wrappedFn); + return context.bind(context.active(), wrappedFn); } return (span: Span) => { @@ -146,7 +146,7 @@ export function makeGrpcClientRemoteCall( spanEnded = true; } }; - context.bind(call); + context.bind(context.active(), call); call.on('error', (err: grpcJs.ServiceError) => { if (call[CALL_SPAN_ENDED]) { return; diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts index 8fad49fd1b8..f30c892a989 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts @@ -56,7 +56,7 @@ function serverStreamAndBidiHandler( } }; - context.bind(call); + context.bind(context.active(), call); call.on('finish', () => { // @grpc/js does not expose a way to check if this call also emitted an error, // e.g. call.status.code !== 0 @@ -143,7 +143,7 @@ function clientStreamAndUnaryHandler( return callback(err, value); }; - context.bind(call); + context.bind(context.active(), call); return (original as Function).call({}, call, patchedCallback); } diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts index 938533024c6..331e7443449 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts @@ -76,7 +76,7 @@ export const makeGrpcClientRemoteCall = function ( span.end(); callback(err, res); }; - return context.bind(wrappedFn); + return context.bind(context.active(), wrappedFn); } return (span: Span) => { @@ -118,7 +118,7 @@ export const makeGrpcClientRemoteCall = function ( spanEnded = true; } }; - context.bind(call); + context.bind(context.active(), call); ((call as unknown) as events.EventEmitter).on( 'error', (err: grpcTypes.ServiceError) => { diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts index 36e26a15309..5d88eeabcc1 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts @@ -71,7 +71,7 @@ export const clientStreamAndUnaryHandler = function ( return callback(err, value, trailer, flags); } - context.bind(call); + context.bind(context.active(), call); return (original as Function).call(self, call, patchedCallback); }; @@ -89,7 +89,7 @@ export const serverStreamAndBidiHandler = function ( } }; - context.bind(call); + context.bind(context.active(), call); call.on('finish', () => { span.setStatus(_grpcStatusCodeToSpanStatus(call.status.code)); span.setAttribute( diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index 25c22e7fbf2..a06861a26a0 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -308,7 +308,7 @@ export class HttpInstrumentation extends InstrumentationBase { this._callResponseHook(span, response); } - context.bind(response); + context.bind(context.active(), response); diag.debug('outgoingRequest on response()'); response.on('end', () => { diag.debug('outgoingRequest on end()'); @@ -389,8 +389,8 @@ export class HttpInstrumentation extends InstrumentationBase { ) ) { return context.with(suppressTracing(context.active()), () => { - context.bind(request); - context.bind(response); + context.bind(context.active(), request); + context.bind(context.active(), response); return original.apply(this, [event, ...args]); }); } @@ -419,8 +419,8 @@ export class HttpInstrumentation extends InstrumentationBase { return context.with( setRPCMetadata(trace.setSpan(ctx, span), rpcMetadata), () => { - context.bind(request); - context.bind(response); + context.bind(context.active(), request); + context.bind(context.active(), response); if (instrumentation._getConfig().requestHook) { instrumentation._callRequestHook(span, request); @@ -556,7 +556,7 @@ export class HttpInstrumentation extends InstrumentationBase { */ const cb = args[args.length - 1]; if (typeof cb === 'function') { - args[args.length - 1] = context.bind(cb, parentContext); + args[args.length - 1] = context.bind(parentContext, cb); } const request: http.ClientRequest = safeExecuteInTheMiddle( @@ -571,7 +571,7 @@ export class HttpInstrumentation extends InstrumentationBase { ); diag.debug('%s instrumentation outgoingRequest', component); - context.bind(request, parentContext); + context.bind(parentContext, request); return instrumentation._traceClientRequest( component, request, diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts index 4635fbf5594..7057965ab81 100644 --- a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts @@ -206,7 +206,7 @@ describe('NodeTracerProvider', () => { assert.deepStrictEqual(trace.getSpan(context.active()), span); return done(); }; - const patchedFn = context.bind(fn, trace.setSpan(context.active(), span)); + const patchedFn = context.bind(trace.setSpan(context.active(), span), fn); return patchedFn(); }); }); diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index 906fb79e9d4..6e96428b4de 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -505,7 +505,7 @@ describe('BasicTracerProvider', () => { assert.deepStrictEqual(trace.getSpan(context.active()), undefined); return done(); }; - const patchedFn = context.bind(fn, trace.setSpan(context.active(), span)); + const patchedFn = context.bind(trace.setSpan(context.active(), span), fn); return patchedFn(); }); }); diff --git a/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts index 5196c86d79e..3413a4aa1ed 100644 --- a/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts +++ b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts @@ -43,7 +43,7 @@ export class TestStackContextManager implements ContextManager { } } - bind(target: T, context?: Context): T { + bind(context: Context, target: T): T { throw new Error('Method not implemented.'); } diff --git a/packages/opentelemetry-web/src/StackContextManager.ts b/packages/opentelemetry-web/src/StackContextManager.ts index 0c87d6e7c60..d8fdf37717f 100644 --- a/packages/opentelemetry-web/src/StackContextManager.ts +++ b/packages/opentelemetry-web/src/StackContextManager.ts @@ -33,12 +33,12 @@ export class StackContextManager implements ContextManager { /** * - * @param target Function to be executed within the context * @param context + * @param target Function to be executed within the context */ private _bindFunction( - target: T, - context = ROOT_CONTEXT + context = ROOT_CONTEXT, + target: T ): T { const manager = this; const contextWrapper = function (this: unknown, ...args: unknown[]) { @@ -62,16 +62,16 @@ export class StackContextManager implements ContextManager { /** * Binds a the certain context or the active one to the target function and then returns the target - * @param target * @param context + * @param target */ - bind(target: T, context = ROOT_CONTEXT): T { + bind(context = ROOT_CONTEXT, target: T): T { // if no specific context to propagate is given, we use the current one if (context === undefined) { context = this.active(); } if (typeof target === 'function') { - return this._bindFunction(target, context); + return this._bindFunction(context, target); } return target; } diff --git a/packages/opentelemetry-web/test/StackContextManager.test.ts b/packages/opentelemetry-web/test/StackContextManager.test.ts index 2753260b866..1509780e7ba 100644 --- a/packages/opentelemetry-web/test/StackContextManager.test.ts +++ b/packages/opentelemetry-web/test/StackContextManager.test.ts @@ -176,46 +176,46 @@ describe('StackContextManager', () => { const ctx = ROOT_CONTEXT.setValue(key1, obj1); obj1.title = 'a2'; const obj2 = new Obj('b1'); - const wrapper: any = contextManager.bind(obj2.getTitle, ctx); + const wrapper: any = contextManager.bind(ctx, obj2.getTitle); assert.ok(wrapper(), 'a2'); }); it('should return the same target (when enabled)', () => { const test = ROOT_CONTEXT.setValue(key1, 1); - assert.deepStrictEqual(contextManager.bind(test), test); + assert.deepStrictEqual(contextManager.bind(contextManager.active(), test), test); }); it('should return the same target (when disabled)', () => { contextManager.disable(); const test = ROOT_CONTEXT.setValue(key1, 1); - assert.deepStrictEqual(contextManager.bind(test), test); + assert.deepStrictEqual(contextManager.bind(contextManager.active(), test), test); contextManager.enable(); }); it('should return current context (when enabled)', done => { const context = ROOT_CONTEXT.setValue(key1, 1); - const fn: any = contextManager.bind(() => { + const fn: any = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), context, 'should have context' ); return done(); - }, context); + }); fn(); }); it('should return current context (when disabled)', done => { contextManager.disable(); const context = ROOT_CONTEXT.setValue(key1, 1); - const fn: any = contextManager.bind(() => { + const fn: any = contextManager.bind(context, () => { assert.strictEqual( contextManager.active(), context, 'should have context' ); return done(); - }, context); + }); fn(); }); });