From e939cd7f63bafefd0f54378b456f9e4132a6392f Mon Sep 17 00:00:00 2001 From: David Festal Date: Fri, 25 Oct 2024 17:43:12 +0200 Subject: [PATCH] refactor(backend-dynamic-feature-service): Providing config-based root logger options. Signed-off-by: David Festal --- .changeset/bright-singers-do.md | 10 ++++ .../report.api.md | 5 +- .../src/features/features.test.ts | 54 ++++++++++++++++--- .../src/features/features.ts | 30 +++++++---- 4 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 .changeset/bright-singers-do.md diff --git a/.changeset/bright-singers-do.md b/.changeset/bright-singers-do.md new file mode 100644 index 0000000000..8b10363612 --- /dev/null +++ b/.changeset/bright-singers-do.md @@ -0,0 +1,10 @@ +--- +'@backstage/backend-dynamic-feature-service': minor +--- + +**BREAKING** The `dynamicPluginsFeatureLoader` options related to the root logger behavior (`transports`, `level`, `format`) are now gathered under a single `logger` option which is a function taking an optional `Config` argument and returning the logger options. + +This breaking change is required for 2 reasons: + +- it's totally possible that the current `Config` would be required to provide the logger options, +- the logger-related options should be gathered under a common `logger` option because, when the root auditing service is introduced, distinct but similarly-named options would be required for the auditor as well. diff --git a/packages/backend-dynamic-feature-service/report.api.md b/packages/backend-dynamic-feature-service/report.api.md index 97e1f0dde9..ee2886a79f 100644 --- a/packages/backend-dynamic-feature-service/report.api.md +++ b/packages/backend-dynamic-feature-service/report.api.md @@ -139,8 +139,9 @@ export const dynamicPluginsFeatureLoader: (( // @public (undocumented) export type DynamicPluginsFeatureLoaderOptions = DynamicPluginsFactoryOptions & - DynamicPluginsSchemasOptions & - DynamicPluginsRootLoggerFactoryOptions; + DynamicPluginsSchemasOptions & { + logger?: (config?: Config) => DynamicPluginsRootLoggerFactoryOptions; + }; // @public @deprecated (undocumented) export const dynamicPluginsFrontendSchemas: BackendFeature; diff --git a/packages/backend-dynamic-feature-service/src/features/features.test.ts b/packages/backend-dynamic-feature-service/src/features/features.test.ts index fe32d61d52..f83a1ad1a7 100644 --- a/packages/backend-dynamic-feature-service/src/features/features.test.ts +++ b/packages/backend-dynamic-feature-service/src/features/features.test.ts @@ -119,8 +119,10 @@ describe('dynamicPluginsFeatureLoader', () => { dynamicPluginsFeatureLoader({ moduleLoader: logger => jestFreeTypescriptAwareModuleLoader(logger, true), - transports: [mockedTransport], - format: winston.format.simple(), + logger: () => ({ + transports: [mockedTransport], + format: winston.format.simple(), + }), }), dynamicPLuginsLister.feature(), ], @@ -180,6 +182,42 @@ describe('dynamicPluginsFeatureLoader', () => { ]); }); + it('should allow overriding logger options based on config', async () => { + const mockedTransport = new MockedTransport(); + await startTestBackend({ + features: [ + mockServices.rootConfig.factory({ + data: { + dynamicPlugins: { + rootDirectory: dynamicPluginsRootDirectory, + }, + customLogLabel: 'a very nice label', + }, + }), + dynamicPluginsFeatureLoader({ + moduleLoader: logger => jestFreeTypescriptAwareModuleLoader(logger), + logger: config => { + const label = config?.getString('customLogLabel') ?? 'no-label'; + return { + transports: [mockedTransport], + format: winston.format.combine( + winston.format.label({ + label, + message: true, + }), + winston.format.simple(), + ), + }; + }, + }), + ], + }); + + expect(mockedTransport.logs).toContainEqual( + 'info: [a very nice label] Found 0 new secrets in config that will be redacted {"service":"backstage"}', + ); + }); + it('should redact the secret config values of dynamic plugin config schemas in logs', async () => { const mockedTransport = new MockedTransport(); await startTestBackend({ @@ -196,8 +234,10 @@ describe('dynamicPluginsFeatureLoader', () => { }), dynamicPluginsFeatureLoader({ moduleLoader: jestFreeTypescriptAwareModuleLoader, - transports: [mockedTransport], - format: winston.format.simple(), + logger: () => ({ + transports: [mockedTransport], + format: winston.format.simple(), + }), }), ], }); @@ -333,8 +373,10 @@ describe('dynamicPluginsFeatureLoader', () => { }), dynamicPluginsFeatureLoader({ moduleLoader: jestFreeTypescriptAwareModuleLoader, - transports: [mockedTransport], - format: winston.format.simple(), + logger: () => ({ + transports: [mockedTransport], + format: winston.format.simple(), + }), }), dynamicPLuginsLister.feature(), ], diff --git a/packages/backend-dynamic-feature-service/src/features/features.ts b/packages/backend-dynamic-feature-service/src/features/features.ts index 4788f1da75..94711816d7 100644 --- a/packages/backend-dynamic-feature-service/src/features/features.ts +++ b/packages/backend-dynamic-feature-service/src/features/features.ts @@ -18,26 +18,28 @@ import { coreServices, createBackendFeatureLoader, } from '@backstage/backend-plugin-api'; -import { - DynamicPluginsSchemasOptions, - dynamicPluginsFrontendSchemas, - dynamicPluginsRootLoggerServiceFactory, - dynamicPluginsSchemasServiceFactory, -} from '../schemas'; +import type { Config } from '@backstage/config'; import { DynamicPluginsFactoryOptions, dynamicPluginsFeatureDiscoveryLoader, dynamicPluginsServiceFactory, } from '../manager'; -import { DynamicPluginsRootLoggerFactoryOptions } from '../schemas'; import { configKey } from '../scanner/plugin-scanner'; +import { + DynamicPluginsRootLoggerFactoryOptions, + DynamicPluginsSchemasOptions, + dynamicPluginsFrontendSchemas, + dynamicPluginsRootLoggerServiceFactory, + dynamicPluginsSchemasServiceFactory, +} from '../schemas'; /** * @public */ export type DynamicPluginsFeatureLoaderOptions = DynamicPluginsFactoryOptions & - DynamicPluginsSchemasOptions & - DynamicPluginsRootLoggerFactoryOptions; + DynamicPluginsSchemasOptions & { + logger?: (config?: Config) => DynamicPluginsRootLoggerFactoryOptions; + }; const dynamicPluginsFeatureLoaderWithOptions = ( options?: DynamicPluginsFeatureLoaderOptions, @@ -49,13 +51,18 @@ const dynamicPluginsFeatureLoaderWithOptions = ( *loader({ config }) { const dynamicPluginsEnabled = config.has(configKey); + let rootLoggerOptions: DynamicPluginsRootLoggerFactoryOptions = {}; + if (options?.logger) { + rootLoggerOptions = options.logger(config); + } + yield* [ dynamicPluginsSchemasServiceFactory(options), dynamicPluginsServiceFactory(options), ]; if (dynamicPluginsEnabled) { yield* [ - dynamicPluginsRootLoggerServiceFactory(options), + dynamicPluginsRootLoggerServiceFactory(rootLoggerOptions), dynamicPluginsFrontendSchemas, dynamicPluginsFeatureDiscoveryLoader, ]; @@ -94,12 +101,13 @@ const dynamicPluginsFeatureLoaderWithOptions = ( * import { dynamicPluginsFeatureLoader } from '@backstage/backend-dynamic-feature-service'; * import { myCustomModuleLoader } from './myCustomModuleLoader'; * import { myCustomSchemaLocator } from './myCustomSchemaLocator'; + * import { myConfiguredLoggerOptions } from './myConfiguredLoggerOptions'; * * const backend = createBackend(); * backend.add(dynamicPluginsFeatureLoader({ * moduleLoader: myCustomModuleLoader, * schemaLocator: myCustomSchemaLocator, - * + * logger: (config) => myConfiguredLoggerOptions(config), * })); * //... * backend.start();