From cc72f9e058ba390b4c46a253020eff156c534af9 Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Thu, 27 Mar 2025 09:04:47 +0000 Subject: [PATCH] feat: disable prettifier for more prod env names This disables log prettification if the environment is either `prod` or `p`, which we use across some systems. This is done in the logger rather than app-info because that would be a breaking change. If app-info normalises the environment in a later version then we can switch logger back to expecting `production` only. Co-Authored-By: Matt Hinchliffe See-also: #1368 --- packages/logger/README.md | 2 +- packages/logger/lib/logger.js | 3 +- packages/logger/test/unit/lib/logger.spec.js | 43 ++++++++++++++------ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/packages/logger/README.md b/packages/logger/README.md index 068be3bf..38a06f8a 100644 --- a/packages/logger/README.md +++ b/packages/logger/README.md @@ -249,7 +249,7 @@ You can also use [built-in transforms](#built-in-transforms) to do things like m #### `options.withPrettifier` -Whether to send prettified logs if available. This option has no effect if you have the `NODE_ENV` environment variable set to either `production` or if you have not installed [pino-pretty](https://github.com/pinojs/pino-pretty#readme). See [local development usage](#local-development-usage) for more information. +Whether to send prettified logs if available. This option has no effect if you have the `NODE_ENV` environment variable set to `production` (`prod` or `p` also work) or if you have not installed [pino-pretty](https://github.com/pinojs/pino-pretty#readme). See [local development usage](#local-development-usage) for more information. Must be a `Boolean` and defaults to `true`. diff --git a/packages/logger/lib/logger.js b/packages/logger/lib/logger.js index 7857a921..9afabd0b 100644 --- a/packages/logger/lib/logger.js +++ b/packages/logger/lib/logger.js @@ -76,7 +76,8 @@ const prettificationAvailable = (() => { * @type {boolean} */ const prettificationAllowed = - appInfo.environment !== 'production' && appInfo.cloudProvider !== 'aws'; + !['production', 'prod', 'p'].includes(appInfo.environment.toLowerCase()) && + appInfo.cloudProvider !== 'aws'; /** * Class representing a logger. diff --git a/packages/logger/test/unit/lib/logger.spec.js b/packages/logger/test/unit/lib/logger.spec.js index cd9f509d..e031c5a3 100644 --- a/packages/logger/test/unit/lib/logger.spec.js +++ b/packages/logger/test/unit/lib/logger.spec.js @@ -991,6 +991,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { afterEach(() => { jest.unmock('pino-pretty'); + appInfo.environment = 'production'; }); it('configures the created Pino logger with prettification', () => { @@ -1025,6 +1026,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { afterEach(() => { jest.unmock('pino-pretty'); + appInfo.environment = 'production'; }); it('does not configure the created Pino logger with prettification', () => { @@ -1052,6 +1054,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { afterEach(() => { jest.unmock('pino-pretty'); + delete process.env.LOG_DISABLE_PRETTIFIER; }); it('does not configure the created Pino logger with prettification', () => { @@ -1060,29 +1063,35 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { }); }); - describe('when pino-pretty is installed and the environment is "production"', () => { + describe('when pino-pretty is installed and the environment is "production", "prod", or "p"', () => { beforeEach(() => { jest.mock('pino-pretty', () => 'mock pino pretty'); - appInfo.environment = 'production'; - - // We have to reset all modules because the checks for pino-pretty are done - // on module load for performance reasons. This resets the cache and reloads - // everything with a new environment. - jest.isolateModules(() => { - Logger = require('../../../lib/logger'); - }); - pino.mockClear(); - logger = new Logger(); + for (const environment of ['production', 'prod', 'p']) { + let Logger; + + appInfo.environment = environment; + // We have to reset all modules because the checks for pino-pretty are done + // on module load for performance reasons. This resets the cache and reloads + // everything with a new environment. + jest.isolateModules(() => { + Logger = require('../../../lib/logger'); + }); + + logger = new Logger(); + } }); afterEach(() => { jest.unmock('pino-pretty'); + appInfo.environment = 'production'; }); it('does not configure the created Pino logger with prettification', () => { - const pinoOptions = pino.mock.calls[0][0]; - expect(pinoOptions.transport).toBeUndefined(); + expect(pino).toHaveBeenCalledTimes(3); + for (const call of pino.mock.calls) { + expect(call[0].transport).toBeUndefined(); + } }); }); @@ -1104,6 +1113,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { afterEach(() => { jest.unmock('pino-pretty'); + appInfo.cloudProvider = null; }); it('does not configure the created Pino logger with prettification', () => { @@ -1114,6 +1124,9 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { describe('when pino-pretty is not installed and the environment is "development"', () => { beforeEach(() => { + jest.mock('pino-pretty', () => { + throw new Error('mock'); + }); appInfo.environment = 'development'; // We have to reset all modules because the checks for pino-pretty are done @@ -1127,6 +1140,10 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => { logger = new Logger(); }); + afterEach(() => { + appInfo.environment = 'production'; + }); + it('configures the created Pino logger without prettification', () => { const pinoOptions = pino.mock.calls[0][0]; expect(pinoOptions.transport).toBeUndefined();