From 3ccb7fc151ba898141ac242c95ddc9dd6cdafe66 Mon Sep 17 00:00:00 2001 From: Paul Schultz Date: Tue, 27 May 2025 15:00:08 -0500 Subject: [PATCH] feat: propagate errors as objects and align Winston service Signed-off-by: Paul Schultz --- .changeset/eleven-foxes-mix.md | 5 ++ .../backend-defaults/report-auditor.api.md | 4 +- .../auditor/DefaultAuditorService.test.ts | 4 +- .../auditor/DefaultAuditorService.ts | 4 +- .../auditor/WinstonRootAuditorService.test.ts | 9 ++- .../auditor/WinstonRootAuditorService.ts | 26 ++++++-- .../auditor/auditorServiceFactory.ts | 63 +++++------------- .../src/entrypoints/auditor/types.ts | 26 ++++++++ .../src/entrypoints/auditor/utils.ts | 66 +++++++++++++++++++ 9 files changed, 150 insertions(+), 57 deletions(-) create mode 100644 .changeset/eleven-foxes-mix.md create mode 100644 packages/backend-defaults/src/entrypoints/auditor/types.ts create mode 100644 packages/backend-defaults/src/entrypoints/auditor/utils.ts diff --git a/.changeset/eleven-foxes-mix.md b/.changeset/eleven-foxes-mix.md new file mode 100644 index 0000000000..965be5435e --- /dev/null +++ b/.changeset/eleven-foxes-mix.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-defaults': minor +--- + +Enhanced error handling in the auditor service factory to pass errors as objects. Aligned WinstonRootAuditorService with the default service factory's error handling. diff --git a/packages/backend-defaults/report-auditor.api.md b/packages/backend-defaults/report-auditor.api.md index 353b6f241f..cc00b5c349 100644 --- a/packages/backend-defaults/report-auditor.api.md +++ b/packages/backend-defaults/report-auditor.api.md @@ -8,6 +8,7 @@ import type { AuditorServiceCreateEventOptions } from '@backstage/backend-plugin import type { AuditorServiceEvent } from '@backstage/backend-plugin-api'; import type { AuditorServiceEventSeverityLevel } from '@backstage/backend-plugin-api'; import type { AuthService } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; import type { Format } from 'logform'; import type { HttpAuthService } from '@backstage/backend-plugin-api'; import type { JsonObject } from '@backstage/types'; @@ -58,7 +59,7 @@ export type AuditorEventStatus = } | { status: 'failed'; - error: string; + error: Error; }; // @public @@ -95,6 +96,7 @@ export class WinstonRootAuditorService { // (undocumented) forPlugin(deps: { auth: AuthService; + config: Config; httpAuth: HttpAuthService; plugin: PluginMetadataService; }): AuditorService; diff --git a/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.test.ts b/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.test.ts index c960a1daa2..ac32c9e911 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.test.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.test.ts @@ -83,7 +83,7 @@ describe('DefaultAuditorService', () => { expect(logFn).toHaveBeenLastCalledWith({ eventId: 'test-event', status: 'failed', - error: error.toString(), + error, plugin: 'test', severityLevel: 'low', actor: {}, @@ -135,7 +135,7 @@ describe('DefaultAuditorService', () => { initiated: 'test', failed: 'test', }, - error: error.toString(), + error, plugin: 'test', severityLevel: 'low', actor: {}, diff --git a/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.ts b/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.ts index 79e206e305..b13565d841 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/DefaultAuditorService.ts @@ -48,7 +48,7 @@ export type AuditorEventStatus = | { status: 'succeeded' } | { status: 'failed'; - error: string; + error: Error; }; /** @@ -195,7 +195,7 @@ export class DefaultAuditorService implements AuditorService { await this.log({ ...options, ...params, - error: String(params.error), + error: params.error, meta: { ...options.meta, ...params?.meta }, status: 'failed', }); diff --git a/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.test.ts b/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.test.ts index 2d9a959c64..d8ffda2473 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.test.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.test.ts @@ -32,6 +32,7 @@ describe('WinstonRootAuditorService', () => { plugin: { getId: () => 'test-plugin', }, + config: mockServices.rootConfig.mock(), }); expect(childLogger).toBeInstanceOf(DefaultAuditorService); }); @@ -45,6 +46,7 @@ describe('WinstonRootAuditorService', () => { plugin: { getId: () => pluginId, }, + config: mockServices.rootConfig.mock(), }); // workaround to spy on private method const auditorSpy = jest.spyOn(auditor as any, 'log'); @@ -68,6 +70,7 @@ describe('WinstonRootAuditorService', () => { plugin: { getId: () => pluginId, }, + config: mockServices.rootConfig.mock(), }); // workaround to spy on private method const auditorSpy = jest.spyOn(auditor as any, 'log'); @@ -95,6 +98,7 @@ describe('WinstonRootAuditorService', () => { plugin: { getId: () => pluginId, }, + config: mockServices.rootConfig.mock(), }); // workaround to spy on private method const auditorSpy = jest.spyOn(auditor as any, 'log'); @@ -111,7 +115,7 @@ describe('WinstonRootAuditorService', () => { eventId: 'test-event', meta: {}, status: 'failed', - error: error.toString(), + error, }); }); @@ -124,6 +128,7 @@ describe('WinstonRootAuditorService', () => { plugin: { getId: () => pluginId, }, + config: mockServices.rootConfig.mock(), }); // workaround to spy on private method const auditorSpy = jest.spyOn(auditor as any, 'log'); @@ -163,7 +168,7 @@ describe('WinstonRootAuditorService', () => { initiated: 'test', failed: 'test', }, - error: error.toString(), + error, }); }); }); diff --git a/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.ts b/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.ts index 6ea5d02606..67a28cf74b 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/WinstonRootAuditorService.ts @@ -20,11 +20,13 @@ import type { HttpAuthService, PluginMetadataService, } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; import type { JsonObject } from '@backstage/types'; import type { Format } from 'logform'; import * as winston from 'winston'; import { WinstonLogger } from '../rootLogger'; import { DefaultAuditorService } from './DefaultAuditorService'; +import { getSeverityLogLevelMappings } from './utils'; /** @public */ export const defaultFormatter = winston.format.combine( @@ -108,12 +110,28 @@ export class WinstonRootAuditorService { forPlugin(deps: { auth: AuthService; + config: Config; httpAuth: HttpAuthService; plugin: PluginMetadataService; }): AuditorService { - return DefaultAuditorService.create( - e => this.winstonLogger.info(`${e.plugin}.${e.eventId}`, e), - deps, - ); + const severityLogLevelMappings = getSeverityLogLevelMappings(deps.config); + + return DefaultAuditorService.create(event => { + if ('error' in event) { + const { error, ...rest } = event; + const childAuditLogger = this.winstonLogger.child(rest); + + childAuditLogger[severityLogLevelMappings[event.severityLevel]]( + `${event.plugin}.${event.eventId}`, + error, + ); + } else { + // the else statement is required for typechecking + this.winstonLogger[severityLogLevelMappings[event.severityLevel]]( + `${event.plugin}.${event.eventId}`, + event, + ); + } + }, deps); } } diff --git a/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts b/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts index 34ea627762..3c82f95ce6 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts @@ -19,15 +19,7 @@ import { createServiceFactory, } from '@backstage/backend-plugin-api'; import { DefaultAuditorService } from './DefaultAuditorService'; -import { z } from 'zod'; -import { InputError } from '@backstage/errors'; - -const CONFIG_ROOT_KEY = 'backend.auditor'; - -const severityLogLevelMappingsSchema = z.record( - z.enum(['low', 'medium', 'high', 'critical']), - z.enum(['debug', 'info', 'warn', 'error']), -); +import { getSeverityLogLevelMappings } from './utils'; /** * Plugin-level auditing. @@ -49,47 +41,26 @@ export const auditorServiceFactory = createServiceFactory({ }, factory({ config, logger, plugin, auth, httpAuth }) { const auditLogger = logger.child({ isAuditEvent: true }); - const auditorConfig = config.getOptionalConfig(CONFIG_ROOT_KEY); - const severityLogLevelMappings = { - low: - auditorConfig?.getOptionalString('severityLogLevelMappings.low') ?? - 'debug', - medium: - auditorConfig?.getOptionalString('severityLogLevelMappings.medium') ?? - 'info', - high: - auditorConfig?.getOptionalString('severityLogLevelMappings.high') ?? - 'info', - critical: - auditorConfig?.getOptionalString('severityLogLevelMappings.critical') ?? - 'info', - } as Required>; - - const res = severityLogLevelMappingsSchema.safeParse( - severityLogLevelMappings, - ); - if (!res.success) { - const key = res.error.issues.at(0)?.path.at(0) as string; - const value = ( - res.error.issues.at(0) as unknown as Record - ).received as string; - const validKeys = ( - res.error.issues.at(0) as unknown as Record - ).options as string[]; - throw new InputError( - `The configuration value for 'backend.auditor.severityLogLevelMappings.${key}' was given an invalid value: '${value}'. Expected one of the following valid values: '${validKeys.join( - ', ', - )}'.`, - ); - } + const severityLogLevelMappings = getSeverityLogLevelMappings(config); return DefaultAuditorService.create( event => { - auditLogger[severityLogLevelMappings[event.severityLevel]]( - `${event.plugin}.${event.eventId}`, - event, - ); + if ('error' in event) { + const { error, ...rest } = event; + const childAuditLogger = auditLogger.child(rest); + + childAuditLogger[severityLogLevelMappings[event.severityLevel]]( + `${event.plugin}.${event.eventId}`, + error, + ); + } else { + // the else statement is required for typechecking + auditLogger[severityLogLevelMappings[event.severityLevel]]( + `${event.plugin}.${event.eventId}`, + event, + ); + } }, { plugin, auth, httpAuth }, ); diff --git a/packages/backend-defaults/src/entrypoints/auditor/types.ts b/packages/backend-defaults/src/entrypoints/auditor/types.ts new file mode 100644 index 0000000000..7827a31d1f --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/auditor/types.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { z } from 'zod'; + +/** @internal */ +export const severityLogLevelMappingsSchema = z.record( + z.enum(['low', 'medium', 'high', 'critical']), + z.enum(['debug', 'info', 'warn', 'error']), +); + +/** @internal */ +export const CONFIG_ROOT_KEY = 'backend.auditor'; diff --git a/packages/backend-defaults/src/entrypoints/auditor/utils.ts b/packages/backend-defaults/src/entrypoints/auditor/utils.ts new file mode 100644 index 0000000000..719a227bf9 --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/auditor/utils.ts @@ -0,0 +1,66 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Config } from '@backstage/config'; +import { InputError } from '@backstage/errors'; +import { z } from 'zod'; +import { CONFIG_ROOT_KEY, severityLogLevelMappingsSchema } from './types'; + +/** + * Gets the `backend.auditor.severityLogLevelMappings` configuration. + * + * @param config - The root Backstage {@link @backstage/config#Config} object. + * @returns The validated severity-to-log-level mappings. + * @throws error - {@link @backstage/errors#InputError} if the mapping configuration is invalid. + */ +export function getSeverityLogLevelMappings(config: Config) { + const auditorConfig = config.getOptionalConfig(CONFIG_ROOT_KEY); + + const severityLogLevelMappings = { + low: + auditorConfig?.getOptionalString('severityLogLevelMappings.low') ?? + 'debug', + medium: + auditorConfig?.getOptionalString('severityLogLevelMappings.medium') ?? + 'info', + high: + auditorConfig?.getOptionalString('severityLogLevelMappings.high') ?? + 'info', + critical: + auditorConfig?.getOptionalString('severityLogLevelMappings.critical') ?? + 'info', + } as Required>; + + const res = severityLogLevelMappingsSchema.safeParse( + severityLogLevelMappings, + ); + if (!res.success) { + const key = res.error.issues.at(0)?.path.at(0) as string; + const value = ( + res.error.issues.at(0) as unknown as Record + ).received as string; + const validKeys = ( + res.error.issues.at(0) as unknown as Record + ).options as string[]; + throw new InputError( + `The configuration value for 'backend.auditor.severityLogLevelMappings.${key}' was given an invalid value: '${value}'. Expected one of the following valid values: '${validKeys.join( + ', ', + )}'.`, + ); + } + + return severityLogLevelMappings; +}