diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index a565ef6bc0b..1ffc9bf4baa 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -14,7 +14,8 @@ All notable changes to this project will be documented in this file. ### :bug: (Bug Fix) -fix(api): prioritize `esnext` export condition as it is more specific [#5458](https://github.com/open-telemetry/opentelemetry-js/pull/5458) +* fix(api): prioritize `esnext` export condition as it is more specific [#5458](https://github.com/open-telemetry/opentelemetry-js/pull/5458) +* fix(api): update diag `consoleLogger` to use original console methods to prevent infinite loop when a console instrumentation is present [#6395](https://github.com/open-telemetry/opentelemetry-js/pull/6395) ### :books: (Refine Doc) diff --git a/api/src/diag/consoleLogger.ts b/api/src/diag/consoleLogger.ts index 1a4f2005311..9540e67b976 100644 --- a/api/src/diag/consoleLogger.ts +++ b/api/src/diag/consoleLogger.ts @@ -25,6 +25,30 @@ const consoleMap: { n: keyof DiagLogger; c: ConsoleMapKeys }[] = [ { n: 'verbose', c: 'trace' }, ]; +// Save original console methods at module load time, before any instrumentation +// can wrap them. This ensures DiagConsoleLogger calls the unwrapped originals. +// Exported for testing only — not part of the public API. +export const _originalConsoleMethods: Partial< + Record +> = {}; +if (typeof console !== 'undefined') { + const keys: (ConsoleMapKeys | 'log')[] = [ + 'error', + 'warn', + 'info', + 'debug', + 'trace', + 'log', + ]; + for (const key of keys) { + // eslint-disable-next-line no-console + if (typeof console[key] === 'function') { + // eslint-disable-next-line no-console + _originalConsoleMethods[key] = console[key]; + } + } +} + /** * A simple Immutable Console based diagnostic logger which will output any messages to the Console. * If you want to limit the amount of logging to a specific level or lower use the @@ -36,20 +60,23 @@ export class DiagConsoleLogger implements DiagLogger { constructor() { function _consoleFunc(funcName: ConsoleMapKeys): DiagLogFunction { return function (...args) { - if (console) { - // Some environments only expose the console when the F12 developer console is open + // Prefer original (pre-instrumentation) methods saved at module load time. + let theFunc = _originalConsoleMethods[funcName]; + // Some environments only expose the console when the F12 developer console is open + if (typeof theFunc !== 'function') { + theFunc = _originalConsoleMethods['log']; + } + // Fall back in case console was not available at module load time but became available later. + if (typeof theFunc !== 'function' && console) { // eslint-disable-next-line no-console - let theFunc = console[funcName]; + theFunc = console[funcName]; if (typeof theFunc !== 'function') { - // Not all environments support all functions // eslint-disable-next-line no-console theFunc = console.log; } - - // One last final check - if (typeof theFunc === 'function') { - return theFunc.apply(console, args); - } + } + if (typeof theFunc === 'function') { + return theFunc.apply(console, args); } }; } diff --git a/api/test/common/diag/consoleLogger.test.ts b/api/test/common/diag/consoleLogger.test.ts index 2db2141273a..bd69b9fd54a 100644 --- a/api/test/common/diag/consoleLogger.test.ts +++ b/api/test/common/diag/consoleLogger.test.ts @@ -17,7 +17,10 @@ /* eslint-disable no-console */ import * as assert from 'assert'; -import { DiagConsoleLogger } from '../../../src/diag/consoleLogger'; +import { + DiagConsoleLogger, + _originalConsoleMethods, +} from '../../../src/diag/consoleLogger'; export const diagLoggerFunctions = [ 'verbose', @@ -27,6 +30,15 @@ export const diagLoggerFunctions = [ 'error', ] as const; +const savedMethodKeys = [ + 'debug', + 'info', + 'warn', + 'error', + 'log', + 'trace', +] as const; + const consoleFuncs = [ 'debug', 'info', @@ -46,13 +58,15 @@ const expectedConsoleMap: { [n: string]: keyof Console } = { describe('DiagConsoleLogger', function () { const origConsole = console; - const orig: any = {}; + const origSavedMethods: any = {}; + const origConsoleMethods: any = {}; const calledArgs: any = {}; - // Save original functions - consoleFuncs.forEach(fName => { - orig[fName] = console[fName]; - calledArgs[fName] = null; + // Save the real values at suite setup + savedMethodKeys.forEach(key => { + origSavedMethods[key] = _originalConsoleMethods[key]; + origConsoleMethods[key] = console[key]; + calledArgs[key] = null; }); let canMockConsole = true; @@ -66,16 +80,22 @@ describe('DiagConsoleLogger', function () { } beforeEach(() => { - // mock Console - consoleFuncs.forEach(fName => { - console[fName] = (...args: unknown[]) => { - calledArgs[fName] = args; + // Replace saved originals with tracking functions + savedMethodKeys.forEach(key => { + (_originalConsoleMethods as any)[key] = (...args: unknown[]) => { + calledArgs[key] = args; }; }); }); afterEach(() => { - // restore + // Restore saved originals + savedMethodKeys.forEach(key => { + (_originalConsoleMethods as any)[key] = origSavedMethods[key]; + calledArgs[key] = null; + }); + + // Restore console if (canMockConsole) { try { // eslint-disable-next-line no-global-assign @@ -87,8 +107,7 @@ describe('DiagConsoleLogger', function () { } consoleFuncs.forEach(fName => { - calledArgs[fName] = null; - console[fName] = orig[fName]; + console[fName] = origConsoleMethods[fName]; }); }); @@ -109,7 +128,7 @@ describe('DiagConsoleLogger', function () { // Make sure only gets logged once let matches = 0; - consoleFuncs.forEach(cName => { + savedMethodKeys.forEach(cName => { if (cName !== expectedConsoleMap[fName]) { assert.deepStrictEqual(calledArgs[cName], null); } else { @@ -125,10 +144,9 @@ describe('DiagConsoleLogger', function () { assert.strictEqual(matches, 1, 'should log at least once'); }); - consoleFuncs.forEach(cName => { - it(`should log ${fName} message even when console doesn't support ${cName} call before construction`, function () { - // @ts-expect-error removing a console property is not allowed by types - console[cName] = undefined; + savedMethodKeys.forEach(cName => { + it(`should log ${fName} message even when saved original doesn't have ${cName} before construction`, function () { + _originalConsoleMethods[cName] = undefined; const consoleLogger: any = new DiagConsoleLogger(); consoleLogger[fName](`${fName} called %s`, 'param1'); if (cName !== expectedConsoleMap[fName]) { @@ -141,10 +159,9 @@ describe('DiagConsoleLogger', function () { } }); - it(`should log ${fName} message even when console doesn't support ${cName} call after construction`, function () { + it(`should log ${fName} message even when saved original doesn't have ${cName} after construction`, function () { const consoleLogger: any = new DiagConsoleLogger(); - // @ts-expect-error removing a console property is not allowed by types - console[cName] = undefined; + _originalConsoleMethods[cName] = undefined; consoleLogger[fName](`${fName} called %s`, 'param1'); if (cName !== expectedConsoleMap[fName]) { assert.deepStrictEqual(calledArgs[cName], null); @@ -158,28 +175,55 @@ describe('DiagConsoleLogger', function () { }); }); + diagLoggerFunctions.forEach(fName => { + it(`should fall back to live console when saved originals are empty for ${fName}`, function () { + // Clear all saved originals + savedMethodKeys.forEach(key => { + _originalConsoleMethods[key] = undefined; + }); + // Set up live console tracking + const liveCalledArgs: any = {}; + consoleFuncs.forEach(fn => { + liveCalledArgs[fn] = null; + console[fn] = (...args: unknown[]) => { + liveCalledArgs[fn] = args; + }; + }); + + const consoleLogger: any = new DiagConsoleLogger(); + consoleLogger[fName](`${fName} called %s`, 'param1'); + + const expectedConsoleFn = expectedConsoleMap[fName]; + assert.deepStrictEqual(liveCalledArgs[expectedConsoleFn], [ + `${fName} called %s`, + 'param1', + ]); + }); + }); + if (canMockConsole) { diagLoggerFunctions.forEach(fName => { - const cName = expectedConsoleMap[fName]; - it(`should not throw even when console is not supported for ${fName} call`, function () { + it(`should not throw even when both saved originals and console are not supported for ${fName} call`, function () { + savedMethodKeys.forEach(key => { + _originalConsoleMethods[key] = undefined; + }); // eslint-disable-next-line no-global-assign (console as any) = undefined; const consoleLogger: any = new DiagConsoleLogger(); consoleLogger[fName](`${fName} called %s`, 'param1'); - assert.deepStrictEqual(calledArgs[cName], null); - assert.deepStrictEqual(calledArgs.log, null); }); - it(`should not throw even when console is disabled after construction for ${fName} call`, function () { + it(`should not throw even when both saved originals and console are disabled after construction for ${fName} call`, function () { const consoleLogger: any = new DiagConsoleLogger(); + savedMethodKeys.forEach(key => { + _originalConsoleMethods[key] = undefined; + }); // eslint-disable-next-line no-global-assign (console as any) = undefined; consoleLogger[fName](`${fName} called %s`, 'param1'); - assert.deepStrictEqual(calledArgs[expectedConsoleMap[fName]], null); - assert.deepStrictEqual(calledArgs.log, null); }); - it(`should not throw even when console is invalid after construction for ${fName} call`, function () { + it(`should not throw even when saved originals are empty and console is invalid after construction for ${fName} call`, function () { const invalidConsole = { debug: 1, warn: 2, @@ -190,14 +234,15 @@ describe('DiagConsoleLogger', function () { }; const consoleLogger = new DiagConsoleLogger(); + savedMethodKeys.forEach(key => { + _originalConsoleMethods[key] = undefined; + }); // eslint-disable-next-line no-global-assign (console as any) = invalidConsole; consoleLogger[fName](`${fName} called %s`, 'param1'); - assert.deepStrictEqual(calledArgs[expectedConsoleMap[fName]], null); - assert.deepStrictEqual(calledArgs.log, null); }); - it(`should not throw even when console is invalid before construction for ${fName} call`, function () { + it(`should not throw even when saved originals are empty and console is invalid before construction for ${fName} call`, function () { const invalidConsole = { debug: 1, warn: 2, @@ -207,12 +252,13 @@ describe('DiagConsoleLogger', function () { log: 6, }; + savedMethodKeys.forEach(key => { + _originalConsoleMethods[key] = undefined; + }); // eslint-disable-next-line no-global-assign (console as any) = invalidConsole; const consoleLogger = new DiagConsoleLogger(); consoleLogger[fName](`${fName} called %s`, 'param1'); - assert.deepStrictEqual(calledArgs[expectedConsoleMap[fName]], null); - assert.deepStrictEqual(calledArgs.log, null); }); }); }