Skip to content

Commit c9fdd1a

Browse files
authored
fix(instrumentation-pino): log-sending was losing records when the level was reduced (open-telemetry#2699)
Before this fix, if a Pino logger level was reduced (e.g. from 'info' to 'debug'), then log.debug('msg') would *not* work. The same thing would happen if a child logger was created at a lower-level than its parent. This issue was that PinoInstrumentation is internally using a pino multistream (https://getpino.io/#/docs/api?id=pino-multistream) and statically setting the level of those streams to the current Logger level. If the logger level was set lower, the multistream levels would end up filtering records at that original higher level. The fix is to set the stream levels to 0 to never filter out records. Closes: open-telemetry#2696
1 parent 0057f15 commit c9fdd1a

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,9 @@ export class PinoInstrumentation extends InstrumentationBase<PinoInstrumentation
109109
const origStream = logger[moduleExports.symbols.streamSym];
110110
logger[moduleExports.symbols.streamSym] = moduleExports.multistream(
111111
[
112-
{ level: logger.level, stream: origStream },
113-
{ level: logger.level, stream: otelStream },
112+
// Use level `0` to never not log a record given to the stream.
113+
{ level: 0, stream: origStream },
114+
{ level: 0, stream: otelStream },
114115
],
115116
{ levels: logger.levels.values }
116117
);

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,49 @@ describe('PinoInstrumentation', () => {
473473
});
474474
});
475475

476+
it('emits log records to a lower level after level change', () => {
477+
const logRecords = memExporter.getFinishedLogRecords();
478+
479+
logger.debug('first msg at debug');
480+
logger.trace('first msg at trace'); // Should *not* see this.
481+
482+
logger.level = 'trace';
483+
logger.debug('second msg at debug');
484+
logger.trace('second msg at trace'); // *Should* see this.
485+
486+
assert.strictEqual(logRecords.length, 3);
487+
assert.strictEqual(logRecords[0].severityNumber, SeverityNumber.DEBUG);
488+
assert.strictEqual(logRecords[0].severityText, 'debug');
489+
assert.strictEqual(logRecords[0].body, 'first msg at debug');
490+
assert.strictEqual(logRecords[1].severityNumber, SeverityNumber.DEBUG);
491+
assert.strictEqual(logRecords[1].severityText, 'debug');
492+
assert.strictEqual(logRecords[1].body, 'second msg at debug');
493+
assert.strictEqual(logRecords[2].severityNumber, SeverityNumber.TRACE);
494+
assert.strictEqual(logRecords[2].severityText, 'trace');
495+
assert.strictEqual(logRecords[2].body, 'second msg at trace');
496+
});
497+
498+
it('emits log records from child logger at lower level', () => {
499+
const logRecords = memExporter.getFinishedLogRecords();
500+
501+
const child = logger.child({ childField: 42 }, { level: 'trace' });
502+
503+
logger.debug('logger at debug level');
504+
logger.trace('logger at trace level'); // Should *not* see this one.
505+
child.debug('child at debug level');
506+
child.trace('child at trace level'); // *Should* see this one.
507+
assert.strictEqual(logRecords.length, 3);
508+
assert.strictEqual(logRecords[0].severityNumber, SeverityNumber.DEBUG);
509+
assert.strictEqual(logRecords[0].severityText, 'debug');
510+
assert.strictEqual(logRecords[0].body, 'logger at debug level');
511+
assert.strictEqual(logRecords[1].severityNumber, SeverityNumber.DEBUG);
512+
assert.strictEqual(logRecords[1].severityText, 'debug');
513+
assert.strictEqual(logRecords[1].body, 'child at debug level');
514+
assert.strictEqual(logRecords[2].severityNumber, SeverityNumber.TRACE);
515+
assert.strictEqual(logRecords[2].severityText, 'trace');
516+
assert.strictEqual(logRecords[2].body, 'child at trace level');
517+
});
518+
476519
it('does not emit to the Logs SDK if disableLogSending=true', () => {
477520
instrumentation.setConfig({ disableLogSending: true });
478521

0 commit comments

Comments
 (0)