Skip to content

Commit eafbe13

Browse files
iamgergsvozza
andauthored
fix(logger): not passing persistent keys to children (#4740)
Co-authored-by: Stefano Vozza <[email protected]>
1 parent dd84f94 commit eafbe13

File tree

3 files changed

+303
-4
lines changed

3 files changed

+303
-4
lines changed

packages/logger/src/Logger.ts

Lines changed: 14 additions & 2 deletions
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,8 +348,7 @@ class Logger extends Utility implements LoggerInterface {
335348
logFormatter: this.getLogFormatter(),
336349
customConfigService: this.getCustomConfigService(),
337350
environment: this.powertoolsLogData.environment,
338-
persistentLogAttributes:
339-
this.#attributesStore.getPersistentAttributes(),
351+
[persistentKeyName]: this.#attributesStore.getPersistentAttributes(),
340352
jsonReplacerFn: this.#jsonReplacerFn,
341353
correlationIdSearchFn: this.#correlationIdSearchFn,
342354
...(this.#bufferConfig.enabled && {

packages/logger/tests/unit/initializeLogger.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,88 @@ describe('Log levels', () => {
110110
);
111111
});
112112

113+
it('overrides the service name when creating a child logger', () => {
114+
// Prepare
115+
vi.stubEnv('POWERTOOLS_SERVICE_NAME', 'hello-world');
116+
const logger = new Logger();
117+
const childLogger = logger.createChild({ serviceName: 'child-service' });
118+
119+
// Act
120+
childLogger.info('Hello, world!');
121+
122+
// Assess
123+
expect(console.info).toHaveBeenCalledTimes(1);
124+
expect(console.info).toHaveLoggedNth(
125+
1,
126+
expect.objectContaining({ service: 'child-service' })
127+
);
128+
});
129+
130+
it('maintains persistentKeys when creating a child logger', () => {
131+
// Prepare
132+
const mockDate = new Date(1466424490000);
133+
vi.useFakeTimers().setSystemTime(mockDate);
134+
const logger = new Logger({
135+
persistentKeys: {
136+
foo: 'hello',
137+
overridable: 1,
138+
},
139+
});
140+
141+
logger.appendKeys({
142+
resettableKey: 'some-id',
143+
});
144+
145+
logger.appendPersistentKeys({
146+
dynamic: 'stays',
147+
});
148+
149+
const childLogger = logger.createChild({
150+
serviceName: 'child-service',
151+
persistentKeys: {
152+
bar: 'world',
153+
overridable: 2,
154+
},
155+
});
156+
157+
// Act
158+
childLogger.info('Hello, world!');
159+
childLogger.resetKeys();
160+
childLogger.info('Hello again!');
161+
162+
// Assess
163+
expect(console.info).toHaveBeenCalledTimes(2);
164+
expect(console.info).toHaveLoggedNth(
165+
1,
166+
expect.objectContaining({
167+
service: 'child-service',
168+
foo: 'hello',
169+
bar: 'world',
170+
dynamic: 'stays',
171+
message: 'Hello, world!',
172+
overridable: 2,
173+
resettableKey: 'some-id',
174+
})
175+
);
176+
177+
expect(console.info).toHaveLoggedNth(
178+
2,
179+
// using direct match here to ensure removal of resetKeys
180+
{
181+
service: 'child-service',
182+
foo: 'hello',
183+
bar: 'world',
184+
dynamic: 'stays',
185+
message: 'Hello again!',
186+
overridable: 2,
187+
level: 'INFO',
188+
sampling_rate: 0,
189+
timestamp: '2016-06-20T12:08:10.000Z',
190+
xray_trace_id: '1-abcdef12-3456abcdef123456abcdef12',
191+
}
192+
);
193+
});
194+
113195
it('`logRecordOrder` should be passed down to child logger', () => {
114196
// Prepare
115197
const expectedKeys = [

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

Lines changed: 207 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,11 +519,16 @@ describe('Working with keys', () => {
519519
});
520520

521521
// Act
522-
const childLogger = logger.createChild();
522+
const childLogger = logger.createChild({
523+
persistentKeys: {
524+
bar: 'foo',
525+
},
526+
});
523527

524528
// Assess
525529
expect(childLogger.getPersistentLogAttributes()).toEqual({
526530
foo: 'bar',
531+
bar: 'foo',
527532
});
528533
});
529534

@@ -671,7 +676,11 @@ describe('Working with keys', () => {
671676

672677
it('should pass persistentKeys to child with no warning', () => {
673678
// Prepare
674-
const logger = new Logger();
679+
const logger = new Logger({
680+
persistentKeys: {
681+
foo: 'bar',
682+
},
683+
});
675684
logger.createChild({ persistentKeys: { abc: 'xyz' } });
676685

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

0 commit comments

Comments
 (0)