From 89f1c4238d6108d084be53a2189224340f6ab184 Mon Sep 17 00:00:00 2001 From: ZhongpinWang <101632744+ZhongpinWang@users.noreply.github.com> Date: Wed, 25 May 2022 10:01:30 +0200 Subject: [PATCH] feat: use NODE_ENV and cds.env.features.kibana_formatter for format (#2481) * chore: use NODE_ENV and cds.env.features.kibana_formatter for format * docs: add compatibility note using changeset * fix: add missing dependency * fix: lint * fix: rewrite logic for cds kibana settings * fix:lint * chore: improve the format logic * feat: support changing log format * fix: add missing docs * fix: lint * fix: lint * docs: update changelog --- .changeset/silent-cougars-bake.md | 5 ++ .../util/src/logger/cloud-sdk-logger.spec.ts | 80 ++++++++++++++--- packages/util/src/logger/cloud-sdk-logger.ts | 89 ++++++++++++++++++- .../test/exception-logger.spec.ts | 10 ++- 4 files changed, 164 insertions(+), 20 deletions(-) create mode 100644 .changeset/silent-cougars-bake.md diff --git a/.changeset/silent-cougars-bake.md b/.changeset/silent-cougars-bake.md new file mode 100644 index 0000000000..1b0df312b3 --- /dev/null +++ b/.changeset/silent-cougars-bake.md @@ -0,0 +1,5 @@ +--- +'@sap-cloud-sdk/util': minor +--- + +[Compatibility Note] Stop using `VCAP_SERVICES` to determine the log format. Use `setLogFormat` and `setGlobalLogFormat` to specify the log format. By default, the log format is set to `kibana` for `NODE_ENV=production` and `local` otherwise. diff --git a/packages/util/src/logger/cloud-sdk-logger.spec.ts b/packages/util/src/logger/cloud-sdk-logger.spec.ts index 6e7a30a8c7..036f523d03 100755 --- a/packages/util/src/logger/cloud-sdk-logger.spec.ts +++ b/packages/util/src/logger/cloud-sdk-logger.spec.ts @@ -3,11 +3,17 @@ import { createLogger, disableExceptionLogger, enableExceptionLogger, + getGlobalLogFormat, getGlobalLogLevel, getLogger, + logFormat, muteLoggers, + resetCustomLogFormats, + resetCustomLogLevels, sanitizeRecord, + setGlobalLogFormat, setGlobalLogLevel, + setLogFormat, setLogLevel, unmuteLoggers } from './cloud-sdk-logger'; @@ -19,8 +25,13 @@ describe('Cloud SDK Logger', () => { let logger; afterEach(() => { - logger.close(); + if (logger) { + logger.close(); + } setLogLevel('', messageContext); + setGlobalLogFormat(logFormat.local); + resetCustomLogLevels(); + resetCustomLogFormats(); }); describe('createLogger', () => { @@ -183,41 +194,34 @@ describe('Cloud SDK Logger', () => { describe('set log level', () => { const level = 'silly'; - it('before creating a logger, should set the log level on creation', () => { + it('should set the log level on creation before creating a logger', () => { setLogLevel(level, messageContext); logger = createLogger(messageContext); expect(logger.level).toEqual(level); }); - it('after creating a logger, should set the log level on creation', () => { + it('should set the log level with messageContext after creating a logger', () => { logger = createLogger(messageContext); setLogLevel(level, messageContext); expect(logger.level).toEqual(level); }); - it('on a logger should set the log level', () => { + it('should set the log level with logger after creating a logger', () => { logger = createLogger(messageContext); setLogLevel(level, logger); expect(logger.level).toEqual(level); }); - - it('set global log level after logger creation should override the log level', () => { - logger = createLogger({ messageContext, level }); - setGlobalLogLevel('error'); - - expect(getGlobalLogLevel()).toEqual(logger.level); - }); }); describe('set global log level', () => { const level = 'error'; - beforeAll(() => { + beforeEach(() => { setGlobalLogLevel(level); }); it('global log level getter and setter work', () => { - expect(level).toEqual(getGlobalLogLevel()); + expect(getGlobalLogLevel()).toEqual(level); }); it('should have the global log level, if not applied a more specific level', () => { @@ -241,6 +245,56 @@ describe('Cloud SDK Logger', () => { }); }); + describe('set log format', () => { + it('should set the log format on creation before creating a logger', () => { + setLogFormat(logFormat.kibana, messageContext); + logger = createLogger(messageContext); + expect(logger.format).toEqual(logFormat.kibana); + }); + + it('should set the log format with messageContext after creating a logger', () => { + logger = createLogger(messageContext); + setLogFormat(logFormat.kibana, messageContext); + expect(logger.format).toEqual(logFormat.kibana); + }); + + it('should set the log format with logger after creating a logger', () => { + logger = createLogger(messageContext); + setLogFormat(logFormat.kibana, logger); + expect(logger.format).toEqual(logFormat.kibana); + }); + }); + + describe('set global log format', () => { + beforeEach(() => { + setGlobalLogFormat(logFormat.kibana); + }); + + it('global log format getter and setter work', () => { + expect(getGlobalLogFormat()).toEqual(logFormat.kibana); + }); + + it('should have the global log format, if not applied a more specific format', () => { + logger = createLogger(messageContext); + + expect(logger.format).toEqual(logFormat.kibana); + }); + + it('should have the log format, if applied a more specific format after creation', () => { + logger = createLogger(messageContext); + setLogFormat(logFormat.kibana, messageContext); + + expect(logger.format).toEqual(logFormat.kibana); + }); + + it('should have the log format, if applied a more specific format before creation', () => { + setLogFormat(logFormat.kibana, messageContext); + logger = createLogger(messageContext); + + expect(logger.format).toEqual(logFormat.kibana); + }); + }); + describe('sanitize records', () => { it('does not remove any unwanted record properties for common response headers', () => { const responseHeaders = { diff --git a/packages/util/src/logger/cloud-sdk-logger.ts b/packages/util/src/logger/cloud-sdk-logger.ts index 0878669e6c..542ae41f37 100755 --- a/packages/util/src/logger/cloud-sdk-logger.ts +++ b/packages/util/src/logger/cloud-sdk-logger.ts @@ -1,3 +1,4 @@ +import { Format } from 'logform'; import { Container, Logger, @@ -6,13 +7,26 @@ import { } from 'winston'; import { kibana, local } from './format'; -const format = process.env.VCAP_SERVICES ? kibana : local; const loggerReference = 'sap-cloud-sdk-logger'; const exceptionLoggerId = 'sap-cloud-sdk-exception-logger'; const container = new Container(); + +/** + * Log formats provided by the util package. + */ +export const logFormat = { + kibana, + local +}; + +// Set default format based on NODE_ENV +container.options.format = + process.env.NODE_ENV === 'production' ? logFormat.kibana : logFormat.local; + const exceptionTransport = new transports.Console(); const customLogLevels = {}; +const customLogFormats = {}; const DEFAULT_LOGGER__MESSAGE_CONTEXT = '__DEFAULT_LOGGER__MESSAGE_CONTEXT'; let silent = false; @@ -49,7 +63,7 @@ export function unmuteLoggers(): void { */ export const cloudSdkExceptionLogger = container.get(exceptionLoggerId, { defaultMeta: { logger: loggerReference, test: 'exception' }, - format, + format: container.options.format, exceptionHandlers: [exceptionTransport] }); @@ -112,7 +126,11 @@ export function createLogger( }), logger: customFields.logger || loggerReference }, - format, + format: + customLogFormats[customFields.messageContext] || + customFields.format || + container.options.format || + logFormat.local, transports: [new transports.Console()] }); @@ -185,6 +203,57 @@ export function getGlobalLogLevel(): string | undefined { return container.options.level; } +/** + * Change the log format of a logger based on its message context. + * e.g., to set the log format for the destination accessor module of the SDK to `local`, simply call `setLogFormat(logFormat.local, 'destination-accessor')`. + * @param format - Format to set the logger to. Use `logFormat` to get the pre-defined log formats or use a custom log format. + * @param messageContextOrLogger - Message context of the logger to change the log level for or the logger itself. + */ +export function setLogFormat( + format: Format, + messageContextOrLogger: string | Logger = DEFAULT_LOGGER__MESSAGE_CONTEXT +): void { + const messageContext = + typeof messageContextOrLogger === 'string' + ? messageContextOrLogger + : getMessageContext(messageContextOrLogger); + + if (messageContext) { + customLogFormats[messageContext] = format; + + if (container.has(messageContext)) { + const logger = container.get(messageContext); + logger.format = format; + } + } else if (typeof messageContextOrLogger !== 'string') { + moduleLogger.warn( + 'Setting log format for logger with unknown message context' + ); + messageContextOrLogger.format = format; + } +} + +/** + * Change the global log format of the container which will set default format for all active loggers. + * e.g., to set the global log format to `local` call `setGlobalLogLevel(logFormat.local)` or use a custom log format. + * @param format - The log format to set the global log format to. + */ +export function setGlobalLogFormat(format: Format): void { + container.options.format = format; + // Update existing loggers' log level with global level. + container.loggers.forEach(logger => { + logger.format = format; + }); +} + +/** + * Get the global log format of the container. + * @returns The global log format, or `undefined` when not defined. + */ +export function getGlobalLogFormat(): Format | undefined { + return container.options.format; +} + const defaultSensitiveKeys = [ 'access_token', 'authentication', @@ -257,6 +326,20 @@ function getMessageContext(logger: Logger): string | undefined { } } +/** + * Reset all the custom log levels for loggers and message context. + */ +export function resetCustomLogLevels(): void { + Object.keys(customLogLevels).forEach(key => delete customLogLevels[key]); +} + +/** + * Reset all the custom log formats for loggers and message context. + */ +export function resetCustomLogFormats(): void { + Object.keys(customLogFormats).forEach(key => delete customLogFormats[key]); +} + /** * Npm log levels used for the SAP Cloud SDK logger. */ diff --git a/test-packages/integration-tests/test/exception-logger.spec.ts b/test-packages/integration-tests/test/exception-logger.spec.ts index 901b6c474d..ccbae4363c 100755 --- a/test-packages/integration-tests/test/exception-logger.spec.ts +++ b/test-packages/integration-tests/test/exception-logger.spec.ts @@ -12,22 +12,24 @@ describe('exception logger', () => { } }); - it('should log exception with stack if they fly locally', async () => { + it('should log exception with local format if they fly in development mode', async () => { + process.env.NODE_ENV = 'development'; await expect( execa('ts-node', [ resolve(__dirname, 'throw-exception-with-logger-script.ts') ]) ).rejects.toThrowError(/Test Exception Logger\n\s*at Object/); + delete process.env.NODE_ENV; }, 15000); - it('should log exception with stack if they fly on CF', async () => { - process.env.VCAP_SERVICES = 'exists'; + it('should log exception with kibana format if they fly in production mode', async () => { + process.env.NODE_ENV = 'production'; await expect( execa('ts-node', [ resolve(__dirname, 'throw-exception-with-logger-script.ts') ]) ).rejects.toThrowError(/Test Exception Logger\\n\s*at Object/); - delete process.env.VCAP_SERVICES; + delete process.env.NODE_ENV; }, 15000); it('should not log the stack multiple times', async () => {