From 54f6a70313850c0ea958132a87d6cb00ed739c6d Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Thu, 18 Mar 2021 17:47:40 +0200 Subject: [PATCH 1/6] feat: add NET_TRANSPORT IPC attributes --- .../opentelemetry-semantic-conventions/src/trace/general.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/opentelemetry-semantic-conventions/src/trace/general.ts b/packages/opentelemetry-semantic-conventions/src/trace/general.ts index 281f9045ac..6fa64be3aa 100644 --- a/packages/opentelemetry-semantic-conventions/src/trace/general.ts +++ b/packages/opentelemetry-semantic-conventions/src/trace/general.ts @@ -36,4 +36,6 @@ export const GeneralAttribute = { IP_TCP: 'IP.TCP', IP_UDP: 'IP.UDP', INPROC: 'inproc', + PIPE: 'pipe', + UNIX: 'Unix', }; From 4f11b34bb61d76c9da29149fa9a119c0563113f5 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 23 Mar 2021 11:21:36 +0200 Subject: [PATCH 2/6] fix: http instrumentation context activation --- .../src/http.ts | 68 ++++++++----- .../src/patch.ts | 99 +++++++++++++++++++ .../src/types.ts | 2 +- .../test/functionals/http-enable.test.ts | 33 +++++-- 4 files changed, 171 insertions(+), 31 deletions(-) create mode 100644 packages/opentelemetry-instrumentation-http/src/patch.ts diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index 69d581c78a..bb9f336add 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -16,6 +16,7 @@ import { SpanStatusCode, context, + Context, propagation, Span, SpanKind, @@ -43,6 +44,7 @@ import { ParsedRequestOptions, ResponseEndArgs, } from './types'; +import { bindEmitter } from './patch'; import * as utils from './utils'; import { VERSION } from './version'; import { @@ -276,7 +278,9 @@ export class HttpInstrumentation extends InstrumentationBase { component: 'http' | 'https', request: http.ClientRequest, options: ParsedRequestOptions, - span: Span + span: Span, + requestContext: Context, + parentContext: Context ): http.ClientRequest { const hostname = options.hostname || @@ -291,7 +295,7 @@ export class HttpInstrumentation extends InstrumentationBase { this._callRequestHook(span, request); } - request.on( + request.prependListener( 'response', (response: http.IncomingMessage & { aborted?: boolean }) => { const responseAttributes = utils.getOutgoingRequestAttributesOnResponse( @@ -303,7 +307,10 @@ export class HttpInstrumentation extends InstrumentationBase { this._callResponseHook(span, response); } - context.bind(response); + bindEmitter(response, event => + event === 'data' ? requestContext : parentContext + ); + diag.debug('outgoingRequest on response()'); response.on('end', () => { diag.debug('outgoingRequest on end()'); @@ -427,6 +434,7 @@ export class HttpInstrumentation extends InstrumentationBase { response.end = originalEnd; // Cannot pass args of type ResponseEndArgs, const returned = safeExecuteInTheMiddle( + // eslint-disable-next-line @typescript-eslint/no-explicit-any () => response.end.apply(this, arguments as any), error => { if (error) { @@ -527,34 +535,46 @@ export class HttpInstrumentation extends InstrumentationBase { const spanOptions: SpanOptions = { kind: SpanKind.CLIENT, }; + const span = instrumentation._startHttpSpan(operationName, spanOptions); + + const parentContext = context.active(); + const requestContext = setSpan(parentContext, span); + if (!optionsParsed.headers) { optionsParsed.headers = {}; } - propagation.inject( - setSpan(context.active(), span), - optionsParsed.headers - ); + propagation.inject(requestContext, optionsParsed.headers); - const request: http.ClientRequest = safeExecuteInTheMiddle( - () => original.apply(this, [optionsParsed, ...args]), - error => { - if (error) { - utils.setSpanWithError(span, error); - instrumentation._closeHttpSpan(span); - throw error; - } + return context.with(requestContext, () => { + const cb = args[args.length - 1]; + if (typeof cb === 'function') { + args[args.length - 1] = context.bind(cb); } - ); - diag.debug('%s instrumentation outgoingRequest', component); - context.bind(request); - return instrumentation._traceClientRequest( - component, - request, - optionsParsed, - span - ); + const request: http.ClientRequest = safeExecuteInTheMiddle( + () => original.apply(this, [optionsParsed, ...args]), + error => { + if (error) { + utils.setSpanWithError(span, error); + instrumentation._closeHttpSpan(span); + throw error; + } + } + ); + + context.bind(request); + + diag.debug('%s instrumentation outgoingRequest', component); + return instrumentation._traceClientRequest( + component, + request, + optionsParsed, + span, + requestContext, + parentContext + ); + }); }; } diff --git a/packages/opentelemetry-instrumentation-http/src/patch.ts b/packages/opentelemetry-instrumentation-http/src/patch.ts new file mode 100644 index 0000000000..8dd04aa021 --- /dev/null +++ b/packages/opentelemetry-instrumentation-http/src/patch.ts @@ -0,0 +1,99 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { context, Context } from '@opentelemetry/api'; +import { EventEmitter } from 'events'; + +const LISTENER_SYMBOL = Symbol('OtHttpInstrumentationListeners'); + +const ADD_LISTENER_METHODS = [ + 'addListener' as const, + 'on' as const, + 'once' as const, + 'prependListener' as const, + 'prependOnceListener' as const, +]; + +interface ListenerMap { + [name: string]: WeakMap; +} + +function getListenerMap(emitter: EventEmitter): ListenerMap { + let map: ListenerMap = (emitter as never)[LISTENER_SYMBOL]; + if (map === undefined) { + map = {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (emitter as any)[LISTENER_SYMBOL] = map; + } + return map; +} + +export function bindEmitter( + emitter: E, + getContext: (event: string) => Context +): E { + function getPatchedAddFunction(original: Function) { + return function (this: never, event: string, listener: Function) { + const listenerMap = getListenerMap(emitter); + let listeners = listenerMap[event]; + if (listeners === undefined) { + listeners = new WeakMap(); + listenerMap[event] = listeners; + } + const patchedListener = context.bind(listener, getContext(event)); + listeners.set(listener, patchedListener); + return original.call(this, event, patchedListener); + }; + } + + function getPatchedRemoveListener(original: Function) { + return function (this: never, event: string, listener: Function) { + const listeners = getListenerMap(emitter)[event]; + if (listeners === undefined) { + return original.call(this, event, listener); + } + return original.call(this, event, listeners.get(listener) || listener); + }; + } + + function getPatchedRemoveAllListeners(original: Function) { + return function (this: never, event: string) { + delete getListenerMap(emitter)[event]; + return original.call(this, event); + }; + } + + for (const method of ADD_LISTENER_METHODS) { + if (emitter[method] !== undefined) { + emitter[method] = getPatchedAddFunction(emitter[method]); + } + } + + if (typeof emitter.removeListener === 'function') { + emitter.removeListener = getPatchedRemoveListener(emitter.removeListener); + } + + if (typeof emitter.off === 'function') { + emitter.off = getPatchedRemoveListener(emitter.off); + } + + if (typeof emitter.removeAllListeners === 'function') { + emitter.removeAllListeners = getPatchedRemoveAllListeners( + emitter.removeAllListeners + ); + } + + return emitter; +} diff --git a/packages/opentelemetry-instrumentation-http/src/types.ts b/packages/opentelemetry-instrumentation-http/src/types.ts index 94cfb52f60..a9927b35d7 100644 --- a/packages/opentelemetry-instrumentation-http/src/types.ts +++ b/packages/opentelemetry-instrumentation-http/src/types.ts @@ -44,7 +44,7 @@ export type ParsedRequestOptions = | http.RequestOptions; export type Http = typeof http; export type Https = typeof https; -/* tslint:disable-next-line:no-any */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any export type Func = (...args: any[]) => T; export type ResponseEndArgs = | [((() => void) | undefined)?] diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index 8f75058b3b..27ea611bc3 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -19,7 +19,7 @@ import { propagation, Span as ISpan, SpanKind, - getSpan, + getSpanContext, setSpan, } from '@opentelemetry/api'; import { NodeTracerProvider } from '@opentelemetry/node'; @@ -181,6 +181,7 @@ describe('HttpInstrumentation', () => { ); }); }); + describe('with good instrumentation options', () => { beforeEach(() => { memoryExporter.reset(); @@ -701,11 +702,31 @@ describe('HttpInstrumentation', () => { ); }); - it('should not set span as active in context for outgoing request', done => { - assert.deepStrictEqual(getSpan(context.active()), undefined); - http.get(`${protocol}://${hostname}:${serverPort}/test`, res => { - assert.deepStrictEqual(getSpan(context.active()), undefined); - done(); + it('should set span as active when receiving response or data callbacks', done => { + const tracer = provider.getTracer('test'); + const span = tracer.startSpan('parentSpan'); + const parentSpanId = span.context().spanId; + assert.notEqual(parentSpanId, undefined); + + context.with(setSpan(context.active(), span), () => { + http.get(`${protocol}://${hostname}:${serverPort}/test`, res => { + const requestSpanId = getSpanContext(context.active())?.spanId; + assert.notEqual(requestSpanId, parentSpanId); + + res.on('data', chunk => { + const dataSpanId = getSpanContext(context.active())?.spanId; + assert.notEqual(dataSpanId, parentSpanId); + assert.equal(dataSpanId, requestSpanId); + }); + + res.on('end', () => { + assert.equal( + getSpanContext(context.active())?.spanId, + parentSpanId + ); + done(); + }); + }); }); }); }); From d23893fd48d0b761451b811d5a2d1605fe6719d3 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 23 Mar 2021 14:43:12 +0200 Subject: [PATCH 3/6] test: add more tests --- .../test/functionals/http-enable.test.ts | 37 +++++- .../test/functionals/patch.test.ts | 110 ++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index 27ea611bc3..a66c00c5aa 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -702,9 +702,8 @@ describe('HttpInstrumentation', () => { ); }); - it('should set span as active when receiving response or data callbacks', done => { - const tracer = provider.getTracer('test'); - const span = tracer.startSpan('parentSpan'); + it('should set span as active when receiving response or data callbacks when using .get', done => { + const span = provider.getTracer('test').startSpan('parentSpan'); const parentSpanId = span.context().spanId; assert.notEqual(parentSpanId, undefined); @@ -729,6 +728,38 @@ describe('HttpInstrumentation', () => { }); }); }); + + it('should set span as active when receiving response or data callbacks when using .request', done => { + const span = provider.getTracer('test').startSpan('parentSpan'); + const parentSpanId = span.context().spanId; + assert.notEqual(parentSpanId, undefined); + + context.with(setSpan(context.active(), span), () => { + http + .request( + { host: hostname, port: serverPort, path: '/test' }, + res => { + const requestSpanId = getSpanContext(context.active())?.spanId; + assert.notEqual(requestSpanId, parentSpanId); + + res.on('data', chunk => { + const dataSpanId = getSpanContext(context.active())?.spanId; + assert.notEqual(dataSpanId, parentSpanId); + assert.equal(dataSpanId, requestSpanId); + }); + + res.on('end', () => { + assert.equal( + getSpanContext(context.active())?.spanId, + parentSpanId + ); + done(); + }); + } + ) + .end(); + }); + }); }); describe('with require parent span', () => { diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts new file mode 100644 index 0000000000..abfa72b519 --- /dev/null +++ b/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts @@ -0,0 +1,110 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import * as assert from 'assert'; +import { EventEmitter } from 'events'; +import { bindEmitter } from '../../src/patch'; +import { + context, + ContextManager, + getSpan, + getSpanContext, + ROOT_CONTEXT, + setSpan, +} from '@opentelemetry/api'; +import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; +import { NodeTracerProvider } from '@opentelemetry/node'; +import { + InMemorySpanExporter, + SimpleSpanProcessor, +} from '@opentelemetry/tracing'; + +const memoryExporter = new InMemorySpanExporter(); +const provider = new NodeTracerProvider(); +provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); + +const TEST_EVENT = 'custom'; + +describe('EventEmitter binding', () => { + let emitter: EventEmitter; + let contextManager: ContextManager; + + beforeEach(() => { + emitter = new EventEmitter(); + contextManager = new AsyncHooksContextManager().enable(); + context.setGlobalContextManager(contextManager); + }); + + it('binds correct context for registered listeners', () => { + const span = provider.getTracer('test').startSpan('myspan'); + const boundContext = setSpan(ROOT_CONTEXT, span); + + bindEmitter(emitter, ev => { + assert.strictEqual(ev, TEST_EVENT); + return boundContext; + }); + + const handledEvents = []; + + const check = (event: string) => () => { + assert.deepStrictEqual(getSpanContext(context.active()), span.context()); + handledEvents.push(event); + }; + + assert.notDeepStrictEqual(getSpanContext(context.active()), span.context()); + emitter.on(TEST_EVENT, check('on')); + emitter.addListener(TEST_EVENT, check('addListener')); + emitter.once(TEST_EVENT, check('once')); + emitter.prependListener(TEST_EVENT, check('prependListener')); + emitter.prependOnceListener(TEST_EVENT, check('prependOnceListener')); + + emitter.emit(TEST_EVENT); + + assert.deepStrictEqual(handledEvents, [ + 'prependOnceListener', + 'prependListener', + 'on', + 'addListener', + 'once', + ]); + }); + + it('is possible to remove a listener', () => { + bindEmitter(emitter, () => context.active()); + + const listener = () => assert.fail('listener should not be called'); + + emitter.removeListener(TEST_EVENT, listener); + emitter.on(TEST_EVENT, listener); + emitter.removeListener(TEST_EVENT, listener); + emitter.emit(TEST_EVENT); + }); + + it('is possible to remove all listeners', () => { + bindEmitter(emitter, () => context.active()); + + const listener = () => assert.fail('listener should not be called'); + + emitter.on(TEST_EVENT, listener); + emitter.once(TEST_EVENT, listener); + emitter.addListener(TEST_EVENT, listener); + emitter.prependListener(TEST_EVENT, listener); + emitter.prependOnceListener(TEST_EVENT, listener); + + emitter.removeAllListeners(TEST_EVENT); + + emitter.emit(TEST_EVENT); + }); +}); From 52a8333511356eb729664b903d3e7cbdc2c130ed Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 23 Mar 2021 14:44:35 +0200 Subject: [PATCH 4/6] fix: compilation --- .../test/functionals/patch.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts index abfa72b519..2ddc64e588 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts @@ -19,7 +19,6 @@ import { bindEmitter } from '../../src/patch'; import { context, ContextManager, - getSpan, getSpanContext, ROOT_CONTEXT, setSpan, @@ -56,7 +55,7 @@ describe('EventEmitter binding', () => { return boundContext; }); - const handledEvents = []; + const handledEvents: string[] = []; const check = (event: string) => () => { assert.deepStrictEqual(getSpanContext(context.active()), span.context()); From e9ce6a130b6530357aede25dff8f5719db2a17be Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Thu, 25 Mar 2021 21:22:49 +0200 Subject: [PATCH 5/6] fix: http: revert context activation outgoing context is still activated to enable connect instrumentation --- .../src/http.ts | 27 ++--- .../src/patch.ts | 99 ---------------- .../test/functionals/http-enable.test.ts | 65 ++--------- .../test/functionals/patch.test.ts | 109 ------------------ 4 files changed, 25 insertions(+), 275 deletions(-) delete mode 100644 packages/opentelemetry-instrumentation-http/src/patch.ts delete mode 100644 packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index bb9f336add..707d84bcc2 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -16,7 +16,6 @@ import { SpanStatusCode, context, - Context, propagation, Span, SpanKind, @@ -44,7 +43,6 @@ import { ParsedRequestOptions, ResponseEndArgs, } from './types'; -import { bindEmitter } from './patch'; import * as utils from './utils'; import { VERSION } from './version'; import { @@ -278,9 +276,7 @@ export class HttpInstrumentation extends InstrumentationBase { component: 'http' | 'https', request: http.ClientRequest, options: ParsedRequestOptions, - span: Span, - requestContext: Context, - parentContext: Context + span: Span ): http.ClientRequest { const hostname = options.hostname || @@ -295,6 +291,11 @@ export class HttpInstrumentation extends InstrumentationBase { this._callRequestHook(span, request); } + /* + * User 'response' event listeners can be added before our listener, + * force our listener to be the first, so response emitter is bound + * before any user listeners are added to it. + */ request.prependListener( 'response', (response: http.IncomingMessage & { aborted?: boolean }) => { @@ -307,9 +308,7 @@ export class HttpInstrumentation extends InstrumentationBase { this._callResponseHook(span, response); } - bindEmitter(response, event => - event === 'data' ? requestContext : parentContext - ); + context.bind(response); diag.debug('outgoingRequest on response()'); response.on('end', () => { @@ -547,9 +546,13 @@ export class HttpInstrumentation extends InstrumentationBase { propagation.inject(requestContext, optionsParsed.headers); return context.with(requestContext, () => { + /* + * The response callback is registered before ClientRequest is bound, + * thus it is needed to bind it before the function call. + */ const cb = args[args.length - 1]; if (typeof cb === 'function') { - args[args.length - 1] = context.bind(cb); + args[args.length - 1] = context.bind(cb, parentContext); } const request: http.ClientRequest = safeExecuteInTheMiddle( @@ -563,16 +566,14 @@ export class HttpInstrumentation extends InstrumentationBase { } ); - context.bind(request); + context.bind(request, parentContext); diag.debug('%s instrumentation outgoingRequest', component); return instrumentation._traceClientRequest( component, request, optionsParsed, - span, - requestContext, - parentContext + span ); }); }; diff --git a/packages/opentelemetry-instrumentation-http/src/patch.ts b/packages/opentelemetry-instrumentation-http/src/patch.ts deleted file mode 100644 index 8dd04aa021..0000000000 --- a/packages/opentelemetry-instrumentation-http/src/patch.ts +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { context, Context } from '@opentelemetry/api'; -import { EventEmitter } from 'events'; - -const LISTENER_SYMBOL = Symbol('OtHttpInstrumentationListeners'); - -const ADD_LISTENER_METHODS = [ - 'addListener' as const, - 'on' as const, - 'once' as const, - 'prependListener' as const, - 'prependOnceListener' as const, -]; - -interface ListenerMap { - [name: string]: WeakMap; -} - -function getListenerMap(emitter: EventEmitter): ListenerMap { - let map: ListenerMap = (emitter as never)[LISTENER_SYMBOL]; - if (map === undefined) { - map = {}; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (emitter as any)[LISTENER_SYMBOL] = map; - } - return map; -} - -export function bindEmitter( - emitter: E, - getContext: (event: string) => Context -): E { - function getPatchedAddFunction(original: Function) { - return function (this: never, event: string, listener: Function) { - const listenerMap = getListenerMap(emitter); - let listeners = listenerMap[event]; - if (listeners === undefined) { - listeners = new WeakMap(); - listenerMap[event] = listeners; - } - const patchedListener = context.bind(listener, getContext(event)); - listeners.set(listener, patchedListener); - return original.call(this, event, patchedListener); - }; - } - - function getPatchedRemoveListener(original: Function) { - return function (this: never, event: string, listener: Function) { - const listeners = getListenerMap(emitter)[event]; - if (listeners === undefined) { - return original.call(this, event, listener); - } - return original.call(this, event, listeners.get(listener) || listener); - }; - } - - function getPatchedRemoveAllListeners(original: Function) { - return function (this: never, event: string) { - delete getListenerMap(emitter)[event]; - return original.call(this, event); - }; - } - - for (const method of ADD_LISTENER_METHODS) { - if (emitter[method] !== undefined) { - emitter[method] = getPatchedAddFunction(emitter[method]); - } - } - - if (typeof emitter.removeListener === 'function') { - emitter.removeListener = getPatchedRemoveListener(emitter.removeListener); - } - - if (typeof emitter.off === 'function') { - emitter.off = getPatchedRemoveListener(emitter.off); - } - - if (typeof emitter.removeAllListeners === 'function') { - emitter.removeAllListeners = getPatchedRemoveAllListeners( - emitter.removeAllListeners - ); - } - - return emitter; -} diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index a66c00c5aa..736daa6233 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -19,7 +19,7 @@ import { propagation, Span as ISpan, SpanKind, - getSpanContext, + getSpan, setSpan, } from '@opentelemetry/api'; import { NodeTracerProvider } from '@opentelemetry/node'; @@ -702,62 +702,19 @@ describe('HttpInstrumentation', () => { ); }); - it('should set span as active when receiving response or data callbacks when using .get', done => { - const span = provider.getTracer('test').startSpan('parentSpan'); - const parentSpanId = span.context().spanId; - assert.notEqual(parentSpanId, undefined); + it('should not set span as active in context for outgoing request', done => { + assert.deepStrictEqual(getSpan(context.active()), undefined); + http.get(`${protocol}://${hostname}:${serverPort}/test`, res => { + assert.deepStrictEqual(getSpan(context.active()), undefined); - context.with(setSpan(context.active(), span), () => { - http.get(`${protocol}://${hostname}:${serverPort}/test`, res => { - const requestSpanId = getSpanContext(context.active())?.spanId; - assert.notEqual(requestSpanId, parentSpanId); - - res.on('data', chunk => { - const dataSpanId = getSpanContext(context.active())?.spanId; - assert.notEqual(dataSpanId, parentSpanId); - assert.equal(dataSpanId, requestSpanId); - }); - - res.on('end', () => { - assert.equal( - getSpanContext(context.active())?.spanId, - parentSpanId - ); - done(); - }); + res.on('data', () => { + assert.deepStrictEqual(getSpan(context.active()), undefined); }); - }); - }); - it('should set span as active when receiving response or data callbacks when using .request', done => { - const span = provider.getTracer('test').startSpan('parentSpan'); - const parentSpanId = span.context().spanId; - assert.notEqual(parentSpanId, undefined); - - context.with(setSpan(context.active(), span), () => { - http - .request( - { host: hostname, port: serverPort, path: '/test' }, - res => { - const requestSpanId = getSpanContext(context.active())?.spanId; - assert.notEqual(requestSpanId, parentSpanId); - - res.on('data', chunk => { - const dataSpanId = getSpanContext(context.active())?.spanId; - assert.notEqual(dataSpanId, parentSpanId); - assert.equal(dataSpanId, requestSpanId); - }); - - res.on('end', () => { - assert.equal( - getSpanContext(context.active())?.spanId, - parentSpanId - ); - done(); - }); - } - ) - .end(); + res.on('end', () => { + assert.deepStrictEqual(getSpan(context.active()), undefined); + done(); + }); }); }); }); diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts deleted file mode 100644 index 2ddc64e588..0000000000 --- a/packages/opentelemetry-instrumentation-http/test/functionals/patch.test.ts +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import * as assert from 'assert'; -import { EventEmitter } from 'events'; -import { bindEmitter } from '../../src/patch'; -import { - context, - ContextManager, - getSpanContext, - ROOT_CONTEXT, - setSpan, -} from '@opentelemetry/api'; -import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; -import { NodeTracerProvider } from '@opentelemetry/node'; -import { - InMemorySpanExporter, - SimpleSpanProcessor, -} from '@opentelemetry/tracing'; - -const memoryExporter = new InMemorySpanExporter(); -const provider = new NodeTracerProvider(); -provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); - -const TEST_EVENT = 'custom'; - -describe('EventEmitter binding', () => { - let emitter: EventEmitter; - let contextManager: ContextManager; - - beforeEach(() => { - emitter = new EventEmitter(); - contextManager = new AsyncHooksContextManager().enable(); - context.setGlobalContextManager(contextManager); - }); - - it('binds correct context for registered listeners', () => { - const span = provider.getTracer('test').startSpan('myspan'); - const boundContext = setSpan(ROOT_CONTEXT, span); - - bindEmitter(emitter, ev => { - assert.strictEqual(ev, TEST_EVENT); - return boundContext; - }); - - const handledEvents: string[] = []; - - const check = (event: string) => () => { - assert.deepStrictEqual(getSpanContext(context.active()), span.context()); - handledEvents.push(event); - }; - - assert.notDeepStrictEqual(getSpanContext(context.active()), span.context()); - emitter.on(TEST_EVENT, check('on')); - emitter.addListener(TEST_EVENT, check('addListener')); - emitter.once(TEST_EVENT, check('once')); - emitter.prependListener(TEST_EVENT, check('prependListener')); - emitter.prependOnceListener(TEST_EVENT, check('prependOnceListener')); - - emitter.emit(TEST_EVENT); - - assert.deepStrictEqual(handledEvents, [ - 'prependOnceListener', - 'prependListener', - 'on', - 'addListener', - 'once', - ]); - }); - - it('is possible to remove a listener', () => { - bindEmitter(emitter, () => context.active()); - - const listener = () => assert.fail('listener should not be called'); - - emitter.removeListener(TEST_EVENT, listener); - emitter.on(TEST_EVENT, listener); - emitter.removeListener(TEST_EVENT, listener); - emitter.emit(TEST_EVENT); - }); - - it('is possible to remove all listeners', () => { - bindEmitter(emitter, () => context.active()); - - const listener = () => assert.fail('listener should not be called'); - - emitter.on(TEST_EVENT, listener); - emitter.once(TEST_EVENT, listener); - emitter.addListener(TEST_EVENT, listener); - emitter.prependListener(TEST_EVENT, listener); - emitter.prependOnceListener(TEST_EVENT, listener); - - emitter.removeAllListeners(TEST_EVENT); - - emitter.emit(TEST_EVENT); - }); -}); From dd89f2e002fa97f95d8cbc77595de1655e15d9ad Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Thu, 25 Mar 2021 21:57:07 +0200 Subject: [PATCH 6/6] refactor: remove whitespace --- packages/opentelemetry-instrumentation-http/src/http.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index 707d84bcc2..6a23a4030b 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -309,7 +309,6 @@ export class HttpInstrumentation extends InstrumentationBase { } context.bind(response); - diag.debug('outgoingRequest on response()'); response.on('end', () => { diag.debug('outgoingRequest on end()'); @@ -534,7 +533,6 @@ export class HttpInstrumentation extends InstrumentationBase { const spanOptions: SpanOptions = { kind: SpanKind.CLIENT, }; - const span = instrumentation._startHttpSpan(operationName, spanOptions); const parentContext = context.active(); @@ -566,9 +564,8 @@ export class HttpInstrumentation extends InstrumentationBase { } ); - context.bind(request, parentContext); - diag.debug('%s instrumentation outgoingRequest', component); + context.bind(request, parentContext); return instrumentation._traceClientRequest( component, request,