Adding ComponentLogger into instrumentations (#2261)

* chore: adding diag component logget to instrumentations * chore: fixing tests * chore: fixing tests
open-telemetry · Jun 9, 2021 · 4a1f2e5 · 4a1f2e5
1 parent 3d98420
commit 4a1f2e5
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 44 deletions.
diff --git 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:

diff --git 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<Record<string, unknown>> = {};
       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<Request | RequestInit> = {}
   ): 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
@@ -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
@@ -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');
+      instrumentation._diag.debug('patched gRPC server');
       return function register<RequestType, ResponseType>(
         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));
+              instrumentation._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');
+      instrumentation._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');
+    instrumentation._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');
+      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
@@ -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');
+      instrumentation._diag.debug('patched gRPC server');
 
       return function register<RequestType, ResponseType>(
         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));
+              instrumentation._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');
+      instrumentation._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');
+      instrumentation._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
@@ -15,7 +15,6 @@
  */
 import {
   context,
-  diag,
   INVALID_SPAN_CONTEXT,
   propagation,
   ROOT_CONTEXT,
@@ -86,7 +85,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       '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<Http> {
       },
       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<Http> {
       '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<Http> {
       },
       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<Http> {
         }
 
         context.bind(context.active(), 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<Http> {
       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<Http> {
         : '/';
       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<Http> {
         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<Http> {
           }
         );
 
-        diag.debug('%s instrumentation outgoingRequest', component);
+        instrumentation._diag.debug('%s instrumentation outgoingRequest', component);
         context.bind(parentContext, request);
         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
@@ -118,7 +118,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
       const headers: Partial<Record<string, unknown>> = {};
       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<XMLHttpRe
             return;
           }
 
-          api.diag.error('applyCustomAttributesOnSpan', error);
+          this._diag.error('applyCustomAttributesOnSpan', error);
         },
         true
       );
@@ -327,7 +327,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
     method: string
   ): api.Span | undefined {
     if (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 spanName = `HTTP ${method.toUpperCase()}`;
@@ -511,16 +511,16 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
    * implements enable function
    */
   override enable() {
-    api.diag.debug('applying patch to', this.moduleName, this.version);
+    this._diag.debug('applying patch to', this.moduleName, this.version);
 
     if (isWrapped(XMLHttpRequest.prototype.open)) {
       this._unwrap(XMLHttpRequest.prototype, 'open');
-      api.diag.debug('removing previous patch from method open');
+      this._diag.debug('removing previous patch from method open');
     }
 
     if (isWrapped(XMLHttpRequest.prototype.send)) {
       this._unwrap(XMLHttpRequest.prototype, 'send');
-      api.diag.debug('removing previous patch from method send');
+      this._diag.debug('removing previous patch from method send');
     }
 
     this._wrap(XMLHttpRequest.prototype, 'open', this._patchOpen());
@@ -531,7 +531,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
    * implements disable function
    */
   override disable() {
-    api.diag.debug('removing patch from', this.moduleName, this.version);
+    this._diag.debug('removing patch from', this.moduleName, this.version);
 
     this._unwrap(XMLHttpRequest.prototype, 'open');
     this._unwrap(XMLHttpRequest.prototype, 'send');

diff --git a/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts
@@ -589,7 +589,7 @@ describe('xhr', () => {
 
             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'
               );
             });