From fd3fdd0e33383a704a00aec5044f7e4db6433e46 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 Sep 2023 11:12:22 +0200 Subject: [PATCH 1/2] backend-common: lazy root logger initialization Signed-off-by: Patrik Oldsberg --- .changeset/friendly-teachers-attend.md | 5 ++ .../src/logging/createRootLogger.ts | 57 ++++++++++++++++--- .../src/logging/globalLoggers.ts | 4 ++ 3 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 .changeset/friendly-teachers-attend.md diff --git a/.changeset/friendly-teachers-attend.md b/.changeset/friendly-teachers-attend.md new file mode 100644 index 0000000000..6d5c3cf745 --- /dev/null +++ b/.changeset/friendly-teachers-attend.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +The root logger is now initialized lazily, fixing a circular dependency issue with `@backstage/backend-app-api` that would result in `Cannot read properties of undefined (reading 'redacter')`. diff --git a/packages/backend-common/src/logging/createRootLogger.ts b/packages/backend-common/src/logging/createRootLogger.ts index 2e6fe20525..f9a3ed4ba0 100644 --- a/packages/backend-common/src/logging/createRootLogger.ts +++ b/packages/backend-common/src/logging/createRootLogger.ts @@ -17,12 +17,26 @@ import { WinstonLogger } from '@backstage/backend-app-api'; import { merge } from 'lodash'; import * as winston from 'winston'; -import { LoggerOptions } from 'winston'; +import { format, LoggerOptions } from 'winston'; import { setRootLogger } from './globalLoggers'; +import { TransformableInfo } from 'logform'; -const redacter = WinstonLogger.redacter(); +const getRedacter = (() => { + let redacter: ReturnType | undefined = + undefined; + return () => { + if (!redacter) { + redacter = WinstonLogger.redacter(); + } + return redacter; + }; +})(); -export const setRootLoggerRedactionList = redacter.add; +export const setRootLoggerRedactionList = ( + redactions: Iterable, +): void => { + getRedacter().add(redactions); +}; /** * A winston formatting function that finds occurrences of filteredKeys @@ -33,15 +47,44 @@ export const setRootLoggerRedactionList = redacter.add; export function redactWinstonLogLine( info: winston.Logform.TransformableInfo, ): winston.Logform.TransformableInfo { - return redacter.format.transform(info) as winston.Logform.TransformableInfo; + return getRedacter().format.transform( + info, + ) as winston.Logform.TransformableInfo; } +const colorizer = format.colorize(); + +// NOTE: This is a copy of the WinstonLogger.colorFormat to avoid a circular dependency /** * Creates a pretty printed winston log formatter. * * @public */ -export const coloredFormat = WinstonLogger.colorFormat(); +export const coloredFormat = format.combine( + format.timestamp(), + format.colorize({ + colors: { + timestamp: 'dim', + prefix: 'blue', + field: 'cyan', + debug: 'grey', + }, + }), + format.printf((info: TransformableInfo) => { + const { timestamp, level, message, plugin, service, ...fields } = info; + const prefix = plugin || service; + const timestampColor = colorizer.colorize('timestamp', timestamp); + const prefixColor = colorizer.colorize('prefix', prefix); + + const extraFields = Object.entries(fields) + .map( + ([key, value]) => `${colorizer.colorize('field', `${key}`)}=${value}`, + ) + .join(' '); + + return `${timestampColor} ${prefixColor} ${level} ${message} ${extraFields}`; + }), +); /** * Creates a default "root" logger. This also calls {@link setRootLogger} under @@ -64,7 +107,7 @@ export function createRootLogger( { level: env.LOG_LEVEL || 'info', format: winston.format.combine( - redacter.format, + getRedacter().format, env.NODE_ENV === 'production' ? winston.format.json() : WinstonLogger.colorFormat(), @@ -84,5 +127,3 @@ export function createRootLogger( return logger; } - -setRootLogger(createRootLogger()); diff --git a/packages/backend-common/src/logging/globalLoggers.ts b/packages/backend-common/src/logging/globalLoggers.ts index 7567deb646..8b473db211 100644 --- a/packages/backend-common/src/logging/globalLoggers.ts +++ b/packages/backend-common/src/logging/globalLoggers.ts @@ -15,6 +15,7 @@ */ import * as winston from 'winston'; +import { createRootLogger } from './createRootLogger'; /** * A logger that just throws away all messages. @@ -35,6 +36,9 @@ let rootLogger: winston.Logger; * @public */ export function getRootLogger(): winston.Logger { + if (!rootLogger) { + rootLogger = createRootLogger(); + } return rootLogger; } From 1309be15bb5c4b99ed33ae32d18f93b6255380b3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 Sep 2023 12:29:10 +0200 Subject: [PATCH 2/2] make sure root logger is loaded before FS mock in tests Signed-off-by: Patrik Oldsberg --- plugins/app-backend/src/service/appPlugin.test.ts | 4 ++++ .../actions/builtin/publish/githubPullRequest.test.ts | 5 ++++- .../actions/builtin/publish/gitlabMergeRequest.test.ts | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/plugins/app-backend/src/service/appPlugin.test.ts b/plugins/app-backend/src/service/appPlugin.test.ts index b6a972b4c7..09061b287d 100644 --- a/plugins/app-backend/src/service/appPlugin.test.ts +++ b/plugins/app-backend/src/service/appPlugin.test.ts @@ -19,6 +19,10 @@ import { resolve as resolvePath } from 'path'; import fetch from 'node-fetch'; import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; import { appPlugin } from './appPlugin'; +import { createRootLogger } from '@backstage/backend-common'; + +// Make sure root logger is initialized ahead of FS mock +createRootLogger(); describe('appPlugin', () => { beforeEach(() => { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index 82f8d22aea..b6987335f9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { getRootLogger } from '@backstage/backend-common'; +import { createRootLogger, getRootLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { GithubCredentialsProvider, @@ -33,6 +33,9 @@ import { OctokitWithPullRequestPluginClient, } from './githubPullRequest'; +// Make sure root logger is initialized ahead of FS mock +createRootLogger(); + const root = os.platform() === 'win32' ? 'C:\\root' : '/root'; const workspacePath = resolvePath(root, 'my-workspace'); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts index afb93b348a..6445d243b5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { getRootLogger } from '@backstage/backend-common'; +import { createRootLogger, getRootLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; @@ -23,6 +23,9 @@ import { resolve as resolvePath } from 'path'; import { Writable } from 'stream'; import { createPublishGitlabMergeRequestAction } from './gitlabMergeRequest'; +// Make sure root logger is initialized ahead of FS mock +createRootLogger(); + const root = os.platform() === 'win32' ? 'C:\\root' : '/root'; const workspacePath = resolvePath(root, 'my-workspace');