Skip to content

Commit cdca132

Browse files
iamgergiamgerg
committed
address PR feedback
1 parent fec3943 commit cdca132

File tree

2 files changed

+211
-1
lines changed

2 files changed

+211
-1
lines changed

‎packages/logger/src/Logger.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,19 @@ class Logger extends Utility implements LoggerInterface {
323323
* @param options - The options to initialize the child logger with.
324324
*/
325325
public createChild(options: ConstructorOptions = {}): Logger {
326+
// Determine proper key to use when merging persistent options with
327+
// parent. Preferring persistentKeys over persistentLogAttributes.
328+
// NOTE: when both are passed in for options warning message in
329+
// setOptions is still triggered
330+
const persistentKeyName: keyof Pick<
331+
// extra type safety since dealing with string values and merge does not care
332+
ConstructorOptions,
333+
'persistentKeys' | 'persistentLogAttributes'
334+
> =
335+
'persistentLogAttributes' in options && !('persistentKeys' in options)
336+
? 'persistentLogAttributes'
337+
: 'persistentKeys';
338+
326339
const childLogger = this.createLogger(
327340
// Merge parent logger options with options passed to createChild,
328341
// the latter having precedence.
@@ -335,7 +348,7 @@ class Logger extends Utility implements LoggerInterface {
335348
logFormatter: this.getLogFormatter(),
336349
customConfigService: this.getCustomConfigService(),
337350
environment: this.powertoolsLogData.environment,
338-
persistentKeys: this.#attributesStore.getPersistentAttributes(),
351+
[persistentKeyName]: this.#attributesStore.getPersistentAttributes(),
339352
jsonReplacerFn: this.#jsonReplacerFn,
340353
correlationIdSearchFn: this.#correlationIdSearchFn,
341354
...(this.#bufferConfig.enabled && {

‎packages/logger/tests/unit/workingWithkeys.test.ts‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { log } from 'node:console';
12
import { setTimeout } from 'node:timers/promises';
23
import context from '@aws-lambda-powertools/testing-utils/context';
34
import middy from '@middy/core';
@@ -825,4 +826,200 @@ describe('Working with keys', () => {
825826
);
826827
}
827828
);
829+
830+
describe('deprecated persistentLogAttributes usage', () => {
831+
it('should set #attributesStore on the logger', () => {
832+
// Prepare
833+
const logger = new Logger({
834+
persistentLogAttributes: {
835+
foo: 'bar',
836+
},
837+
});
838+
839+
// Assess
840+
expect(logger.getPersistentLogAttributes()).toEqual({
841+
foo: 'bar',
842+
});
843+
844+
expect(console.warn).not.toHaveLogged(
845+
expect.objectContaining({
846+
message:
847+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
848+
})
849+
);
850+
});
851+
852+
it('should get overridden by persistentKeys usage', () => {
853+
// Prepare
854+
const logger = new Logger({
855+
persistentKeys: {
856+
foo: 'bar',
857+
},
858+
// @ts-expect-error
859+
persistentLogAttributes: {
860+
foo: 'not-bar',
861+
},
862+
});
863+
864+
// Assess
865+
expect(logger.getPersistentLogAttributes()).toEqual({
866+
foo: 'bar',
867+
});
868+
869+
expect(console.warn).toHaveLogged(
870+
expect.objectContaining({
871+
message:
872+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
873+
})
874+
);
875+
});
876+
877+
it('should persist for child loggers', () => {
878+
// Prepare
879+
const logger = new Logger({
880+
persistentLogAttributes: {
881+
foo: 'bar',
882+
},
883+
});
884+
885+
// Act
886+
const child = logger.createChild();
887+
888+
// Assess
889+
expect(child.getPersistentLogAttributes()).toEqual({
890+
foo: 'bar',
891+
});
892+
893+
expect(console.warn).not.toHaveLogged(
894+
expect.objectContaining({
895+
message:
896+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
897+
})
898+
);
899+
});
900+
901+
it('should persist for child loggers using persistentLogAttributes', () => {
902+
// Prepare
903+
const logger = new Logger({
904+
persistentLogAttributes: {
905+
foo: 'bar',
906+
},
907+
});
908+
909+
// Act
910+
const child = logger.createChild({
911+
persistentLogAttributes: {
912+
bar: 'foo',
913+
},
914+
});
915+
916+
// Assess
917+
expect(child.getPersistentLogAttributes()).toEqual({
918+
foo: 'bar',
919+
bar: 'foo',
920+
});
921+
922+
expect(console.warn).not.toHaveLogged(
923+
expect.objectContaining({
924+
message:
925+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
926+
})
927+
);
928+
});
929+
930+
it('should persist for child loggers using persistKeys', () => {
931+
// Prepare
932+
const logger = new Logger({
933+
persistentLogAttributes: {
934+
foo: 'bar',
935+
},
936+
});
937+
938+
// Act
939+
const child = logger.createChild({
940+
persistentKeys: {
941+
bar: 'foo',
942+
},
943+
});
944+
945+
// Assess
946+
expect(child.getPersistentLogAttributes()).toEqual({
947+
foo: 'bar',
948+
bar: 'foo',
949+
});
950+
951+
expect(console.warn).not.toHaveLogged(
952+
expect.objectContaining({
953+
message:
954+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
955+
})
956+
);
957+
});
958+
959+
it('should persist for child loggers using persistentLogAttributes when parent used persistentLogAttributes', () => {
960+
// Prepare
961+
const logger = new Logger({
962+
persistentLogAttributes: {
963+
foo: 'bar',
964+
},
965+
});
966+
967+
// Act
968+
const child = logger.createChild({
969+
persistentKeys: {
970+
bar: 'foo',
971+
},
972+
// @ts-expect-error
973+
persistentLogAttributes: {
974+
bar: 'not-foo',
975+
},
976+
});
977+
978+
// Assess
979+
expect(child.getPersistentLogAttributes()).toEqual({
980+
foo: 'bar',
981+
bar: 'foo',
982+
});
983+
984+
expect(console.warn).toHaveLogged(
985+
expect.objectContaining({
986+
message:
987+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
988+
})
989+
);
990+
});
991+
992+
it('should persist for child loggers using persistentLogAttributes when parent used persistentKeys', () => {
993+
// Prepare
994+
const logger = new Logger({
995+
persistentKeys: {
996+
foo: 'bar',
997+
},
998+
});
999+
1000+
// Act
1001+
const child = logger.createChild({
1002+
persistentKeys: {
1003+
bar: 'foo',
1004+
},
1005+
// @ts-expect-error
1006+
persistentLogAttributes: {
1007+
bar: 'not-foo',
1008+
},
1009+
});
1010+
1011+
// Assess
1012+
expect(child.getPersistentLogAttributes()).toEqual({
1013+
foo: 'bar',
1014+
bar: 'foo',
1015+
});
1016+
1017+
expect(console.warn).toHaveLogged(
1018+
expect.objectContaining({
1019+
message:
1020+
'Both persistentLogAttributes and persistentKeys options were provided. Using persistentKeys as persistentLogAttributes is deprecated and will be removed in future releases',
1021+
})
1022+
);
1023+
});
1024+
});
8281025
});

0 commit comments

Comments
 (0)