From 277e4f7e17abea7ee8b9f571dd564396315ec20b Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 7 Jun 2021 23:01:59 +0200 Subject: [PATCH 1/3] chore: adding diag component logget to instrumentations --- .../src/fetch.ts | 8 +++---- .../test/fetch.test.ts | 2 +- .../src/grpc-js/index.ts | 15 ++++++------ .../src/grpc/index.ts | 17 +++++++------- .../src/http.ts | 23 +++++++++---------- .../src/xhr.ts | 14 +++++------ .../test/xhr.test.ts | 2 +- .../src/instrumentation.ts | 13 ++++++++++- 8 files changed, 51 insertions(+), 43 deletions(-) diff --git a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 42b45b2d8e2..a8860947ba9 100644 --- a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -159,7 +159,7 @@ export class FetchInstrumentation extends InstrumentationBase< const headers: Partial> = {}; api.propagation.inject(api.context.active(), headers); if (Object.keys(headers).length > 0) { - api.diag.debug('headers inject skipped due to CORS policy'); + this._diag.debug('headers inject skipped due to CORS policy'); } return; } @@ -198,7 +198,7 @@ export class FetchInstrumentation extends InstrumentationBase< options: Partial = {} ): api.Span | undefined { if (core.isUrlIgnored(url, this._getConfig().ignoreUrls)) { - api.diag.debug('ignoring span as url matches ignored url'); + this._diag.debug('ignoring span as url matches ignored url'); return; } const method = (options.method || 'GET').toUpperCase(); @@ -417,7 +417,7 @@ export class FetchInstrumentation extends InstrumentationBase< return; } - api.diag.error('applyCustomAttributesOnSpan', error); + this._diag.error('applyCustomAttributesOnSpan', error); }, true ); @@ -461,7 +461,7 @@ export class FetchInstrumentation extends InstrumentationBase< override enable() { if (isWrapped(window.fetch)) { this._unwrap(window, 'fetch'); - api.diag.debug('removing previous patch for constructor'); + this._diag.debug('removing previous patch for constructor'); } this._wrap(window, 'fetch', this._patchConstructor()); } diff --git a/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index 017aea8056d..4c6de800c67 100644 --- a/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -571,7 +571,7 @@ describe('fetch', () => { }); it('should debug info that injecting headers was skipped', () => { assert.strictEqual( - spyDebug.lastCall.args[0], + spyDebug.lastCall.args[1], 'headers inject skipped due to CORS policy' ); }); diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts index 753ac06b7b4..66f77291fc8 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts @@ -35,7 +35,6 @@ import { } from './types'; import { context, - diag, propagation, ROOT_CONTEXT, SpanOptions, @@ -76,7 +75,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { '@grpc/grpc-js', ['1.*'], (moduleExports, version) => { - diag.debug(`Applying patch for @grpc/grpc-js@${version}`); + this._diag.debug(`Applying patch for @grpc/grpc-js@${version}`); if (isWrapped(moduleExports.Server.prototype.register)) { this._unwrap(moduleExports.Server.prototype, 'register'); } @@ -115,7 +114,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { }, (moduleExports, version) => { if (moduleExports === undefined) return; - diag.debug(`Removing patch for @grpc/grpc-js@${version}`); + this._diag.debug(`Removing patch for @grpc/grpc-js@${version}`); this._unwrap(moduleExports.Server.prototype, 'register'); this._unwrap(moduleExports, 'makeClientConstructor'); @@ -136,7 +135,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { const instrumentation = this; return (originalRegister: ServerRegisterFunction) => { const config = this._config; - diag.debug('patched gRPC server'); + this._diag.debug('patched gRPC server'); return function register( this: grpcJs.Server, name: string, @@ -186,7 +185,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { kind: SpanKind.SERVER, }; - diag.debug('patch func: %s', JSON.stringify(spanOptions)); + this._diag.debug('patch func: %s', JSON.stringify(spanOptions)); context.with( propagation.extract(ROOT_CONTEXT, call.metadata, { @@ -231,7 +230,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { ) => MakeClientConstructorFunction { const instrumentation = this; return (original: MakeClientConstructorFunction) => { - diag.debug('patching client'); + this._diag.debug('patching client'); return function makeClientConstructor( this: typeof grpcJs.Client, methods: grpcJs.ServiceDefinition, @@ -255,7 +254,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { */ private _patchLoadPackageDefinition(grpcClient: typeof grpcJs) { const instrumentation = this; - diag.debug('patching loadPackageDefinition'); + this._diag.debug('patching loadPackageDefinition'); return (original: typeof grpcJs.loadPackageDefinition) => { return function patchedLoadPackageDefinition( this: null, @@ -279,7 +278,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { ): (original: GrpcClientFunc) => () => EventEmitter { const instrumentation = this; return (original: GrpcClientFunc) => { - diag.debug('patch all client methods'); + this._diag.debug('patch all client methods'); return function clientMethodTrace(this: grpcJs.Client) { const name = `grpc.${original.path.replace('/', '')}`; const args = [...arguments]; diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts index ade2ea3aabc..54fa743547a 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts @@ -31,7 +31,6 @@ import { import { GrpcInstrumentationConfig } from '../types'; import { context, - diag, propagation, SpanOptions, SpanKind, @@ -75,7 +74,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< 'grpc', ['1.*'], (moduleExports, version) => { - diag.debug(`Applying patch for grpc@${version}`); + this._diag.debug(`Applying patch for grpc@${version}`); grpcClient = moduleExports; if (isWrapped(moduleExports.Server.prototype.register)) { @@ -99,7 +98,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< }, (moduleExports, version) => { if (moduleExports === undefined) return; - diag.debug(`Removing patch for grpc@${version}`); + this._diag.debug(`Removing patch for grpc@${version}`); this._unwrap(moduleExports.Server.prototype, 'register'); }, @@ -113,7 +112,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< moduleExports: GrpcInternalClientTypes, version?: string ) => { - diag.debug(`Applying internal patch for grpc@${version}`); + this._diag.debug(`Applying internal patch for grpc@${version}`); if (isWrapped(moduleExports.makeClientConstructor)) { this._unwrap(moduleExports, 'makeClientConstructor'); } @@ -125,7 +124,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< version?: string ) => { if (moduleExports === undefined) return; - diag.debug(`Removing internal patch for grpc@${version}`); + this._diag.debug(`Removing internal patch for grpc@${version}`); this._unwrap(moduleExports, 'makeClientConstructor'); }; return [ @@ -147,7 +146,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _patchServer(grpcModule: typeof grpcTypes) { const instrumentation = this; return (originalRegister: typeof grpcTypes.Server.prototype.register) => { - diag.debug('patched gRPC server'); + this._diag.debug('patched gRPC server'); return function register( this: grpcTypes.Server & { handlers: any }, @@ -191,7 +190,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< kind: SpanKind.SERVER, }; - diag.debug('patch func: %s', JSON.stringify(spanOptions)); + this._diag.debug('patch func: %s', JSON.stringify(spanOptions)); context.with( propagation.extract(context.active(), call.metadata, { @@ -243,7 +242,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _patchClient() { const instrumentation = this; return (original: typeof grpcTypes.makeGenericClientConstructor): never => { - diag.debug('patching client'); + this._diag.debug('patching client'); return function makeClientConstructor( this: typeof grpcTypes.Client, methods: { [key: string]: { originalName?: string } }, @@ -288,7 +287,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _getPatchedClientMethods() { const instrumentation = this; return (original: GrpcClientFunc) => { - diag.debug('patch all client methods'); + this._diag.debug('patch all client methods'); return function clientMethodTrace(this: grpcTypes.Client) { const name = `grpc.${(original.path as string | undefined)?.replace( '/', diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index b3a2db9931f..945c5c552d0 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -15,7 +15,6 @@ */ import { context, - diag, INVALID_SPAN_CONTEXT, propagation, ROOT_CONTEXT, @@ -86,7 +85,7 @@ export class HttpInstrumentation extends InstrumentationBase { 'http', ['*'], moduleExports => { - diag.debug(`Applying patch for http@${this._version}`); + this._diag.debug(`Applying patch for http@${this._version}`); if (isWrapped(moduleExports.request)) { this._unwrap(moduleExports, 'request'); } @@ -115,7 +114,7 @@ export class HttpInstrumentation extends InstrumentationBase { }, moduleExports => { if (moduleExports === undefined) return; - diag.debug(`Removing patch for http@${this._version}`); + this._diag.debug(`Removing patch for http@${this._version}`); this._unwrap(moduleExports, 'request'); this._unwrap(moduleExports, 'get'); @@ -129,7 +128,7 @@ export class HttpInstrumentation extends InstrumentationBase { 'https', ['*'], moduleExports => { - diag.debug(`Applying patch for https@${this._version}`); + this._diag.debug(`Applying patch for https@${this._version}`); if (isWrapped(moduleExports.request)) { this._unwrap(moduleExports, 'request'); } @@ -158,7 +157,7 @@ export class HttpInstrumentation extends InstrumentationBase { }, moduleExports => { if (moduleExports === undefined) return; - diag.debug(`Removing patch for https@${this._version}`); + this._diag.debug(`Removing patch for https@${this._version}`); this._unwrap(moduleExports, 'request'); this._unwrap(moduleExports, 'get'); @@ -309,9 +308,9 @@ export class HttpInstrumentation extends InstrumentationBase { } context.bind(response); - diag.debug('outgoingRequest on response()'); + this._diag.debug('outgoingRequest on response()'); response.on('end', () => { - diag.debug('outgoingRequest on end()'); + this._diag.debug('outgoingRequest on end()'); let status: SpanStatus; if (response.aborted && !response.complete) { @@ -353,7 +352,7 @@ export class HttpInstrumentation extends InstrumentationBase { this._closeHttpSpan(span); }); - diag.debug('http.ClientRequest return request'); + this._diag.debug('http.ClientRequest return request'); return request; } @@ -379,13 +378,13 @@ export class HttpInstrumentation extends InstrumentationBase { : '/'; const method = request.method || 'GET'; - diag.debug('%s instrumentation incomingRequest', component); + instrumentation._diag.debug('%s instrumentation incomingRequest', component); if ( utils.isIgnored( pathname, instrumentation._getConfig().ignoreIncomingPaths, - (e: Error) => diag.error('caught ignoreIncomingPaths error: ', e) + (e: Error) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e) ) ) { return context.with(suppressTracing(context.active()), () => { @@ -529,7 +528,7 @@ export class HttpInstrumentation extends InstrumentationBase { utils.isIgnored( origin + pathname, instrumentation._getConfig().ignoreOutgoingUrls, - (e: Error) => diag.error('caught ignoreOutgoingUrls error: ', e) + (e: Error) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e) ) ) { return original.apply(this, [optionsParsed, ...args]); @@ -570,7 +569,7 @@ export class HttpInstrumentation extends InstrumentationBase { } ); - diag.debug('%s instrumentation outgoingRequest', component); + instrumentation._diag.debug('%s instrumentation outgoingRequest', component); context.bind(request, parentContext); return instrumentation._traceClientRequest( component, diff --git a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 25ff68ec18b..1b5ade2660d 100644 --- a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -118,7 +118,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase> = {}; api.propagation.inject(api.context.active(), headers); if (Object.keys(headers).length > 0) { - api.diag.debug('headers inject skipped due to CORS policy'); + this._diag.debug('headers inject skipped due to CORS policy'); } return; } @@ -190,7 +190,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { it('should debug info that injecting headers was skipped', () => { assert.strictEqual( - spyDebug.lastCall.args[0], + spyDebug.lastCall.args[1], 'headers inject skipped due to CORS policy' ); }); diff --git a/packages/opentelemetry-instrumentation/src/instrumentation.ts b/packages/opentelemetry-instrumentation/src/instrumentation.ts index c1389ee12f1..56779a5e64c 100644 --- a/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { TracerProvider, Tracer, trace } from '@opentelemetry/api'; +import { + diag, + DiagLogger, + trace, + Tracer, + TracerProvider, +} from '@opentelemetry/api'; import { Meter, MeterProvider, metrics } from '@opentelemetry/api-metrics'; import * as shimmer from 'shimmer'; import { InstrumentationModuleDefinition } from './platform/node'; @@ -29,6 +35,7 @@ export abstract class InstrumentationAbstract private _tracer: Tracer; private _meter: Meter; + protected _diag: DiagLogger; constructor( public readonly instrumentationName: string, @@ -40,6 +47,10 @@ export abstract class InstrumentationAbstract ...config, }; + this._diag = diag.createComponentLogger({ + namespace: instrumentationName, + }); + this._tracer = trace.getTracer(instrumentationName, instrumentationVersion); this._meter = metrics.getMeter(instrumentationName, instrumentationVersion); From 6133abb68797615c5a77040ae19b6ddec8327cf4 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Tue, 8 Jun 2021 18:48:43 +0200 Subject: [PATCH 2/3] chore: fixing tests --- .../src/grpc-js/index.ts | 10 +++++----- .../src/grpc/index.ts | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts index 66f77291fc8..0cf2114891e 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts @@ -135,7 +135,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { const instrumentation = this; return (originalRegister: ServerRegisterFunction) => { const config = this._config; - this._diag.debug('patched gRPC server'); + instrumentation._diag.debug('patched gRPC server'); return function register( this: grpcJs.Server, name: string, @@ -185,7 +185,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { kind: SpanKind.SERVER, }; - this._diag.debug('patch func: %s', JSON.stringify(spanOptions)); + instrumentation._diag.debug('patch func: %s', JSON.stringify(spanOptions)); context.with( propagation.extract(ROOT_CONTEXT, call.metadata, { @@ -230,7 +230,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { ) => MakeClientConstructorFunction { const instrumentation = this; return (original: MakeClientConstructorFunction) => { - this._diag.debug('patching client'); + instrumentation._diag.debug('patching client'); return function makeClientConstructor( this: typeof grpcJs.Client, methods: grpcJs.ServiceDefinition, @@ -254,7 +254,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { */ private _patchLoadPackageDefinition(grpcClient: typeof grpcJs) { const instrumentation = this; - this._diag.debug('patching loadPackageDefinition'); + instrumentation._diag.debug('patching loadPackageDefinition'); return (original: typeof grpcJs.loadPackageDefinition) => { return function patchedLoadPackageDefinition( this: null, @@ -278,7 +278,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase { ): (original: GrpcClientFunc) => () => EventEmitter { const instrumentation = this; return (original: GrpcClientFunc) => { - this._diag.debug('patch all client methods'); + instrumentation._diag.debug('patch all client methods'); return function clientMethodTrace(this: grpcJs.Client) { const name = `grpc.${original.path.replace('/', '')}`; const args = [...arguments]; diff --git a/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts b/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts index 54fa743547a..87486ef157f 100644 --- a/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts +++ b/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts @@ -146,7 +146,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _patchServer(grpcModule: typeof grpcTypes) { const instrumentation = this; return (originalRegister: typeof grpcTypes.Server.prototype.register) => { - this._diag.debug('patched gRPC server'); + instrumentation._diag.debug('patched gRPC server'); return function register( this: grpcTypes.Server & { handlers: any }, @@ -190,7 +190,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< kind: SpanKind.SERVER, }; - this._diag.debug('patch func: %s', JSON.stringify(spanOptions)); + instrumentation._diag.debug('patch func: %s', JSON.stringify(spanOptions)); context.with( propagation.extract(context.active(), call.metadata, { @@ -242,7 +242,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _patchClient() { const instrumentation = this; return (original: typeof grpcTypes.makeGenericClientConstructor): never => { - this._diag.debug('patching client'); + instrumentation._diag.debug('patching client'); return function makeClientConstructor( this: typeof grpcTypes.Client, methods: { [key: string]: { originalName?: string } }, @@ -287,7 +287,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase< private _getPatchedClientMethods() { const instrumentation = this; return (original: GrpcClientFunc) => { - this._diag.debug('patch all client methods'); + instrumentation._diag.debug('patch all client methods'); return function clientMethodTrace(this: grpcTypes.Client) { const name = `grpc.${(original.path as string | undefined)?.replace( '/', From ee3bcccc8852f6570d29e0edeffad8899b54e87c Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Wed, 9 Jun 2021 18:30:57 +0200 Subject: [PATCH 3/3] chore: fixing tests --- .github/workflows/unit-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index a88efbbcc09..5c9b3f0faae 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -53,7 +53,7 @@ jobs: run: npm run test - name: Report Coverage run: npm run codecov - if: ${{ matrix.container }} == 'node:14' + if: ${{ matrix.container == 'node:14' }} browser-tests: runs-on: ubuntu-latest container: