backend-common: prefer LoggerService over Logger

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/lucky-taxis-vanish.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Replaced dependencies on the `Logger` type from `winston` with `LoggerService` from `@backstage/backend-plugin-api`. This is not a breaking change as the `LoggerService` is a subset of the `Logger` interface.

packages/backend-common/api-report.md

@@ -151,7 +151,7 @@ export class BitbucketServerUrlReader implements UrlReader {
 export class BitbucketUrlReader implements UrlReader {
   constructor(
     integration: BitbucketIntegration,
-    logger: Logger,
+    logger: LoggerService,
     deps: {
       treeResponseFactory: ReadTreeResponseFactory;
     },
@@ -187,7 +187,7 @@ export class CacheManager {
 
 // @public
 export type CacheManagerOptions = {
-  logger?: Logger;
+  logger?: LoggerService;
   onError?: (err: Error) => void;
 };
 
@@ -239,7 +239,7 @@ export function createServiceBuilder(_module: NodeModule): ServiceBuilder;
 
 // @public
 export function createStatusCheckRouter(options: {
-  logger: Logger;
+  logger: LoggerService;
   path?: string;
   statusCheck?: StatusCheck;
 }): Promise<express.Router>;
@@ -256,7 +256,7 @@ export class DatabaseManager {
 // @public
 export type DatabaseManagerOptions = {
   migrations?: PluginDatabaseManager['migrations'];
-  logger?: Logger;
+  logger?: LoggerService;
 };
 
 // @public
@@ -280,7 +280,7 @@ export function errorHandler(
 // @public
 export type ErrorHandlerOptions = {
   showStackTraces?: boolean;
-  logger?: Logger;
+  logger?: LoggerService;
   logClientErrors?: boolean;
 };
 
@@ -381,7 +381,7 @@ export class Git {
     username?: string;
     password?: string;
     token?: string;
-    logger?: Logger;
+    logger?: LoggerService;
   }) => Git;
   // (undocumented)
   init(options: { dir: string; defaultBranch?: string }): Promise<void>;
@@ -502,7 +502,7 @@ export type KubernetesContainerRunnerOptions = {
 
 // @public
 export function loadBackendConfig(options: {
-  logger: Logger;
+  logger: LoggerService;
   remote?: LoadConfigOptionsRemote;
   argv: string[];
 }): Promise<Config>;
@@ -525,7 +525,7 @@ export { PluginEndpointDiscovery };
 // @public
 export type ReaderFactory = (options: {
   config: Config;
-  logger: Logger;
+  logger: LoggerService;
   treeResponseFactory: ReadTreeResponseFactory;
 }) => UrlReaderPredicateTuple[];
 
@@ -593,10 +593,12 @@ export function redactWinstonLogLine(
 ): winston.Logform.TransformableInfo;
 
 // @public
-export function requestLoggingHandler(logger?: Logger): RequestHandler;
+export function requestLoggingHandler(logger?: LoggerService): RequestHandler;
 
 // @public
-export type RequestLoggingHandlerFactory = (logger?: Logger) => RequestHandler;
+export type RequestLoggingHandlerFactory = (
+  logger?: LoggerService,
+) => RequestHandler;
 
 // @public
 export function resolvePackagePath(name: string, ...paths: string[]): string;
@@ -640,7 +642,7 @@ export class ServerTokenManager implements TokenManager {
 
 // @public
 export interface ServerTokenManagerOptions {
-  logger: Logger;
+  logger: LoggerService;
 }
 
 // @public
@@ -648,7 +650,7 @@ export type ServiceBuilder = {
   loadConfig(config: Config): ServiceBuilder;
   setPort(port: number): ServiceBuilder;
   setHost(host: string): ServiceBuilder;
-  setLogger(logger: Logger): ServiceBuilder;
+  setLogger(logger: LoggerService): ServiceBuilder;
   enableCors(options: cors.CorsOptions): ServiceBuilder;
   setHttpsSettings(settings: {
     certificate:
@@ -718,7 +720,7 @@ export class UrlReaders {
 // @public
 export type UrlReadersOptions = {
   config: Config;
-  logger: Logger;
+  logger: LoggerService;
   factories?: ReaderFactory[];
 };
 

packages/backend-common/src/cache/CacheManager.test.ts

@@ -240,7 +240,10 @@ describe('CacheManager', () => {
       // Invoke the actual error handler.
       const expectedError = new Error('some error');
       realHandler(expectedError);
-      expect(mockLogger.error).toHaveBeenCalledWith(expectedError);
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        'Failed to create cache client',
+        expectedError,
+      );
     });
 
     it('calls provided handler', () => {

packages/backend-common/src/cache/CacheManager.ts

@@ -18,7 +18,7 @@ import { Config } from '@backstage/config';
 import Keyv from 'keyv';
 import KeyvMemcache from '@keyv/memcache';
 import KeyvRedis from '@keyv/redis';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { getRootLogger } from '../logging';
 import { DefaultCacheClient, CacheClient } from './CacheClient';
 import { NoStore } from './NoStore';
@@ -50,7 +50,7 @@ export class CacheManager {
    */
   private readonly memoryStore = new Map();
 
-  private readonly logger: Logger;
+  private readonly logger: LoggerService;
   private readonly store: keyof CacheManager['storeFactories'];
   private readonly connection: string;
   private readonly errorHandler: CacheManagerOptions['onError'];
@@ -79,7 +79,7 @@ export class CacheManager {
   private constructor(
     store: string,
     connectionString: string,
-    logger: Logger,
+    logger: LoggerService,
     errorHandler: CacheManagerOptions['onError'],
   ) {
     if (!this.storeFactories.hasOwnProperty(store)) {
@@ -105,7 +105,7 @@ export class CacheManager {
         // Always provide an error handler to avoid stopping the process.
         concreteClient.on('error', (err: Error) => {
           // In all cases, just log the error.
-          this.logger.error(err);
+          this.logger.error('Failed to create cache client', err);
 
           // Invoke any custom error handler if provided.
           if (typeof this.errorHandler === 'function') {

packages/backend-common/src/cache/types.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 
 export type {
   CacheService as PluginCacheManager,
@@ -30,7 +30,7 @@ export type CacheManagerOptions = {
   /**
    * An optional logger for use by the PluginCacheManager.
    */
-  logger?: Logger;
+  logger?: LoggerService;
 
   /**
    * An optional handler for connection errors emitted from the underlying data

packages/backend-common/src/config.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { ConfigReader } from '@backstage/config';
 import { ObservableConfigProxy } from './config';
 
@@ -23,7 +23,7 @@ describe('ObservableConfigProxy', () => {
     error: (message: string) => {
       throw new Error(message);
     },
-  } as unknown as Logger;
+  } as unknown as LoggerService;
 
   it('should notify subscribers', () => {
     const config = new ObservableConfigProxy(errLogger);

packages/backend-common/src/config.ts

@@ -16,7 +16,7 @@
 
 import { resolve as resolvePath } from 'path';
 import parseArgs from 'minimist';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { findPaths } from '@backstage/cli-common';
 import {
   loadConfigSchema,
@@ -37,7 +37,7 @@ import { setRootLoggerRedactionList } from './logging/rootLogger';
 const updateRedactionList = (
   schema: ConfigSchema,
   configs: AppConfig[],
-  logger: Logger,
+  logger: LoggerService,
 ) => {
   const secretAppConfigs = schema.process(configs, {
     visibility: ['secret'],
@@ -67,7 +67,7 @@ export class ObservableConfigProxy implements Config {
   private readonly subscribers: (() => void)[] = [];
 
   constructor(
-    private readonly logger: Logger,
+    private readonly logger: LoggerService,
     private readonly parent?: ObservableConfigProxy,
     private parentKey?: string,
   ) {
@@ -183,7 +183,7 @@ let currentCancelFunc: () => void;
  * @public
  */
 export async function loadBackendConfig(options: {
-  logger: Logger;
+  logger: LoggerService;
   // process.argv or any other overrides
   remote?: LoadConfigOptionsRemote;
   argv: string[];

packages/backend-common/src/database/DatabaseManager.ts

@@ -29,7 +29,7 @@ import {
 } from './connection';
 import { PluginDatabaseManager } from './types';
 import path from 'path';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { stringifyError } from '@backstage/errors';
 
 /**
@@ -46,7 +46,7 @@ function pluginPath(pluginId: string): string {
  */
 export type DatabaseManagerOptions = {
   migrations?: PluginDatabaseManager['migrations'];
-  logger?: Logger;
+  logger?: LoggerService;
 };
 
 /**

packages/backend-common/src/middleware/errorHandler.test.ts

@@ -155,7 +155,10 @@ describe('errorHandler', () => {
 
     await request(app).get('/breaks');
 
-    expect(mockLogger.error).toHaveBeenCalledWith(thrownError);
+    expect(mockLogger.error).toHaveBeenCalledWith(
+      'Request failed with status 500',
+      thrownError,
+    );
   });
 
   it('does not log 400 errors', async () => {

packages/backend-common/src/middleware/errorHandler.ts

@@ -25,7 +25,7 @@ import {
   serializeError,
 } from '@backstage/errors';
 import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { getRootLogger } from '../logging';
 
 /**
@@ -46,7 +46,7 @@ export type ErrorHandlerOptions = {
    *
    * If not specified, the root logger will be used.
    */
-  logger?: Logger;
+  logger?: LoggerService;
 
   /**
    * Whether any 4xx errors should be logged or not.
@@ -83,7 +83,7 @@ export function errorHandler(
   return (error: Error, req: Request, res: Response, next: NextFunction) => {
     const statusCode = getStatusCode(error);
     if (options.logClientErrors || statusCode >= 500) {
-      logger.error(error);
+      logger.error(`Request failed with status ${statusCode}`, error);
     }
 
     if (res.headersSent) {

packages/backend-common/src/middleware/requestLoggingHandler.ts

@@ -15,7 +15,7 @@
  */
 
 import { RequestHandler } from 'express';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import morgan from 'morgan';
 import { getRootLogger } from '../logging';
 
@@ -26,7 +26,7 @@ import { getRootLogger } from '../logging';
  * @param logger - An optional logger to use. If not specified, the root logger will be used.
  * @returns An Express request handler
  */
-export function requestLoggingHandler(logger?: Logger): RequestHandler {
+export function requestLoggingHandler(logger?: LoggerService): RequestHandler {
   const actualLogger = (logger || getRootLogger()).child({
     type: 'incomingRequest',
   });

packages/backend-common/src/reading/BitbucketUrlReader.ts

@@ -28,7 +28,7 @@ import parseGitUrl from 'git-url-parse';
 import { trimEnd } from 'lodash';
 import { Minimatch } from 'minimatch';
 import { Readable } from 'stream';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import {
   ReaderFactory,
   ReadTreeOptions,
@@ -70,7 +70,7 @@ export class BitbucketUrlReader implements UrlReader {
 
   constructor(
     private readonly integration: BitbucketIntegration,
-    logger: Logger,
+    logger: LoggerService,
     private readonly deps: { treeResponseFactory: ReadTreeResponseFactory },
   ) {
     const { host, token, username, appPassword } = integration.config;

packages/backend-common/src/reading/UrlReaders.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { Config } from '@backstage/config';
 import { ReaderFactory, UrlReader } from './types';
 import { UrlReaderPredicateMux } from './UrlReaderPredicateMux';
@@ -40,7 +40,7 @@ export type UrlReadersOptions = {
   /** Root config object */
   config: Config;
   /** Logger used by all the readers */
-  logger: Logger;
+  logger: LoggerService;
   /** A list of factories used to construct individual readers that match on URLs */
   factories?: ReaderFactory[];
 };

packages/backend-common/src/reading/types.ts

@@ -15,11 +15,11 @@
  */
 
 import { Readable } from 'stream';
-import { Logger } from 'winston';
 import { Config } from '@backstage/config';
 import {
   UrlReaderService,
   ReadTreeResponse,
+  LoggerService,
 } from '@backstage/backend-plugin-api';
 
 export type {
@@ -54,7 +54,7 @@ export type UrlReaderPredicateTuple = {
  */
 export type ReaderFactory = (options: {
   config: Config;
-  logger: Logger;
+  logger: LoggerService;
   treeResponseFactory: ReadTreeResponseFactory;
 }) => UrlReaderPredicateTuple[];
 

packages/backend-common/src/scm/git.ts

@@ -21,7 +21,7 @@ import git, {
 } from 'isomorphic-git';
 import http from 'isomorphic-git/http/node';
 import fs from 'fs-extra';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 
 /*
 provider          username         password
@@ -52,7 +52,7 @@ export class Git {
       username?: string;
       password?: string;
       token?: string;
-      logger?: Logger;
+      logger?: LoggerService;
     },
   ) {
     this.headers = {
@@ -308,7 +308,7 @@ export class Git {
     username?: string;
     password?: string;
     token?: string;
-    logger?: Logger;
+    logger?: LoggerService;
   }) => {
     const { username, password, token, logger } = options;
     return new Git({ username, password, token, logger });

packages/backend-common/src/service/createStatusCheckRouter.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import Router from 'express-promise-router';
 import express from 'express';
 import { errorHandler, statusCheckHandler, StatusCheck } from '../middleware';
@@ -32,7 +32,7 @@ import { errorHandler, statusCheckHandler, StatusCheck } from '../middleware';
  * @public
  */
 export async function createStatusCheckRouter(options: {
-  logger: Logger;
+  logger: LoggerService;
   /**
    * The path (including a leading slash) that the health check should be
    * mounted on.

packages/backend-common/src/service/lib/ServiceBuilderImpl.ts

@@ -22,7 +22,7 @@ import helmet from 'helmet';
 import { ContentSecurityPolicyOptions } from 'helmet/dist/types/middlewares/content-security-policy';
 import * as http from 'http';
 import stoppable from 'stoppable';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { useHotCleanup } from '../../hot';
 import { getRootLogger } from '../../logging';
 import {
@@ -48,7 +48,7 @@ const DEFAULT_HOST = '';
 export class ServiceBuilderImpl implements ServiceBuilder {
   private port: number | undefined;
   private host: string | undefined;
-  private logger: Logger | undefined;
+  private logger: LoggerService | undefined;
   private corsOptions: cors.CorsOptions | undefined;
   private cspOptions: Record<string, string[] | false> | undefined;
   private httpsSettings: HttpsSettings | undefined;
@@ -111,7 +111,7 @@ export class ServiceBuilderImpl implements ServiceBuilder {
     return this;
   }
 
-  setLogger(logger: Logger): ServiceBuilder {
+  setLogger(logger: LoggerService): ServiceBuilder {
     this.logger = logger;
     return this;
   }

packages/backend-common/src/service/lib/hostFactory.ts

@@ -19,7 +19,7 @@ import { resolve as resolvePath, dirname } from 'path';
 import express from 'express';
 import * as http from 'http';
 import * as https from 'https';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { HttpsSettings } from './config';
 import forge from 'node-forge';
 
@@ -37,7 +37,7 @@ const IP_HOSTNAME_REGEX = /:|^\d+\.\d+\.\d+\.\d+$/;
  */
 export function createHttpServer(
   app: express.Express,
-  logger?: Logger,
+  logger?: LoggerService,
 ): http.Server {
   logger?.info('Initializing http server');
 
@@ -56,7 +56,7 @@ export function createHttpServer(
 export async function createHttpsServer(
   app: express.Express,
   httpsSettings: HttpsSettings,
-  logger?: Logger,
+  logger?: LoggerService,
 ): Promise<http.Server> {
   logger?.info('Initializing https server');
 
@@ -83,7 +83,7 @@ export async function createHttpsServer(
   return https.createServer(credentials, app) as http.Server;
 }
 
-function getCertificateExpiration(cert: string, logger?: Logger) {
+function getCertificateExpiration(cert: string, logger?: LoggerService) {
   try {
     const crt = forge.pki.certificateFromPem(cert);
     return crt.validity.notAfter.getTime() - Date.now();
@@ -93,7 +93,10 @@ function getCertificateExpiration(cert: string, logger?: Logger) {
   }
 }
 
-async function getGeneratedCertificate(hostname: string, logger?: Logger) {
+async function getGeneratedCertificate(
+  hostname: string,
+  logger?: LoggerService,
+) {
   const hasModules = await fs.pathExists('node_modules');
   let certPath;
   if (hasModules) {

packages/backend-common/src/service/types.ts

@@ -18,7 +18,7 @@ import { Config } from '@backstage/config';
 import cors from 'cors';
 import { Router, RequestHandler, ErrorRequestHandler } from 'express';
 import { Server } from 'http';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 
 /**
  * A helper for building backend service instances.
@@ -60,7 +60,7 @@ export type ServiceBuilder = {
    *
    * @param logger - A winston logger
    */
-  setLogger(logger: Logger): ServiceBuilder;
+  setLogger(logger: LoggerService): ServiceBuilder;
 
   /**
    * Enables CORS handling using the given settings.
@@ -128,4 +128,6 @@ export type ServiceBuilder = {
  *
  * @public
  */
-export type RequestLoggingHandlerFactory = (logger?: Logger) => RequestHandler;
+export type RequestLoggingHandlerFactory = (
+  logger?: LoggerService,
+) => RequestHandler;

packages/backend-common/src/tokens/ServerTokenManager.test.ts

@@ -16,7 +16,6 @@
 
 import { ConfigReader } from '@backstage/config';
 import * as jose from 'jose';
-import { Logger } from 'winston';
 import { getVoidLogger } from '../logging/voidLogger';
 import { ServerTokenManager } from './ServerTokenManager';
 import { TokenManager } from './types';
@@ -26,12 +25,11 @@ const configWithSecret = new ConfigReader({
   backend: { auth: { keys: [{ secret: 'a-secret-key' }] } },
 });
 const env = process.env;
-let logger: Logger;
+const logger = getVoidLogger();
 
 describe('ServerTokenManager', () => {
   beforeEach(() => {
     process.env = { ...env };
-    logger = getVoidLogger();
   });
 
   afterEach(() => {

packages/backend-common/src/tokens/ServerTokenManager.ts

@@ -18,7 +18,7 @@ import { Config } from '@backstage/config';
 import { AuthenticationError } from '@backstage/errors';
 import { base64url, exportJWK, generateSecret, jwtVerify, SignJWT } from 'jose';
 import { DateTime, Duration } from 'luxon';
-import { Logger } from 'winston';
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { TokenManager } from './types';
 
 const TOKEN_ALG = 'HS256';
@@ -49,7 +49,7 @@ export interface ServerTokenManagerOptions {
   /**
    * The logger to use.
    */
-  logger: Logger;
+  logger: LoggerService;
 }
 
 /**