From fec3943708ad6dd651ceeba46691c74ac1bb0d18 Mon Sep 17 00:00:00 2001 From: IAmGerg Date: Wed, 12 Nov 2025 07:29:29 -0600 Subject: [PATCH 1/3] fix(logger): not passing persistentKeys to child --- packages/logger/src/Logger.ts | 3 +- .../tests/unit/initializeLogger.test.ts | 82 +++++++++++++++++++ .../logger/tests/unit/workingWithkeys.test.ts | 13 ++- 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 4276437f83..c9b044bcb8 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -335,8 +335,7 @@ class Logger extends Utility implements LoggerInterface { logFormatter: this.getLogFormatter(), customConfigService: this.getCustomConfigService(), environment: this.powertoolsLogData.environment, - persistentLogAttributes: - this.#attributesStore.getPersistentAttributes(), + persistentKeys: this.#attributesStore.getPersistentAttributes(), jsonReplacerFn: this.#jsonReplacerFn, correlationIdSearchFn: this.#correlationIdSearchFn, ...(this.#bufferConfig.enabled && { diff --git a/packages/logger/tests/unit/initializeLogger.test.ts b/packages/logger/tests/unit/initializeLogger.test.ts index 1e2f4c9224..7bad0df979 100644 --- a/packages/logger/tests/unit/initializeLogger.test.ts +++ b/packages/logger/tests/unit/initializeLogger.test.ts @@ -110,6 +110,88 @@ describe('Log levels', () => { ); }); + it('overrides the service name when creating a child logger', () => { + // Prepare + vi.stubEnv('POWERTOOLS_SERVICE_NAME', 'hello-world'); + const logger = new Logger(); + const childLogger = logger.createChild({ serviceName: 'child-service' }); + + // Act + childLogger.info('Hello, world!'); + + // Assess + expect(console.info).toHaveBeenCalledTimes(1); + expect(console.info).toHaveLoggedNth( + 1, + expect.objectContaining({ service: 'child-service' }) + ); + }); + + it('maintains persistentKeys when creating a child logger', () => { + // Prepare + const mockDate = new Date(1466424490000); + vi.useFakeTimers().setSystemTime(mockDate); + const logger = new Logger({ + persistentKeys: { + foo: 'hello', + overridable: 1, + }, + }); + + logger.appendKeys({ + resettableKey: 'some-id', + }); + + logger.appendPersistentKeys({ + dynamic: 'stays', + }); + + const childLogger = logger.createChild({ + serviceName: 'child-service', + persistentKeys: { + bar: 'world', + overridable: 2, + }, + }); + + // Act + childLogger.info('Hello, world!'); + childLogger.resetKeys(); + childLogger.info('Hello again!'); + + // Assess + expect(console.info).toHaveBeenCalledTimes(2); + expect(console.info).toHaveLoggedNth( + 1, + expect.objectContaining({ + service: 'child-service', + foo: 'hello', + bar: 'world', + dynamic: 'stays', + message: 'Hello, world!', + overridable: 2, + resettableKey: 'some-id', + }) + ); + + expect(console.info).toHaveLoggedNth( + 2, + // using direct match here to ensure removal of resetKeys + { + service: 'child-service', + foo: 'hello', + bar: 'world', + dynamic: 'stays', + message: 'Hello again!', + overridable: 2, + level: 'INFO', + sampling_rate: 0, + timestamp: '2016-06-20T12:08:10.000Z', + xray_trace_id: '1-abcdef12-3456abcdef123456abcdef12', + } + ); + }); + it('`logRecordOrder` should be passed down to child logger', () => { // Prepare const expectedKeys = [ diff --git a/packages/logger/tests/unit/workingWithkeys.test.ts b/packages/logger/tests/unit/workingWithkeys.test.ts index d575f1ecc7..e9b935c77a 100644 --- a/packages/logger/tests/unit/workingWithkeys.test.ts +++ b/packages/logger/tests/unit/workingWithkeys.test.ts @@ -519,11 +519,16 @@ describe('Working with keys', () => { }); // Act - const childLogger = logger.createChild(); + const childLogger = logger.createChild({ + persistentKeys: { + bar: 'foo', + }, + }); // Assess expect(childLogger.getPersistentLogAttributes()).toEqual({ foo: 'bar', + bar: 'foo', }); }); @@ -671,7 +676,11 @@ describe('Working with keys', () => { it('should pass persistentKeys to child with no warning', () => { // Prepare - const logger = new Logger(); + const logger = new Logger({ + persistentKeys: { + foo: 'bar', + }, + }); logger.createChild({ persistentKeys: { abc: 'xyz' } }); // Assess From cdca1328ee970c3b415c91b638527d9e8d909881 Mon Sep 17 00:00:00 2001 From: IAmGerg Date: Wed, 12 Nov 2025 10:17:24 -0600 Subject: [PATCH 2/3] address PR feedback --- packages/logger/src/Logger.ts | 15 +- .../logger/tests/unit/workingWithkeys.test.ts | 197 ++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index c9b044bcb8..0413c5fddb 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -323,6 +323,19 @@ class Logger extends Utility implements LoggerInterface { * @param options - The options to initialize the child logger with. */ public createChild(options: ConstructorOptions = {}): Logger { + // Determine proper key to use when merging persistent options with + // parent. Preferring persistentKeys over persistentLogAttributes. + // NOTE: when both are passed in for options warning message in + // setOptions is still triggered + const persistentKeyName: keyof Pick< + // extra type safety since dealing with string values and merge does not care + ConstructorOptions, + 'persistentKeys' | 'persistentLogAttributes' + > = + 'persistentLogAttributes' in options && !('persistentKeys' in options) + ? 'persistentLogAttributes' + : 'persistentKeys'; + const childLogger = this.createLogger( // Merge parent logger options with options passed to createChild, // the latter having precedence. @@ -335,7 +348,7 @@ class Logger extends Utility implements LoggerInterface { logFormatter: this.getLogFormatter(), customConfigService: this.getCustomConfigService(), environment: this.powertoolsLogData.environment, - persistentKeys: this.#attributesStore.getPersistentAttributes(), + [persistentKeyName]: this.#attributesStore.getPersistentAttributes(), jsonReplacerFn: this.#jsonReplacerFn, correlationIdSearchFn: this.#correlationIdSearchFn, ...(this.#bufferConfig.enabled && { diff --git a/packages/logger/tests/unit/workingWithkeys.test.ts b/packages/logger/tests/unit/workingWithkeys.test.ts index e9b935c77a..4085b7ee4f 100644 --- a/packages/logger/tests/unit/workingWithkeys.test.ts +++ b/packages/logger/tests/unit/workingWithkeys.test.ts @@ -1,3 +1,4 @@ +import { log } from 'node:console'; import { setTimeout } from 'node:timers/promises'; import context from '@aws-lambda-powertools/testing-utils/context'; import middy from '@middy/core'; @@ -825,4 +826,200 @@ describe('Working with keys', () => { ); } ); + + describe('deprecated persistentLogAttributes usage', () => { + it('should set #attributesStore on the logger', () => { + // Prepare + const logger = new Logger({ + persistentLogAttributes: { + foo: 'bar', + }, + }); + + // Assess + expect(logger.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + }); + + expect(console.warn).not.toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should get overridden by persistentKeys usage', () => { + // Prepare + const logger = new Logger({ + persistentKeys: { + foo: 'bar', + }, + // @ts-expect-error + persistentLogAttributes: { + foo: 'not-bar', + }, + }); + + // Assess + expect(logger.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + }); + + expect(console.warn).toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should persist for child loggers', () => { + // Prepare + const logger = new Logger({ + persistentLogAttributes: { + foo: 'bar', + }, + }); + + // Act + const child = logger.createChild(); + + // Assess + expect(child.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + }); + + expect(console.warn).not.toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should persist for child loggers using persistentLogAttributes', () => { + // Prepare + const logger = new Logger({ + persistentLogAttributes: { + foo: 'bar', + }, + }); + + // Act + const child = logger.createChild({ + persistentLogAttributes: { + bar: 'foo', + }, + }); + + // Assess + expect(child.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + bar: 'foo', + }); + + expect(console.warn).not.toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should persist for child loggers using persistKeys', () => { + // Prepare + const logger = new Logger({ + persistentLogAttributes: { + foo: 'bar', + }, + }); + + // Act + const child = logger.createChild({ + persistentKeys: { + bar: 'foo', + }, + }); + + // Assess + expect(child.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + bar: 'foo', + }); + + expect(console.warn).not.toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should persist for child loggers using persistentLogAttributes when parent used persistentLogAttributes', () => { + // Prepare + const logger = new Logger({ + persistentLogAttributes: { + foo: 'bar', + }, + }); + + // Act + const child = logger.createChild({ + persistentKeys: { + bar: 'foo', + }, + // @ts-expect-error + persistentLogAttributes: { + bar: 'not-foo', + }, + }); + + // Assess + expect(child.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + bar: 'foo', + }); + + expect(console.warn).toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + + it('should persist for child loggers using persistentLogAttributes when parent used persistentKeys', () => { + // Prepare + const logger = new Logger({ + persistentKeys: { + foo: 'bar', + }, + }); + + // Act + const child = logger.createChild({ + persistentKeys: { + bar: 'foo', + }, + // @ts-expect-error + persistentLogAttributes: { + bar: 'not-foo', + }, + }); + + // Assess + expect(child.getPersistentLogAttributes()).toEqual({ + foo: 'bar', + bar: 'foo', + }); + + expect(console.warn).toHaveLogged( + expect.objectContaining({ + message: + 'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases', + }) + ); + }); + }); }); From 4440aa8bdff8ebf4477b195a1fcc5a349bfa3ac7 Mon Sep 17 00:00:00 2001 From: IAmGerg Date: Wed, 12 Nov 2025 11:27:39 -0600 Subject: [PATCH 3/3] address PR feedback --- .../logger/tests/unit/workingWithkeys.test.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/logger/tests/unit/workingWithkeys.test.ts b/packages/logger/tests/unit/workingWithkeys.test.ts index 4085b7ee4f..1bf67e7a93 100644 --- a/packages/logger/tests/unit/workingWithkeys.test.ts +++ b/packages/logger/tests/unit/workingWithkeys.test.ts @@ -1,4 +1,3 @@ -import { log } from 'node:console'; import { setTimeout } from 'node:timers/promises'; import context from '@aws-lambda-powertools/testing-utils/context'; import middy from '@middy/core'; @@ -828,7 +827,7 @@ describe('Working with keys', () => { ); describe('deprecated persistentLogAttributes usage', () => { - it('should set #attributesStore on the logger', () => { + it('sets #attributesStore on the logger', () => { // Prepare const logger = new Logger({ persistentLogAttributes: { @@ -849,7 +848,7 @@ describe('Working with keys', () => { ); }); - it('should get overridden by persistentKeys usage', () => { + it('gets overridden by persistentKeys usage', () => { // Prepare const logger = new Logger({ persistentKeys: { @@ -874,7 +873,7 @@ describe('Working with keys', () => { ); }); - it('should persist for child loggers', () => { + it('persists for child loggers', () => { // Prepare const logger = new Logger({ persistentLogAttributes: { @@ -898,7 +897,7 @@ describe('Working with keys', () => { ); }); - it('should persist for child loggers using persistentLogAttributes', () => { + it('persists for child loggers using persistentLogAttributes', () => { // Prepare const logger = new Logger({ persistentLogAttributes: { @@ -927,7 +926,7 @@ describe('Working with keys', () => { ); }); - it('should persist for child loggers using persistKeys', () => { + it('persists for child loggers using persistKeys', () => { // Prepare const logger = new Logger({ persistentLogAttributes: { @@ -956,7 +955,7 @@ describe('Working with keys', () => { ); }); - it('should persist for child loggers using persistentLogAttributes when parent used persistentLogAttributes', () => { + it('persists for child loggers using persistentLogAttributes when parent used persistentLogAttributes', () => { // Prepare const logger = new Logger({ persistentLogAttributes: { @@ -989,7 +988,7 @@ describe('Working with keys', () => { ); }); - it('should persist for child loggers using persistentLogAttributes when parent used persistentKeys', () => { + it('persists for child loggers using persistentLogAttributes when parent used persistentKeys', () => { // Prepare const logger = new Logger({ persistentKeys: {