From 4d2b87c44b05db3d4a0cf26e24f3b70886f9009b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:47:56 +0100 Subject: [PATCH] backend-app-api: flip around config and logger dependency Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 3 +-- .../src/config/ObservableConfigProxy.test.ts | 13 +++------- .../src/config/ObservableConfigProxy.ts | 9 ++++--- packages/backend-app-api/src/config/config.ts | 8 +++---- .../src/logging/WinstonLogger.ts | 6 ++--- .../implementations/config/configFactory.ts | 24 ++++--------------- .../rootLogger/rootLoggerFactory.ts | 15 +++++++++--- packages/backend-plugin-api/api-report.md | 5 +--- .../services/definitions/RootLoggerService.ts | 4 +--- 9 files changed, 33 insertions(+), 54 deletions(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 03d1ecc2be..bcf746aa5a 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -165,7 +165,6 @@ export const lifecycleFactory: () => ServiceFactory; // @public export function loadBackendConfig(options: { - logger: LoggerService; remote?: LoadConfigOptionsRemote; argv: string[]; }): Promise<{ @@ -260,7 +259,7 @@ export const urlReaderFactory: () => ServiceFactory; // @public export class WinstonLogger implements RootLoggerService { // (undocumented) - addRedactions(redactions: string[]): void; + addRedactions(redactions: Iterable): void; // (undocumented) child(meta: LogMeta): LoggerService; static colorFormat(): Format; diff --git a/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts index e90225db1a..876f5ac7d3 100644 --- a/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts @@ -14,19 +14,12 @@ * limitations under the License. */ -import { LoggerService } from '@backstage/backend-plugin-api'; import { ConfigReader } from '@backstage/config'; import { ObservableConfigProxy } from './ObservableConfigProxy'; describe('ObservableConfigProxy', () => { - const errLogger = { - error: (message: string) => { - throw new Error(message); - }, - } as unknown as LoggerService; - it('should notify subscribers', () => { - const config = new ObservableConfigProxy(errLogger); + const config = new ObservableConfigProxy(); const fn = jest.fn(); const sub = config.subscribe(fn); @@ -51,7 +44,7 @@ describe('ObservableConfigProxy', () => { }); it('should forward subscriptions', () => { - const config1 = new ObservableConfigProxy(errLogger); + const config1 = new ObservableConfigProxy(); const fn1 = jest.fn(); const fn2 = jest.fn(); @@ -119,7 +112,7 @@ describe('ObservableConfigProxy', () => { }); it('should make sub configs available as expected', () => { - const config = new ObservableConfigProxy(errLogger); + const config = new ObservableConfigProxy(); config.setConfig(new ConfigReader({ a: { x: 1 } })); diff --git a/packages/backend-app-api/src/config/ObservableConfigProxy.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.ts index 8fcc3f060f..dd3334c9a5 100644 --- a/packages/backend-app-api/src/config/ObservableConfigProxy.ts +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ConfigService, LoggerService } from '@backstage/backend-plugin-api'; +import { ConfigService } from '@backstage/backend-plugin-api'; import { ConfigReader } from '@backstage/config'; import { JsonValue } from '@backstage/types'; @@ -24,7 +24,6 @@ export class ObservableConfigProxy implements ConfigService { private readonly subscribers: (() => void)[] = []; constructor( - private readonly logger: LoggerService, private readonly parent?: ObservableConfigProxy, private parentKey?: string, ) { @@ -42,7 +41,7 @@ export class ObservableConfigProxy implements ConfigService { try { subscriber(); } catch (error) { - this.logger.error(`Config subscriber threw error, ${error}`); + console.error(`Config subscriber threw error, ${error}`); } } } @@ -89,11 +88,11 @@ export class ObservableConfigProxy implements ConfigService { return this.select(false)?.getOptional(key); } getConfig(key: string): ConfigService { - return new ObservableConfigProxy(this.logger, this, key); + return new ObservableConfigProxy(this, key); } getOptionalConfig(key: string): ConfigService | undefined { if (this.select(false)?.has(key)) { - return new ObservableConfigProxy(this.logger, this, key); + return new ObservableConfigProxy(this, key); } return undefined; } diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index 6adebbdf52..b1da4fb85b 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -68,8 +68,6 @@ export async function createConfigSecretEnumerator(options: { * @public */ export async function loadBackendConfig(options: { - logger: LoggerService; - // process.argv or any other overrides remote?: LoadConfigOptionsRemote; argv: string[]; }): Promise<{ config: Config }> { @@ -84,14 +82,14 @@ export async function loadBackendConfig(options: { let currentCancelFunc: (() => void) | undefined = undefined; - const config = new ObservableConfigProxy(options.logger); + const config = new ObservableConfigProxy(); const { appConfigs } = await loadConfig({ configRoot: paths.targetRoot, configTargets: configTargets, remote: options.remote, watch: { onChange(newConfigs) { - options.logger.info( + console.info( `Reloaded config from ${newConfigs.map(c => c.context).join(', ')}`, ); @@ -112,7 +110,7 @@ export async function loadBackendConfig(options: { }, }); - options.logger.info( + console.info( `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`, ); diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts index 706e3cf7a7..cfee65d053 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -45,7 +45,7 @@ export interface WinstonLoggerOptions { */ export class WinstonLogger implements RootLoggerService { #winston: Logger; - #addRedactions?: RootLoggerService['addRedactions']; + #addRedactions?: (redactions: Iterable) => void; /** * Creates a {@link WinstonLogger} instance. @@ -143,7 +143,7 @@ export class WinstonLogger implements RootLoggerService { private constructor( winston: Logger, - addRedactions?: RootLoggerService['addRedactions'], + addRedactions?: (redactions: Iterable) => void, ) { this.#winston = winston; this.#addRedactions = addRedactions; @@ -169,7 +169,7 @@ export class WinstonLogger implements RootLoggerService { return new WinstonLogger(this.#winston.child(meta)); } - addRedactions(redactions: string[]) { + addRedactions(redactions: Iterable) { this.#addRedactions?.(redactions); } } diff --git a/packages/backend-app-api/src/services/implementations/config/configFactory.ts b/packages/backend-app-api/src/services/implementations/config/configFactory.ts index b0f5a17f3f..0ec15c29fe 100644 --- a/packages/backend-app-api/src/services/implementations/config/configFactory.ts +++ b/packages/backend-app-api/src/services/implementations/config/configFactory.ts @@ -19,10 +19,7 @@ import { createServiceFactory, } from '@backstage/backend-plugin-api'; import { LoadConfigOptionsRemote } from '@backstage/config-loader'; -import { - createConfigSecretEnumerator, - loadBackendConfig, -} from '../../../config'; +import { loadBackendConfig } from '../../../config'; /** @public */ export interface ConfigFactoryOptions { @@ -40,22 +37,11 @@ export interface ConfigFactoryOptions { /** @public */ export const configFactory = createServiceFactory({ service: coreServices.config, - deps: { - logger: coreServices.rootLogger, - }, - async factory({ logger }, options?: ConfigFactoryOptions) { - const argv = options?.argv ?? process.argv; - const secretEnumerator = await createConfigSecretEnumerator({ logger }); - - const { config } = await loadBackendConfig({ - argv, - logger, - remote: options?.remote, - }); - - logger.addRedactions(secretEnumerator(config)); - config.subscribe?.(() => logger.addRedactions(secretEnumerator(config))); + deps: {}, + async factory({}, options?: ConfigFactoryOptions) { + const { argv = process.argv, remote } = options ?? {}; + const { config } = await loadBackendConfig({ argv, remote }); return config; }, }); diff --git a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts index 33443613b6..1c2214b98c 100644 --- a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts @@ -20,13 +20,16 @@ import { } from '@backstage/backend-plugin-api'; import { WinstonLogger } from '../../../logging'; import { transports, format } from 'winston'; +import { createConfigSecretEnumerator } from '../../../config'; /** @public */ export const rootLoggerFactory = createServiceFactory({ service: coreServices.rootLogger, - deps: {}, - async factory() { - return WinstonLogger.create({ + deps: { + config: coreServices.config, + }, + async factory({ config }) { + const logger = WinstonLogger.create({ meta: { service: 'backstage', }, @@ -37,5 +40,11 @@ export const rootLoggerFactory = createServiceFactory({ : WinstonLogger.colorFormat(), transports: [new transports.Console()], }); + + const secretEnumerator = await createConfigSecretEnumerator({ logger }); + logger.addRedactions(secretEnumerator(config)); + config.subscribe?.(() => logger.addRedactions(secretEnumerator(config))); + + return logger; }, }); diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 408908ca71..33190277c9 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -282,10 +282,7 @@ export interface RootHttpRouterService { export interface RootLifecycleService extends LifecycleService {} // @public (undocumented) -export interface RootLoggerService extends LoggerService { - // (undocumented) - addRedactions(redactions: Iterable): void; -} +export interface RootLoggerService extends LoggerService {} // @public (undocumented) export interface SchedulerService extends PluginTaskScheduler {} diff --git a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts index a6840f48c0..ad318ef53f 100644 --- a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts +++ b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts @@ -17,6 +17,4 @@ import { LoggerService } from './LoggerService'; /** @public */ -export interface RootLoggerService extends LoggerService { - addRedactions(redactions: Iterable): void; -} +export interface RootLoggerService extends LoggerService {}