Skip to content

Commit

Permalink
feat: unify the signatures of bind and with
Browse files Browse the repository at this point in the history
  • Loading branch information
@rauno56
rauno56 committed Jun 2, 2021
1 parent 7fa4ff7 commit 4b2554b
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 77 deletions.
16 changes: 8 additions & 8 deletions packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts
View file
Expand Up @@ -51,18 +51,18 @@ export abstract class AbstractAsyncHooksContextManager

abstract disable(): this;

bind<T>(target: T, context: Context = this.active()): T {
bind<T>(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<T extends Function>(target: T, context: Context): T {
private _bindFunction<T extends Function>(context: Context, target: T): T {
const manager = this;
const contextWrapper = function (this: never, ...args: unknown[]) {
return manager.with(context, () => target.apply(this, args));
Expand All @@ -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<T extends EventEmitter>(
ee: T,
context: Context
context: Context,
ee: T
): T {
const map = this._getPatchMap(ee);
if (map !== undefined) return ee;
Expand Down Expand Up @@ -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);
Expand Down
40 changes: 20 additions & 20 deletions packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts
View file
Expand Up @@ -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();
});

Expand All @@ -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(
Expand All @@ -352,7 +352,7 @@ for (const contextManagerClass of [
);
return done();
}, 100);
}, context);
});
fn();
});

Expand All @@ -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();
});
Expand All @@ -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);
Expand All @@ -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');
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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(() => {
Expand All @@ -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);
Expand Down
18 changes: 9 additions & 9 deletions packages/opentelemetry-context-zone-peer-dep/src/ZoneContextManager.ts
View file
Expand Up @@ -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<T extends Function>(target: T, context: Context): T {
private _bindFunction<T extends Function>(context: Context, target: T): T {
const manager = this;
const contextWrapper = function (this: any, ...args: unknown[]) {
return manager.with(context, () => target.apply(this, args));
Expand All @@ -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<T>(obj: T, context: Context): T {
private _bindListener<T>(context: Context, obj: T): T {
const target = (obj as unknown) as TargetWithEvents;
if (target.__ot_listeners !== undefined) {
return obj;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<T>(target: T | TargetWithEvents, context: Context): T {
bind<T>(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;
}
Expand Down
18 changes: 9 additions & 9 deletions packages/opentelemetry-context-zone-peer-dep/test/ZoneContextManager.test.ts
View file
Expand Up @@ -251,54 +251,54 @@ 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();
});

it('should bind the the certain context to the target "addEventListener" function', done => {
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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts
View file
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts
View file
Expand Up @@ -56,7 +56,7 @@ function serverStreamAndBidiHandler<RequestType, ResponseType>(
}
};

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
Expand Down Expand Up @@ -143,7 +143,7 @@ function clientStreamAndUnaryHandler<RequestType, ResponseType>(
return callback(err, value);
};

context.bind(call);
context.bind(context.active(), call);
return (original as Function).call({}, call, patchedCallback);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts
View file
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts
View file
Expand Up @@ -71,7 +71,7 @@ export const clientStreamAndUnaryHandler = function <RequestType, ResponseType>(
return callback(err, value, trailer, flags);
}

context.bind(call);
context.bind(context.active(), call);
return (original as Function).call(self, call, patchedCallback);
};

Expand All @@ -89,7 +89,7 @@ export const serverStreamAndBidiHandler = function <RequestType, ResponseType>(
}
};

context.bind(call);
context.bind(context.active(), call);
call.on('finish', () => {
span.setStatus(_grpcStatusCodeToSpanStatus(call.status.code));
span.setAttribute(
Expand Down

0 comments on commit 4b2554b

Please sign in to comment.