Skip to content

Commit aead95b

Browse files
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 <i-like-robots@users.noreply.github.com> See-also: #1368
1 parent 3bc489b commit aead95b

File tree

3 files changed

+33
-15
lines changed

3 files changed

+33
-15
lines changed

packages/logger/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ You can also use [built-in transforms](#built-in-transforms) to do things like m
249249
250250
#### `options.withPrettifier`
251251
252-
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.
252+
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.
253253
254254
Must be a `Boolean` and defaults to `true`.
255255

packages/logger/lib/logger.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ const prettificationAvailable = (() => {
7676
* @type {boolean}
7777
*/
7878
const prettificationAllowed =
79-
appInfo.environment !== 'production' && appInfo.cloudProvider !== 'aws';
79+
!['production', 'prod', 'p'].includes(appInfo.environment.toLowerCase()) &&
80+
appInfo.cloudProvider !== 'aws';
8081

8182
/**
8283
* Class representing a logger.

packages/logger/test/unit/lib/logger.spec.js

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
991991

992992
afterEach(() => {
993993
jest.unmock('pino-pretty');
994+
appInfo.environment = 'production';
994995
});
995996

996997
it('configures the created Pino logger with prettification', () => {
@@ -1025,6 +1026,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
10251026

10261027
afterEach(() => {
10271028
jest.unmock('pino-pretty');
1029+
appInfo.environment = 'production';
10281030
});
10291031

10301032
it('does not configure the created Pino logger with prettification', () => {
@@ -1052,6 +1054,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
10521054

10531055
afterEach(() => {
10541056
jest.unmock('pino-pretty');
1057+
delete process.env.LOG_DISABLE_PRETTIFIER;
10551058
});
10561059

10571060
it('does not configure the created Pino logger with prettification', () => {
@@ -1060,29 +1063,35 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
10601063
});
10611064
});
10621065

1063-
describe('when pino-pretty is installed and the environment is "production"', () => {
1066+
describe('when pino-pretty is installed and the environment is "production", "prod", or "p"', () => {
10641067
beforeEach(() => {
10651068
jest.mock('pino-pretty', () => 'mock pino pretty');
1066-
appInfo.environment = 'production';
1067-
1068-
// We have to reset all modules because the checks for pino-pretty are done
1069-
// on module load for performance reasons. This resets the cache and reloads
1070-
// everything with a new environment.
1071-
jest.isolateModules(() => {
1072-
Logger = require('../../../lib/logger');
1073-
});
1074-
10751069
pino.mockClear();
1076-
logger = new Logger();
1070+
for (const environment of ['production', 'prod', 'p']) {
1071+
let Logger;
1072+
1073+
appInfo.environment = environment;
1074+
// We have to reset all modules because the checks for pino-pretty are done
1075+
// on module load for performance reasons. This resets the cache and reloads
1076+
// everything with a new environment.
1077+
jest.isolateModules(() => {
1078+
Logger = require('../../../lib/logger');
1079+
});
1080+
1081+
logger = new Logger();
1082+
}
10771083
});
10781084

10791085
afterEach(() => {
10801086
jest.unmock('pino-pretty');
1087+
appInfo.environment = 'production';
10811088
});
10821089

10831090
it('does not configure the created Pino logger with prettification', () => {
1084-
const pinoOptions = pino.mock.calls[0][0];
1085-
expect(pinoOptions.transport).toBeUndefined();
1091+
expect(pino).toHaveBeenCalledTimes(3);
1092+
for (const call of pino.mock.calls) {
1093+
expect(call[0].transport).toBeUndefined();
1094+
}
10861095
});
10871096
});
10881097

@@ -1104,6 +1113,7 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
11041113

11051114
afterEach(() => {
11061115
jest.unmock('pino-pretty');
1116+
appInfo.cloudProvider = null;
11071117
});
11081118

11091119
it('does not configure the created Pino logger with prettification', () => {
@@ -1114,6 +1124,9 @@ describe('@dotcom-reliability-kit/logger/lib/logger', () => {
11141124

11151125
describe('when pino-pretty is not installed and the environment is "development"', () => {
11161126
beforeEach(() => {
1127+
jest.mock('pino-pretty', () => {
1128+
throw new Error('mock');
1129+
});
11171130
appInfo.environment = 'development';
11181131

11191132
// 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', () => {
11271140
logger = new Logger();
11281141
});
11291142

1143+
afterEach(() => {
1144+
appInfo.environment = 'production';
1145+
});
1146+
11301147
it('configures the created Pino logger without prettification', () => {
11311148
const pinoOptions = pino.mock.calls[0][0];
11321149
expect(pinoOptions.transport).toBeUndefined();

0 commit comments

Comments
 (0)