impl disableLogCorrelation config; undo the previous commit so that l…

…og-correlation *will* follow the live instr enable/disable state

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

@@ -121,35 +121,31 @@ export class PinoInstrumentation extends InstrumentationBase {
         const otelLogger = logs.getLogger(PACKAGE_NAME, PACKAGE_VERSION);
 
         const patchedPino = Object.assign((...args: unknown[]) => {
-          // XXX instrumentatin.getConfig(); usage
-          // const config = instrumentation.getConfig();
-          const optLogCorrelation = true;
-          const optLogSending = true;
+          const config = instrumentation.getConfig();
           const isEnabled = instrumentation.isEnabled();
 
           const logger = moduleExports(...args);
 
-          if (isEnabled && optLogCorrelation) {
-            // Note: If the Pino logger is configured with `nestedKey`, then
-            // the `trace_id` et al fields added by `otelMixin` will be nested
-            // as well. https://getpino.io/#/docs/api?id=mixin-function
-            const otelMixin = instrumentation._getMixinFunction();
-            const mixinSym = moduleExports.symbols.mixinSym;
-            const origMixin = logger[mixinSym];
-            if (origMixin === undefined) {
-              // console.warn('XXX adding otelMixin', )
-              logger[mixinSym] = otelMixin;
-            } else {
-              logger[mixinSym] = (ctx: object, level: number) => {
-                return Object.assign(
-                  otelMixin(ctx, level),
-                  origMixin(ctx, level)
-                );
-              };
-            }
+          // Setup "log correlation" -- injection of `trace_id` et al fields.
+          // Note: If the Pino logger is configured with `nestedKey`, then
+          // the `trace_id` et al fields added by `otelMixin` will be nested
+          // as well. https://getpino.io/#/docs/api?id=mixin-function
+          const otelMixin = instrumentation._getMixinFunction();
+          const mixinSym = moduleExports.symbols.mixinSym;
+          const origMixin = logger[mixinSym];
+          if (origMixin === undefined) {
+            logger[mixinSym] = otelMixin;
+          } else {
+            logger[mixinSym] = (ctx: object, level: number) => {
+              return Object.assign(
+                otelMixin(ctx, level),
+                origMixin(ctx, level)
+              );
+            };
           }
 
-          if (isEnabled && optLogSending) {
+          // Setup "log sending" -- sending log records to the Logs API.
+          if (isEnabled && !config.disableLogSending) {
             // Shim the Pino logger's `stream.write` for log sending.
             const stream = logger[moduleExports.symbols.streamSym];
             if (typeof stream.write !== 'function') {
@@ -261,6 +257,13 @@ export class PinoInstrumentation extends InstrumentationBase {
   private _getMixinFunction() {
     const instrumentation = this;
     return function otelMixin(_context: object, level: number) {
+      if (
+        !instrumentation.isEnabled() ||
+        instrumentation.getConfig().disableLogCorrelation
+      ) {
+        return {};
+      }
+
       const span = trace.getSpan(context.active());
 
       if (!span) {

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

@@ -25,9 +25,31 @@ export type LogHookFunction = (
 ) => void;
 
 export interface PinoInstrumentationConfig extends InstrumentationConfig {
+  /**
+   * Whether to disable the automatic sending of log records to the
+   * OpenTelemetry Logs SDK.
+   * @default false
+   */
+  disableLogSending?: boolean;
+
+  /**
+   * Whether to disable the injection trace-context fields, and possibly other
+   * fields from `logHook()`, into log records for log correlation.
+   * @default false
+   */
+  disableLogCorrelation?: boolean;
+
+  /**
+   * A function that allows injecting additional fields in log records. It is
+   * called, as `logHook(span, record)`, for each log record emitted in a valid
+   * span context. It requires `disableLogCorrelation` to be false.
+   */
   logHook?: LogHookFunction;
 
-  /** Configure the names of field injected into logs when there is span context available.  */
+  /**
+   * Configure the names of field injected into logs when there is span context
+   * available.
+   */
   logKeys?: {
     traceId: string;
     spanId: string;

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

@@ -111,6 +111,18 @@ describe('PinoInstrumentation', () => {
         assert.strictEqual(record['resource.service.name'], undefined);
       });
     });
+
+    it('injects span context once re-enabled', () => {
+      instrumentation.enable();
+      tracer.startActiveSpan('abc', span => {
+        logger.info('a message');
+        span.end();
+
+        sinon.assert.calledOnce(writeSpy);
+        const record = JSON.parse(writeSpy.firstCall.args[0].toString());
+        assertRecord(record, span);
+      });
+    });
   });
 
   describe('log correlation', () => {
@@ -235,6 +247,33 @@ describe('PinoInstrumentation', () => {
       });
     });
 
+    it('does not inject or call logHook if disableLogCorrelation=true', () => {
+      instrumentation.setConfig({
+        disableLogCorrelation: true,
+        logHook: (_span, record) => {
+          record['resource.service.name'] = 'test-service';
+        },
+      });
+      tracer.startActiveSpan('abc', span => {
+        logger.info('foo');
+        span.end();
+
+        sinon.assert.calledOnce(writeSpy);
+        const record = JSON.parse(writeSpy.firstCall.args[0].toString());
+        assert.strictEqual('foo', record['msg']);
+        assert.strictEqual(record['trace_id'], undefined);
+        assert.strictEqual(record['span_id'], undefined);
+        assert.strictEqual(record['trace_flags'], undefined);
+        assert.strictEqual(record['resource.service.name'], undefined);
+        // XXX restore this
+        // assert.strictEqual(
+        //   memExporter.getFinishedLogRecords().length,
+        //   1,
+        //   'Log sending still works'
+        // );
+      });
+    });
+
     it('instrumentation of `pino.default(...)` works', function () {
       if (!pino.default) {
         this.skip();