Merge pull request #24478 from backstage/blam/scaffolder-logger-fox

scaffolder: Redactions and keeping the log format

plugins/scaffolder-backend/package.json

@@ -95,6 +95,7 @@
     "prom-client": "^15.0.0",
     "uuid": "^9.0.0",
     "winston": "^3.2.1",
+    "winston-transport": "^4.7.0",
     "yaml": "^2.0.0",
     "zen-observable": "^0.10.0",
     "zod": "^3.22.4"

plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts

@@ -55,7 +55,7 @@ import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alp
 import { TaskRecovery } from '@backstage/plugin-scaffolder-common';
 import { PermissionsService } from '@backstage/backend-plugin-api';
 import { loggerToWinstonLogger } from '@backstage/backend-common';
-import { WinstonLogger } from './logger';
+import { BackstageLoggerTransport, WinstonLogger } from './logger';
 
 type NunjucksWorkflowRunnerOptions = {
   workingDirectory: string;
@@ -98,9 +98,11 @@ const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => {
 const createStepLogger = ({
   task,
   step,
+  rootLogger,
 }: {
   task: TaskContext;
   step: TaskStep;
+  rootLogger: winston.Logger;
 }) => {
   const stepLogStream = new PassThrough();
   stepLogStream.on('data', async data => {
@@ -117,8 +119,8 @@ const createStepLogger = ({
       winston.format.simple(),
     ),
     transports: [
-      new winston.transports.Console(),
       new winston.transports.Stream({ stream: stepLogStream }),
+      new BackstageLoggerTransport(rootLogger),
     ],
   });
 
@@ -262,7 +264,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
 
       const action: TemplateAction<JsonObject> =
         this.options.actionRegistry.get(step.action);
-      const { taskLogger, streamLogger } = createStepLogger({ task, step });
+      const { taskLogger, streamLogger } = createStepLogger({
+        task,
+        step,
+        rootLogger: this.options.logger,
+      });
 
       if (task.isDryRun) {
         const redactedSecrets = Object.fromEntries(

plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts

@@ -19,13 +19,8 @@ import {
 } from '@backstage/backend-plugin-api';
 import { JsonObject } from '@backstage/types';
 import { Format, TransformableInfo } from 'logform';
-import {
-  Logger,
-  format,
-  createLogger,
-  transports,
-  transport as Transport,
-} from 'winston';
+import Transport, { TransportStreamOptions } from 'winston-transport';
+import { Logger, format, createLogger, transports } from 'winston';
 
 /**
  * Escapes a given string to be used inside a RegExp.
@@ -43,6 +38,42 @@ interface WinstonLoggerOptions {
   transports: Transport[];
 }
 
+// This is a workaround for being able to preserve the log format of the root logger.
+// Will revisit all of this implementation once we can break the router to use only `LoggerService`.
+export class BackstageLoggerTransport extends Transport {
+  constructor(
+    private readonly backstageLogger: LoggerService,
+    opts?: TransportStreamOptions,
+  ) {
+    super(opts);
+  }
+
+  log(info: unknown, callback: VoidFunction) {
+    if (typeof info !== 'object' || info === null) {
+      callback();
+      return;
+    }
+    const { level, message, ...meta } = info as JsonObject;
+    switch (level) {
+      case 'error':
+        this.backstageLogger.error(String(message), meta);
+        break;
+      case 'warn':
+        this.backstageLogger.warn(String(message), meta);
+        break;
+      case 'info':
+        this.backstageLogger.info(String(message), meta);
+        break;
+      case 'debug':
+        this.backstageLogger.debug(String(message), meta);
+        break;
+      default:
+        this.backstageLogger.info(String(message), meta);
+    }
+    callback();
+  }
+}
+
 export class WinstonLogger implements RootLoggerService {
   #winston: Logger;
   #addRedactions?: (redactions: Iterable<string>) => void;
@@ -76,16 +107,20 @@ export class WinstonLogger implements RootLoggerService {
 
     let redactionPattern: RegExp | undefined = undefined;
 
+    const replace = (obj: TransformableInfo) => {
+      for (const key in obj) {
+        if (obj.hasOwnProperty(key)) {
+          if (typeof obj[key] === 'object') {
+            obj[key] = replace(obj[key] as TransformableInfo);
+          } else if (typeof obj[key] === 'string') {
+            obj[key] = obj[key]?.replace(redactionPattern, '[REDACTED]');
+          }
+        }
+      }
+      return obj;
+    };
     return {
-      format: format(info => {
-        if (redactionPattern && typeof info.message === 'string') {
-          info.message = info.message.replace(redactionPattern, '[REDACTED]');
-        }
-        if (redactionPattern && typeof info.stack === 'string') {
-          info.stack = info.stack.replace(redactionPattern, '[REDACTED]');
-        }
-        return info;
-      })(),
+      format: format(replace)(),
       add(newRedactions) {
         let added = 0;
         for (const redactionToTrim of newRedactions) {