From eabc8e34845059e58fabdc3d8e7a8ad96a6077c2 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Thu, 2 Feb 2023 14:50:35 +0400 Subject: [PATCH 1/2] fix(logger): fix a bug when child logger did get all the parent's attributes --- packages/logger/src/Logger.ts | 7 +- packages/logger/tests/unit/Logger.test.ts | 93 ++++++++++++++++++++++- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index a8ef03c8b5..575f24c12f 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -203,8 +203,13 @@ class Logger extends Utility implements ClassThatLogs { * @returns {Logger} */ public createChild(options: ConstructorOptions = {}): Logger { + const parentsOptions = { + logLevel: this.getLogLevel(), + customConfigService: this.getCustomConfigService(), + logFormatter: this.getLogFormatter(), + }; const parentsPowertoolsLogData = this.getPowertoolLogData(); - const childLogger = new Logger(merge({}, parentsPowertoolsLogData, options)); + const childLogger = new Logger(merge({}, parentsOptions, parentsPowertoolsLogData, options)); const parentsPersistentLogAttributes = this.getPersistentLogAttributes(); childLogger.addPersistentLogAttributes(parentsPersistentLogAttributes); diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 717c964ad1..e2fc35ca4b 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -8,7 +8,7 @@ import { ContextExamples as dummyContext, Events as dummyEvent, LambdaInterface import { createLogger, Logger } from '../../src'; import { EnvironmentVariablesService } from '../../src/config'; import { PowertoolLogFormatter } from '../../src/formatter'; -import { ClassThatLogs, LogJsonIndent } from '../../src/types'; +import { ClassThatLogs, LogJsonIndent, ConstructorOptions } from '../../src/types'; import { Context } from 'aws-lambda'; import { Console } from 'console'; @@ -1297,7 +1297,7 @@ describe('Class: Logger', () => { describe('Method: createChild', () => { - test('Child and grandchild loggers should have all ancestor\'s options', () => { + test('child and grandchild loggers should have all ancestor\'s options', () => { // Prepare const INDENTATION = LogJsonIndent.COMPACT; const loggerOptions = { @@ -1398,7 +1398,7 @@ describe('Class: Logger', () => { }); - test('when called, it returns a DISTINCT clone of the logger instance', () => { + test('child logger should be a DISTINCT clone of the logger instance', () => { // Prepare const INDENTATION = LogJsonIndent.COMPACT; @@ -1716,6 +1716,93 @@ describe('Class: Logger', () => { }, }); }); + + test('child logger should have parent\'s logFormatter', () => { + + // Prepare + class MyCustomLogFormatter extends PowertoolLogFormatter {} + const parentLogger = new Logger({ + logFormatter: new MyCustomLogFormatter() + }); + + // Act + const childLoggerWithParentLogFormatter = parentLogger.createChild(); + + // Assess + expect(childLoggerWithParentLogFormatter).toEqual( + expect.objectContaining({ + logFormatter: expect.any(MyCustomLogFormatter), + }) + ); + }); + + test('child logger with custom logFormatter in options should have provided logFormatter', () => { + + // Prepare + class MyCustomLogFormatter extends PowertoolLogFormatter {} + const parentLogger = new Logger(); + + // Act + const childLoggerWithCustomLogFormatter = parentLogger.createChild({ + logFormatter: new MyCustomLogFormatter() + }); + + // Assess + expect(parentLogger).toEqual( + expect.objectContaining({ + logFormatter: expect.any(PowertoolLogFormatter), + }) + ); + + expect(childLoggerWithCustomLogFormatter).toEqual( + expect.objectContaining({ + logFormatter: expect.any(MyCustomLogFormatter), + }) + ); + }); + + test('child logger should have exact same attributes as the parent logger created with all non-default options', () => { + + // Prepare + class MyCustomLogFormatter extends PowertoolLogFormatter {} + class MyCustomEnvironmentVariablesService extends EnvironmentVariablesService {} + + const options: ConstructorOptions = { + logLevel: 'ERROR', + serviceName: 'test-service-name', + sampleRateValue: 0.77, + logFormatter: new MyCustomLogFormatter(), + customConfigService: new MyCustomEnvironmentVariablesService(), + persistentLogAttributes: { + aws_account_id: '1234567890', + aws_region: 'eu-west-1', + }, + environment: 'local' + }; + const parentLogger = new Logger(options); + + // Act + const childLogger = parentLogger.createChild(); + + // Assess + expect(childLogger).toEqual({ + ...parentLogger, + console: expect.any(Console), + }); + + expect(childLogger).toEqual( + expect.objectContaining({ + logFormatter: expect.any(MyCustomLogFormatter), + }) + ); + + expect(childLogger).toEqual( + expect.objectContaining({ + customConfigService: expect.any(MyCustomEnvironmentVariablesService), + }) + ); + + }); }); From 45d1078eccf671f589f2cf75dcbabe05dd18a7f2 Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Fri, 3 Feb 2023 18:03:21 +0400 Subject: [PATCH 2/2] test(logger): update test, expect any boolean in logsSampled because it depends on random --- packages/logger/src/Logger.ts | 2 +- packages/logger/tests/unit/Logger.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 575f24c12f..2ce5df7059 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -209,7 +209,7 @@ class Logger extends Utility implements ClassThatLogs { logFormatter: this.getLogFormatter(), }; const parentsPowertoolsLogData = this.getPowertoolLogData(); - const childLogger = new Logger(merge({}, parentsOptions, parentsPowertoolsLogData, options)); + const childLogger = new Logger(merge(parentsOptions, parentsPowertoolsLogData, options)); const parentsPersistentLogAttributes = this.getPersistentLogAttributes(); childLogger.addPersistentLogAttributes(parentsPersistentLogAttributes); diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index e2fc35ca4b..4b0775421d 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -1788,6 +1788,7 @@ describe('Class: Logger', () => { expect(childLogger).toEqual({ ...parentLogger, console: expect.any(Console), + logsSampled: expect.any(Boolean), }); expect(childLogger).toEqual(