feat: improve pino instrumentation by patching additional exports of the same function (open-telemetry#1108)

Co-authored-by: Gerhard Stöbich <[email protected]>
Co-authored-by: Rauno Viskus <[email protected]>

Author: 3 people

.github/workflows/unit-test.yml

@@ -28,6 +28,7 @@ jobs:
           - node: "12"
             lerna-extra-args: >-
               --ignore @opentelemetry/instrumentation-redis-4
+              --ignore @opentelemetry/instrumentation-pino
     runs-on: ubuntu-latest
     services:
       memcached:

plugins/node/opentelemetry-instrumentation-pino/.tav.yml

@@ -1,6 +1,13 @@
 pino:
-  versions: "^7.11.0 || 7.8.0 || 7.2.0 || ^6.13.1 || 5.17.0 || 5.14.0"
-  commands: npm run test
+  - versions: "^8.0.0 || ^7.11.0 || 7.8.0 || 7.2.0 || ^6.13.1 || 5.17.0 || 5.14.0"
+    node: ">=14"
+    commands: npm run test
 
-  # Fix missing `contrib-test-utils` package
-  pretest: npm run --prefix ../../../ lerna:link
+    # Fix missing `contrib-test-utils` package
+    pretest: npm run --prefix ../../../ lerna:link
+  - versions: "^7.11.0 || 7.8.0 || 7.2.0 || ^6.13.1 || 5.17.0 || 5.14.0"
+    node: ">=8 <14"
+    commands: npm run test
+
+    # Fix missing `contrib-test-utils` package
+    pretest: npm run --prefix ../../../ lerna:link

plugins/node/opentelemetry-instrumentation-pino/README.md

@@ -55,7 +55,7 @@ When no span context is active or the span context is invalid, injection is skip
 
 ### Supported versions
 
-`>=5.14.0 <8`
+`>=5.14.0 <9`
 
 ## Useful links
 

plugins/node/opentelemetry-instrumentation-pino/package.json

@@ -58,16 +58,16 @@
     "gts": "3.1.0",
     "mocha": "7.2.0",
     "nyc": "15.1.0",
+    "pino": "8.3.1",
     "rimraf": "3.0.2",
+    "semver": "7.3.5",
     "sinon": "14.0.0",
     "test-all-versions": "5.0.1",
     "ts-mocha": "10.0.0",
     "typescript": "4.3.5"
   },
   "dependencies": {
-    "@opentelemetry/instrumentation": "^0.32.0",
-    "pino": "7.10.0",
-    "semver": "^7.3.5"
+    "@opentelemetry/instrumentation": "^0.32.0"
   },
   "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-pino#readme"
 }

plugins/node/opentelemetry-instrumentation-pino/src/instrumentation.ts

@@ -26,11 +26,10 @@ import {
   InstrumentationNodeModuleDefinition,
   safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
-import { Pino, PinoInstrumentationConfig } from './types';
+import { PinoInstrumentationConfig } from './types';
 import { VERSION } from './version';
-import type { pino } from 'pino';
 
-const pinoVersions = ['>=5.14.0 <8'];
+const pinoVersions = ['>=5.14.0 <9'];
 
 export class PinoInstrumentation extends InstrumentationBase {
   constructor(config: PinoInstrumentationConfig = {}) {
@@ -39,7 +38,7 @@ export class PinoInstrumentation extends InstrumentationBase {
 
   protected init() {
     return [
-      new InstrumentationNodeModuleDefinition<Pino>(
+      new InstrumentationNodeModuleDefinition<any>(
         'pino',
         pinoVersions,
         pinoModule => {
@@ -61,17 +60,22 @@ export class PinoInstrumentation extends InstrumentationBase {
                 args.splice(0, 0, {
                   mixin: instrumentation._getMixinFunction(),
                 });
-                return pinoModule(...(args as Parameters<Pino>));
+                return pinoModule(...args);
               }
             }
 
-            args[0] = instrumentation._combineOptions(
-              args[0] as pino.LoggerOptions
-            );
+            args[0] = instrumentation._combineOptions(args[0]);
 
-            return pinoModule(...(args as Parameters<Pino>));
+            return pinoModule(...args);
           }, pinoModule);
 
+          if (typeof patchedPino.pino === 'function') {
+            patchedPino.pino = patchedPino;
+          }
+          if (typeof patchedPino.default === 'function') {
+            patchedPino.default = patchedPino;
+          }
+
           return patchedPino;
         }
       ),
@@ -135,7 +139,7 @@ export class PinoInstrumentation extends InstrumentationBase {
     };
   }
 
-  private _combineOptions(options?: pino.LoggerOptions) {
+  private _combineOptions(options?: any) {
     if (options === undefined) {
       return { mixin: this._getMixinFunction() };
     }

plugins/node/opentelemetry-instrumentation-pino/src/types.ts

@@ -16,7 +16,6 @@
 
 import { Span } from '@opentelemetry/api';
 import { InstrumentationConfig } from '@opentelemetry/instrumentation';
-import type { pino } from 'pino';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export type LogHookFunction = (
@@ -28,5 +27,3 @@ export type LogHookFunction = (
 export interface PinoInstrumentationConfig extends InstrumentationConfig {
   logHook?: LogHookFunction;
 }
-
-export type Pino = typeof pino;

plugins/node/opentelemetry-instrumentation-pino/test/pino.test.ts

@@ -75,11 +75,16 @@ describe('PinoInstrumentation', () => {
     return record;
   }
 
-  function init() {
+  function init(importType: 'global' | 'default' | 'pino' = 'global') {
     stream = new Writable();
     stream._write = () => {};
     writeSpy = sinon.spy(stream, 'write');
-    logger = pino(stream);
+    if (importType === 'global') {
+      logger = pino(stream);
+    } else {
+      // @ts-expect-error the same function reexported
+      logger = pino[importType](stream);
+    }
   }
 
   before(() => {
@@ -100,6 +105,30 @@ describe('PinoInstrumentation', () => {
       });
     });
 
+    it('injects span context to records in default export', function () {
+      // @ts-expect-error the same function reexported
+      if (!pino.default) {
+        this.skip();
+      }
+      init('default');
+      const span = tracer.startSpan('abc');
+      context.with(trace.setSpan(context.active(), span), () => {
+        testInjection(span);
+      });
+    });
+
+    it('injects span context to records in named export', function () {
+      // @ts-expect-error the same function reexported
+      if (!pino.pino) {
+        this.skip();
+      }
+      init('pino');
+      const span = tracer.startSpan('abc');
+      context.with(trace.setSpan(context.active(), span), () => {
+        testInjection(span);
+      });
+    });
+
     it('injects span context to child logger records', () => {
       const span = tracer.startSpan('abc');
       context.with(trace.setSpan(context.active(), span), () => {
@@ -243,7 +272,7 @@ describe('PinoInstrumentation', () => {
       instrumentation.enable();
     });
 
-    beforeEach(init);
+    beforeEach(() => init());
 
     it('does not inject span context', () => {
       const span = tracer.startSpan('abc');