From f7b4c64f3d346b3b09a1c22731a9ace00ff5e3e4 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Fri, 18 Nov 2022 15:52:55 +0400 Subject: [PATCH 1/8] bug(logger): fix bug when child logger with overwritten options clears all other options --- packages/logger/src/Logger.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 57b9ba1a57..8e1321d9d3 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -3,7 +3,6 @@ import type { Context, Handler } from 'aws-lambda'; import { Utility } from '@aws-lambda-powertools/commons'; import { LogFormatterInterface, PowertoolLogFormatter } from './formatter'; import { LogItem } from './log'; -import cloneDeep from 'lodash.clonedeep'; import merge from 'lodash.merge'; import { ConfigServiceInterface, EnvironmentVariablesService } from './config'; import { LogJsonIndent } from './types'; @@ -122,6 +121,8 @@ class Logger extends Utility implements ClassThatLogs { // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; + + private initOptions: ConstructorOptions; private logEvent: boolean = false; @@ -151,7 +152,7 @@ class Logger extends Utility implements ClassThatLogs { */ public constructor(options: ConstructorOptions = {}) { super(); - + this.initOptions = options; this.setOptions(options); } @@ -205,7 +206,7 @@ class Logger extends Utility implements ClassThatLogs { * @returns {Logger} */ public createChild(options: ConstructorOptions = {}): Logger { - return cloneDeep(this).setOptions(options); + return new Logger(merge({}, this.initOptions, options)); } /** From dab8c6dd1b1c438c2ea8f224b3bcf7c0e6ce0b16 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Fri, 18 Nov 2022 15:54:51 +0400 Subject: [PATCH 2/8] test(logger): add tests for createChild method --- packages/logger/tests/unit/Logger.test.ts | 127 ++++++++++++++++++++- packages/logger/tests/unit/helpers.test.ts | 5 +- 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index e5fe9b39b7..3a917fd352 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -568,6 +568,7 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -1236,6 +1237,110 @@ describe('Class: Logger', () => { describe('Method: createChild', () => { + test('Child and grandchild loggers should have all ancestor\'s options', () => { + // Prepare + const INDENTATION = LogJsonIndent.COMPACT; + const loggerOptions = { + serviceName: 'parent-service-name', + sampleRateValue: 0.01, + }; + const parentLogger = new Logger(loggerOptions); + + // Act + const childLoggerOptions = { sampleRateValue: 1 }; + const childLogger = parentLogger.createChild(childLoggerOptions); + + const grandchildLoggerOptions = { serviceName: 'grandchild-logger-name' }; + const grandchildLogger = childLogger.createChild(grandchildLoggerOptions); + + // Assess + expect(parentLogger === childLogger).toBe(false); + expect(childLogger === grandchildLogger).toBe(false); + expect(parentLogger === grandchildLogger).toBe(false); + + expect(parentLogger).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: loggerOptions, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: false, + persistentLogAttributes: {}, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: 0.01, + serviceName: 'parent-service-name', + }, + }); + + expect(childLogger).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: { ...loggerOptions, ...childLoggerOptions }, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: true, + persistentLogAttributes: {}, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: 1, + serviceName: 'parent-service-name', + }, + }); + + expect(grandchildLogger).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: { ...childLoggerOptions ,...grandchildLoggerOptions }, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: true, + persistentLogAttributes: {}, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: 1, + serviceName: 'grandchild-logger-name', + }, + }); + + }); + test('when called, it returns a DISTINCT clone of the logger instance', () => { // Prepare @@ -1244,17 +1349,23 @@ describe('Class: Logger', () => { // Act const childLogger = parentLogger.createChild(); - const childLoggerWithPermanentAttributes = parentLogger.createChild({ + + const optionsWithPermanentAttributes = { persistentLogAttributes: { extra: 'This is an attribute that will be logged only by the child logger', }, - }); - const childLoggerWithSampleRateEnabled = parentLogger.createChild({ + }; + const childLoggerWithPermanentAttributes = parentLogger.createChild(optionsWithPermanentAttributes); + + const optionsWithSampleRateEnabled = { sampleRateValue: 1, // 100% probability to make sure that the logs are sampled - }); - const childLoggerWithErrorLogLevel = parentLogger.createChild({ + }; + const childLoggerWithSampleRateEnabled = parentLogger.createChild(optionsWithSampleRateEnabled); + + const optionsWithErrorLogLevel = { logLevel: 'ERROR', - }); + }; + const childLoggerWithErrorLogLevel = parentLogger.createChild(optionsWithErrorLogLevel); // Assess expect(parentLogger === childLogger).toBe(false); @@ -1272,6 +1383,7 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1298,6 +1410,7 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: optionsWithPermanentAttributes, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1326,6 +1439,7 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: optionsWithSampleRateEnabled, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1352,6 +1466,7 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: optionsWithErrorLogLevel, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), diff --git a/packages/logger/tests/unit/helpers.test.ts b/packages/logger/tests/unit/helpers.test.ts index 3a4298421b..96c2bf6394 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -78,6 +78,7 @@ describe('Helper: createLogger function', () => { defaultServiceName: 'service_undefined', customConfigService: expect.any(EnvironmentVariablesService), envVarsService: expect.any(EnvironmentVariablesService), + initOptions: loggerOptions, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -120,6 +121,7 @@ describe('Helper: createLogger function', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -230,7 +232,7 @@ describe('Helper: createLogger function', () => { test('when no log level is set, returns a Logger instance with INFO level', () => { // Prepare - const loggerOptions:ConstructorOptions = {}; + const loggerOptions: ConstructorOptions = {}; delete process.env.LOG_LEVEL; // Act @@ -243,6 +245,7 @@ describe('Helper: createLogger function', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), + initOptions: loggerOptions, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), From 92f22742c405d992220546e2a9a8f8c869ad2d48 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Fri, 18 Nov 2022 19:16:22 +0400 Subject: [PATCH 3/8] refactor(logger): remove clonedeep dependency --- package-lock.json | 48 +++++++++--------------------------- packages/logger/package.json | 2 -- 2 files changed, 12 insertions(+), 38 deletions(-) diff --git a/package-lock.json b/package-lock.json index e743677bb0..8413791a0d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3915,15 +3915,6 @@ "integrity": "sha512-XOKXa1KIxtNXgASAnwj7cnttJxS4fksBRywK/9LzRV5YxrF80BXZIGeQSuoESQ/VkUj30Ae0+YcuHc15wJCB2g==", "dev": true }, - "node_modules/@types/lodash.clonedeep": { - "version": "4.5.7", - "resolved": "https://registry.npmjs.org/@types/lodash.clonedeep/-/lodash.clonedeep-4.5.7.tgz", - "integrity": "sha512-ccNqkPptFIXrpVqUECi60/DFxjNKsfoQxSQsgcBJCX/fuX1wgyQieojkcWH/KpE3xzLoWN/2k+ZeGqIN3paSvw==", - "dev": true, - "dependencies": { - "@types/lodash": "*" - } - }, "node_modules/@types/lodash.merge": { "version": "4.6.7", "resolved": "https://registry.npmjs.org/@types/lodash.merge/-/lodash.merge-4.6.7.tgz", @@ -11059,11 +11050,6 @@ "integrity": "sha1-DM8tiRZq8Ds2Y8eWU4t1rG4RTZ0=", "dev": true }, - "node_modules/lodash.clonedeep": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz", - "integrity": "sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8=" - }, "node_modules/lodash.defaults": { "version": "4.2.0", "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", @@ -15542,28 +15528,30 @@ }, "packages/commons": { "name": "@aws-lambda-powertools/commons", - "version": "1.4.0", + "version": "1.4.1", "license": "MIT-0" }, "packages/logger": { "name": "@aws-lambda-powertools/logger", - "version": "1.4.0", + "version": "1.4.1", "license": "MIT", "dependencies": { - "lodash.clonedeep": "^4.5.0", + "@aws-lambda-powertools/commons": "^1.4.1", "lodash.merge": "^4.6.2", "lodash.pickby": "^4.6.0" }, "devDependencies": { - "@types/lodash.clonedeep": "^4.5.7", "@types/lodash.merge": "^4.6.7", "@types/lodash.pickby": "^4.6.7" } }, "packages/metrics": { "name": "@aws-lambda-powertools/metrics", - "version": "1.4.0", + "version": "1.4.1", "license": "MIT-0", + "dependencies": { + "@aws-lambda-powertools/commons": "^1.4.1" + }, "devDependencies": { "@types/promise-retry": "^1.1.3", "promise-retry": "^2.0.1" @@ -15571,9 +15559,10 @@ }, "packages/tracer": { "name": "@aws-lambda-powertools/tracer", - "version": "1.4.0", + "version": "1.4.1", "license": "MIT-0", "dependencies": { + "@aws-lambda-powertools/commons": "^1.4.1", "aws-xray-sdk-core": "^3.3.6" }, "devDependencies": { @@ -15800,10 +15789,9 @@ "@aws-lambda-powertools/logger": { "version": "file:packages/logger", "requires": { - "@types/lodash.clonedeep": "^4.5.7", + "@aws-lambda-powertools/commons": "^1.4.1", "@types/lodash.merge": "^4.6.7", "@types/lodash.pickby": "^4.6.7", - "lodash.clonedeep": "^4.5.0", "lodash.merge": "^4.6.2", "lodash.pickby": "^4.6.0" } @@ -15811,6 +15799,7 @@ "@aws-lambda-powertools/metrics": { "version": "file:packages/metrics", "requires": { + "@aws-lambda-powertools/commons": "^1.4.1", "@types/promise-retry": "^1.1.3", "promise-retry": "^2.0.1" } @@ -15818,6 +15807,7 @@ "@aws-lambda-powertools/tracer": { "version": "file:packages/tracer", "requires": { + "@aws-lambda-powertools/commons": "^1.4.1", "@aws-sdk/client-dynamodb": "^3.100.0", "@types/promise-retry": "^1.1.3", "aws-xray-sdk-core": "^3.3.6", @@ -18772,15 +18762,6 @@ "integrity": "sha512-XOKXa1KIxtNXgASAnwj7cnttJxS4fksBRywK/9LzRV5YxrF80BXZIGeQSuoESQ/VkUj30Ae0+YcuHc15wJCB2g==", "dev": true }, - "@types/lodash.clonedeep": { - "version": "4.5.7", - "resolved": "https://registry.npmjs.org/@types/lodash.clonedeep/-/lodash.clonedeep-4.5.7.tgz", - "integrity": "sha512-ccNqkPptFIXrpVqUECi60/DFxjNKsfoQxSQsgcBJCX/fuX1wgyQieojkcWH/KpE3xzLoWN/2k+ZeGqIN3paSvw==", - "dev": true, - "requires": { - "@types/lodash": "*" - } - }, "@types/lodash.merge": { "version": "4.6.7", "resolved": "https://registry.npmjs.org/@types/lodash.merge/-/lodash.merge-4.6.7.tgz", @@ -24326,11 +24307,6 @@ "integrity": "sha1-DM8tiRZq8Ds2Y8eWU4t1rG4RTZ0=", "dev": true }, - "lodash.clonedeep": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz", - "integrity": "sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8=" - }, "lodash.defaults": { "version": "4.2.0", "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", diff --git a/packages/logger/package.json b/packages/logger/package.json index 84478e69b9..211969e096 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -32,7 +32,6 @@ "types": "./lib/index.d.ts", "typedocMain": "src/index.ts", "devDependencies": { - "@types/lodash.clonedeep": "^4.5.7", "@types/lodash.merge": "^4.6.7", "@types/lodash.pickby": "^4.6.7" }, @@ -48,7 +47,6 @@ }, "dependencies": { "@aws-lambda-powertools/commons": "^1.4.1", - "lodash.clonedeep": "^4.5.0", "lodash.merge": "^4.6.2", "lodash.pickby": "^4.6.0" }, From fdd9f5122acc49dbaf25dcd8005547db75c18953 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Fri, 18 Nov 2022 19:21:24 +0400 Subject: [PATCH 4/8] refactor(logger): move initOptions initializer to setOptions --- packages/logger/src/Logger.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 8e1321d9d3..6f52317b4b 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -122,7 +122,8 @@ class Logger extends Utility implements ClassThatLogs { // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; - private initOptions: ConstructorOptions; + // initOptions is initialized in the constructor in setOptions() + private initOptions!: ConstructorOptions; private logEvent: boolean = false; @@ -152,7 +153,6 @@ class Logger extends Utility implements ClassThatLogs { */ public constructor(options: ConstructorOptions = {}) { super(); - this.initOptions = options; this.setOptions(options); } @@ -681,6 +681,17 @@ class Logger extends Utility implements ClassThatLogs { this.envVarsService = new EnvironmentVariablesService(); } + /** + * It initialize and store options, used to create an instance of Logger + * + * @private + * @param {ConstructorOptions} options + * @returns {void} + */ + private setInitOptions(options: ConstructorOptions): void { + this.initOptions = options; + } + /** * If the log event feature is enabled via env variable, it sets a property that tracks whether * the event passed to the Lambda function handler should be logged or not. @@ -780,6 +791,7 @@ class Logger extends Utility implements ClassThatLogs { environment, } = options; + this.setInitOptions(options); this.setEnvVarsService(); // order is important, it uses EnvVarsService() this.setConsole(); From 12f438b3376b5020611ce6ad9623c45574fd1b64 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Tue, 22 Nov 2022 15:50:13 +0400 Subject: [PATCH 5/8] feat(logger): add addPersistentLogAttributes from parent to child logger --- packages/logger/src/Logger.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 6f52317b4b..49ba1bc77e 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -206,7 +206,11 @@ class Logger extends Utility implements ClassThatLogs { * @returns {Logger} */ public createChild(options: ConstructorOptions = {}): Logger { - return new Logger(merge({}, this.initOptions, options)); + const childLogger = new Logger(merge({}, this.initOptions, options)); + const parentsPersistentLogAttributes = this.getPersistentLogAttributes(); + childLogger.addPersistentLogAttributes(parentsPersistentLogAttributes); + + return childLogger; } /** From 2b9121399a7185168a641410a0666ff75f468039 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Tue, 22 Nov 2022 15:55:26 +0400 Subject: [PATCH 6/8] test(logger): add tests for createChild with persistent log attrs --- packages/logger/tests/unit/Logger.test.ts | 118 ++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 3a917fd352..0f47ebf102 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -1489,6 +1489,124 @@ describe('Class: Logger', () => { }); + test('child logger should have parent\'s keys in persistentLogAttributes', () => { + + // Prepare + const INDENTATION = LogJsonIndent.COMPACT; + const parentLogger = new Logger(); + const childLogger = parentLogger.createChild(); + + // Act + parentLogger.appendKeys({ + aws_account_id: '123456789012', + aws_region: 'eu-west-1', + logger: { + name: 'aws-lambda-powertool-typescript', + version: '0.2.4', + }, + test_key: 'key-for-test' + }); + const childLoggerWithKeys = parentLogger.createChild(); + childLoggerWithKeys.removeKeys(['test_key']); + + // Assess + expect(childLogger).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: false, + persistentLogAttributes: {}, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: undefined, + serviceName: 'hello-world', + }, + }); + + expect(childLoggerWithKeys).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: false, + persistentLogAttributes: { + aws_account_id: '123456789012', + aws_region: 'eu-west-1', + logger: { + name: 'aws-lambda-powertool-typescript', + version: '0.2.4', + }, + }, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: undefined, + serviceName: 'hello-world', + }, + }); + + expect(parentLogger).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + initOptions: {}, + logEvent: false, + logIndentation: INDENTATION, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: false, + persistentLogAttributes: { + aws_account_id: '123456789012', + aws_region: 'eu-west-1', + logger: { + name: 'aws-lambda-powertool-typescript', + version: '0.2.4', + }, + test_key: 'key-for-test', + }, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + sampleRateValue: undefined, + serviceName: 'hello-world', + }, + }); + }); + }); describe('Method: logEventIfEnabled', () => { From f1c24831de109057a6220b0a6c48f72f8ed185d4 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Tue, 22 Nov 2022 17:02:56 +0400 Subject: [PATCH 7/8] feat(logger): added addContext support, remove redundant initOption --- packages/logger/src/Logger.ts | 20 ++---- packages/logger/tests/unit/Logger.test.ts | 71 ++++++++++++++++++---- packages/logger/tests/unit/helpers.test.ts | 3 - 3 files changed, 64 insertions(+), 30 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 49ba1bc77e..d579c6574e 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -121,9 +121,6 @@ class Logger extends Utility implements ClassThatLogs { // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; - - // initOptions is initialized in the constructor in setOptions() - private initOptions!: ConstructorOptions; private logEvent: boolean = false; @@ -206,9 +203,12 @@ class Logger extends Utility implements ClassThatLogs { * @returns {Logger} */ public createChild(options: ConstructorOptions = {}): Logger { - const childLogger = new Logger(merge({}, this.initOptions, options)); + const parentsPowertoolsLogData = this.getPowertoolLogData(); + const childLogger = new Logger(merge({}, parentsPowertoolsLogData, options)); const parentsPersistentLogAttributes = this.getPersistentLogAttributes(); childLogger.addPersistentLogAttributes(parentsPersistentLogAttributes); + const childPowertoolsLogData = childLogger.getPowertoolLogData(); + childLogger.addToPowertoolLogData(merge({}, parentsPowertoolsLogData, childPowertoolsLogData)); return childLogger; } @@ -685,17 +685,6 @@ class Logger extends Utility implements ClassThatLogs { this.envVarsService = new EnvironmentVariablesService(); } - /** - * It initialize and store options, used to create an instance of Logger - * - * @private - * @param {ConstructorOptions} options - * @returns {void} - */ - private setInitOptions(options: ConstructorOptions): void { - this.initOptions = options; - } - /** * If the log event feature is enabled via env variable, it sets a property that tracks whether * the event passed to the Lambda function handler should be logged or not. @@ -795,7 +784,6 @@ class Logger extends Utility implements ClassThatLogs { environment, } = options; - this.setInitOptions(options); this.setEnvVarsService(); // order is important, it uses EnvVarsService() this.setConsole(); diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 0f47ebf102..c9c7b94f35 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -568,7 +568,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -1264,7 +1263,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: loggerOptions, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1291,7 +1289,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: { ...loggerOptions, ...childLoggerOptions }, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1318,7 +1315,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: { ...childLoggerOptions ,...grandchildLoggerOptions }, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1383,7 +1379,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1410,7 +1405,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: optionsWithPermanentAttributes, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1439,7 +1433,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: optionsWithSampleRateEnabled, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1466,7 +1459,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: optionsWithErrorLogLevel, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1516,7 +1508,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1543,7 +1534,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1577,7 +1567,6 @@ describe('Class: Logger', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: INDENTATION, logFormatter: expect.any(PowertoolLogFormatter), @@ -1606,6 +1595,66 @@ describe('Class: Logger', () => { }, }); }); + + const context = { + callbackWaitsForEmptyEventLoop: true, + functionVersion: '$LATEST', + functionName: 'foo-bar-function-with-cold-start', + memoryLimitInMB: '128', + logGroupName: '/aws/lambda/foo-bar-function-with-cold-start', + logStreamName: '2021/03/09/[$LATEST]1-5759e988-bd862e3fe1be46a994272793', + invokedFunctionArn: 'arn:aws:lambda:eu-west-1:123456789012:function:foo-bar-function-with-cold-start', + awsRequestId: 'c6af9ac6-7b61-11e6-9a41-93e812345678', + getRemainingTimeInMillis: () => 1234, + done: () => console.log('Done!'), + fail: () => console.log('Failed!'), + succeed: () => console.log('Succeeded!'), + }; + + test('child logger should have parent\'s context in PowertoolLogData', () => { + + // Prepare + const parentLogger = new Logger(); + + // Act + parentLogger.addContext(context); + const childLoggerWithContext = parentLogger.createChild(); + + // Assess + expect(childLoggerWithContext).toEqual({ + console: expect.any(Console), + coldStart: true, + customConfigService: undefined, + defaultServiceName: 'service_undefined', + envVarsService: expect.any(EnvironmentVariablesService), + logEvent: false, + logIndentation: 0, + logFormatter: expect.any(PowertoolLogFormatter), + logLevel: 'DEBUG', + logLevelThresholds: { + DEBUG: 8, + ERROR: 20, + INFO: 12, + WARN: 16, + }, + logsSampled: false, + persistentLogAttributes: {}, + powertoolLogData: { + awsRegion: 'eu-west-1', + environment: '', + lambdaContext: { + awsRequestId: 'c6af9ac6-7b61-11e6-9a41-93e812345678', + coldStart: true, + functionName: 'foo-bar-function-with-cold-start', + functionVersion: '$LATEST', + invokedFunctionArn: 'arn:aws:lambda:eu-west-1:123456789012:function:foo-bar-function-with-cold-start', + memoryLimitInMB: 128, + }, + sampleRateValue: undefined, + serviceName: 'hello-world', + }, + }); + }); }); diff --git a/packages/logger/tests/unit/helpers.test.ts b/packages/logger/tests/unit/helpers.test.ts index 96c2bf6394..4f953620fc 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -78,7 +78,6 @@ describe('Helper: createLogger function', () => { defaultServiceName: 'service_undefined', customConfigService: expect.any(EnvironmentVariablesService), envVarsService: expect.any(EnvironmentVariablesService), - initOptions: loggerOptions, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -121,7 +120,6 @@ describe('Helper: createLogger function', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: {}, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), @@ -245,7 +243,6 @@ describe('Helper: createLogger function', () => { customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), - initOptions: loggerOptions, logEvent: false, logIndentation: 0, logFormatter: expect.any(PowertoolLogFormatter), From 6ed44f4ba3db0d3d50a36c2fac6376eac8fc135a Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Wed, 23 Nov 2022 10:01:01 +0400 Subject: [PATCH 8/8] refactor(logger): add parent's context to child explicitly, without merging --- packages/logger/src/Logger.ts | 7 +++++-- packages/logger/tests/unit/Logger.test.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index d579c6574e..6a57f92eb4 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -205,10 +205,13 @@ class Logger extends Utility implements ClassThatLogs { public createChild(options: ConstructorOptions = {}): Logger { const parentsPowertoolsLogData = this.getPowertoolLogData(); const childLogger = new Logger(merge({}, parentsPowertoolsLogData, options)); + const parentsPersistentLogAttributes = this.getPersistentLogAttributes(); childLogger.addPersistentLogAttributes(parentsPersistentLogAttributes); - const childPowertoolsLogData = childLogger.getPowertoolLogData(); - childLogger.addToPowertoolLogData(merge({}, parentsPowertoolsLogData, childPowertoolsLogData)); + + if (parentsPowertoolsLogData.lambdaContext) { + childLogger.addContext(parentsPowertoolsLogData.lambdaContext as Context); + } return childLogger; } diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index c9c7b94f35..03d7c7ae9b 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -1623,7 +1623,7 @@ describe('Class: Logger', () => { // Assess expect(childLoggerWithContext).toEqual({ console: expect.any(Console), - coldStart: true, + coldStart: false, // This is now false because the `coldStart` attribute has been already accessed once by the `addContext` method customConfigService: undefined, defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService),