Skip to content

Commit 6d31362

Browse files
christiand93christiand93
authored
Merge pull request #182 from SAP/fix-custom-fields
Fix custom fields processing
2 parents 8abac91 + 103582b commit 6d31362

File tree

3 files changed

+121
-42
lines changed

3 files changed

+121
-42
lines changed

‎src/lib/logger/logger.ts‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export default class Logger {
2626
this.recordWriter = RecordWriter.getInstance();
2727
}
2828

29-
createLogger(customFields?: Map<string, any>): Logger {
29+
createLogger(customFields?: Map<string, any> | Object): Logger {
3030

3131
let logger = new Logger(this);
3232
// assign custom fields, if provided
@@ -64,13 +64,13 @@ export default class Logger {
6464

6565
logMessage(level: string | Level, ...args: any) {
6666
if (!this.isLoggingLevel(level)) return;
67-
const loggerCustomFields = Object.assign({}, this.getCustomFieldsFromLogger(this));
67+
const loggerCustomFields = this.getCustomFieldsFromLogger(this);
6868

69-
let levelName : string
69+
let levelName : string;
7070
if (typeof level === 'string') {
71-
levelName = level
71+
levelName = level;
7272
} else {
73-
levelName = LevelUtils.getName(level)
73+
levelName = LevelUtils.getName(level);
7474
}
7575

7676
const record = this.recordFactory.buildMsgRecord(this.registeredCustomFields, loggerCustomFields, levelName, args, this.context);
@@ -130,16 +130,16 @@ export default class Logger {
130130
this.registeredCustomFields.push(...fieldNames);
131131
}
132132

133-
setCustomFields(customFields: Map<string, any>) {
134-
this.customFields = customFields;
133+
setCustomFields(customFields: Map<string, any> | Object) {
134+
if (customFields instanceof Map) {
135+
this.customFields = customFields;
136+
} else if (isValidObject(customFields)) {
137+
this.customFields = new Map(Object.entries(customFields))
138+
}
135139
}
136140

137141
getCustomFields(): Map<string, any> {
138-
if (this.parent) {
139-
return new Map([...this.parent.getCustomFields(), ...this.customFields.entries()])
140-
} else {
141-
return new Map(...this.customFields.entries())
142-
}
142+
return this.getCustomFieldsFromLogger(this)
143143
}
144144

145145
getContextProperty(name: string): string | undefined {
@@ -174,16 +174,11 @@ export default class Logger {
174174
this.setContextProperty("tenant_subdomain", value);
175175
}
176176

177-
private getCustomFieldsFromLogger(logger: Logger): any {
178-
let fields = {};
177+
private getCustomFieldsFromLogger(logger: Logger): Map<string, any> {
179178
if (logger.parent && logger.parent !== this) {
180-
fields = this.getCustomFieldsFromLogger(logger.parent);
179+
let parentFields = this.getCustomFieldsFromLogger(logger.parent);
180+
return new Map([...parentFields, ...logger.customFields]);
181181
}
182-
183-
if (isValidObject(logger.customFields)) {
184-
fields = Object.assign(fields, logger.customFields);
185-
}
186-
187-
return fields;
182+
return logger.customFields;
188183
}
189184
}

‎src/lib/logger/recordFactory.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ export default class RecordFactory {
3636
// init a new record and assign fields with output "msg-log"
3737
buildMsgRecord(registeredCustomFields: Array<string>, loggerCustomFields: Map<string, any>, levelName: string, args: Array<any>, context?: RequestContext): Record {
3838
const lastArg = args[args.length - 1];
39-
let customFieldsFromArgs = {};
39+
let customFieldsFromArgs = new Map<string, any>();
4040
let record = new Record(levelName)
4141

42+
4243
if (typeof lastArg === "object") {
4344
if (this.stacktraceUtils.isErrorWithStacktrace(lastArg)) {
4445
record.metadata.stacktrace = this.stacktraceUtils.prepareStacktrace(lastArg.stack);
@@ -47,6 +48,8 @@ export default class RecordFactory {
4748
record.metadata.stacktrace = this.stacktraceUtils.prepareStacktrace(lastArg._error.stack);
4849
delete lastArg._error;
4950
}
51+
customFieldsFromArgs = new Map<string, any>(Object.entries(lastArg));
52+
} else if (lastArg instanceof Map) {
5053
customFieldsFromArgs = lastArg;
5154
}
5255
args.pop();
@@ -60,7 +63,7 @@ export default class RecordFactory {
6063
record.metadata.message = util.format.apply(util, args);
6164

6265
// assign dynamic fields
63-
this.addDynamicFields(record, Output.MsgLog, Date.now());
66+
this.addDynamicFields(record, Output.MsgLog);
6467

6568
// assign values from request context if present
6669
if (context) {
@@ -89,14 +92,15 @@ export default class RecordFactory {
8992
this.addContext(record, context);
9093

9194
// assign custom fields
92-
const loggerCustomFields = Object.assign({}, req.logger.getCustomFieldsFromLogger(req.logger));
93-
this.addCustomFields(record, req.logger.registeredCustomFields, loggerCustomFields, {});
95+
const loggerCustomFields = req.logger.getCustomFieldsFromLogger(req.logger);
96+
this.addCustomFields(record, req.logger.registeredCustomFields, loggerCustomFields);
9497

9598
return record;
9699
}
97100

98-
private addCustomFields(record: Record, registeredCustomFields: Array<string>, loggerCustomFields: Map<string, any>, customFieldsFromArgs: any) {
99-
const providedFields = Object.assign({}, loggerCustomFields, customFieldsFromArgs);
101+
private addCustomFields(record: Record, registeredCustomFields: Array<string>, loggerCustomFields: Map<string, any>,
102+
customFieldsFromArgs: Map<string, any> = new Map()) {
103+
const providedFields = new Map<string, any>([...loggerCustomFields, ...customFieldsFromArgs]);
100104
const customFieldsFormat = this.config.getConfig().customFieldsFormat!;
101105

102106
// if format "disabled", do not log any custom fields
@@ -105,9 +109,7 @@ export default class RecordFactory {
105109
}
106110

107111
let indexedCustomFields: any = {};
108-
for (let key in providedFields) {
109-
let value = providedFields[key];
110-
112+
providedFields.forEach((value, key) => {
111113
// Stringify, if necessary.
112114
if ((typeof value) != "string") {
113115
value = jsonStringifySafe(value);
@@ -116,12 +118,13 @@ export default class RecordFactory {
116118
if ([CustomFieldsFormat.CloudLogging, CustomFieldsFormat.All, CustomFieldsFormat.Default].includes(customFieldsFormat)
117119
|| record.payload[key] != null || this.config.isSettable(key)) {
118120
record.payload[key] = value;
121+
119122
}
120123

121124
if ([CustomFieldsFormat.ApplicationLogging, CustomFieldsFormat.All].includes(customFieldsFormat)) {
122125
indexedCustomFields[key] = value;
123126
}
124-
}
127+
});
125128

126129
//writes custom fields in the correct order and correlates i to the place in registeredCustomFields
127130
if (Object.keys(indexedCustomFields).length > 0) {

‎src/test/acceptance-test/custom-fields.test.js‎

Lines changed: 92 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ var expect = require('chai').expect;
22
const importFresh = require('import-fresh');
33

44
var log;
5+
var childLogger;
56
var lastOutput;
67

78
describe('Test custom fields', function () {
@@ -16,33 +17,113 @@ describe('Test custom fields', function () {
1617
});
1718
});
1819

19-
20-
describe('Writes log with a global custom field ', function () {
20+
describe('Test writing a log with a global custom field', function () {
2121
beforeEach(function () {
22-
log.setCustomFields({ "field-a": "value" });
23-
log.logMessage("info", "test-message");
22+
log.setCustomFields({'field-a': 'value'});
23+
log.logMessage('info', 'test-message');
2424
});
2525

2626
it('writes a log with a custom field', function () {
2727
expect(lastOutput).to.have.property('field-a', 'value');
2828
});
29+
30+
it('returns the global custom field', function () {
31+
expect([ ...log.getCustomFields().keys() ]).to.have.members(['field-a']);
32+
expect(log.getCustomFields().get('field-a')).to.equal('value');
33+
});
2934
});
3035

31-
describe('Write log with a child custom field ', function () {
36+
describe('Test writing custom fields with a child logger', function () {
3237
beforeEach(function () {
33-
log.setCustomFields({ "field-a": "value" });
34-
log.logMessage("info", "test-message");
38+
log.setCustomFields({'field-a': 'value-a'});
39+
childLogger = log.createLogger({'field-b': 'value-b'});
40+
childLogger.logMessage('info', 'test-message', {'field-c': 'value-c'});
3541
});
3642

37-
it('writes a log with a custom field', function () {
38-
expect(lastOutput).to.have.property('field-a', 'value');
43+
it('writes a log with a global custom field', function () {
44+
expect(lastOutput).to.have.property('field-a', 'value-a');
45+
});
46+
47+
it('writes a log with a child logger custom field', function () {
48+
expect(lastOutput).to.have.property('field-b', 'value-b');
49+
});
50+
51+
it('writes a log with a custom field from logMessage call', function () {
52+
expect(lastOutput).to.have.property('field-c', 'value-c');
53+
});
54+
55+
it('returns the global custom field', function () {
56+
expect([ ...log.getCustomFields().keys() ]).to.have.members(['field-a'])
57+
expect(log.getCustomFields().get('field-a')).to.equal('value-a');
58+
});
59+
60+
it('returns the global and the child logger custom fields', function () {
61+
expect([ ...childLogger.getCustomFields().keys() ]).to.have.members(['field-a', 'field-b']);
62+
expect(childLogger.getCustomFields().get('field-a')).to.equal('value-a');
63+
expect(childLogger.getCustomFields().get('field-b')).to.equal('value-b');
3964
});
4065
});
4166

42-
describe('Test custom field format', function () {
67+
describe('Test overriding custom fields with a child logger', function () {
68+
beforeEach(function () {
69+
log.setCustomFields({'field-a': 'value-a'});
70+
childLogger = log.createLogger({'field-a': 'value-override-a','field-b': 'value-b'});
71+
childLogger.logMessage('info', 'test-message', {'field-b': 'value-override-b'});
72+
});
4373

44-
describe('"cloud-logging" format', function () {
74+
it('writes a log with an overridden global custom field', function () {
75+
expect(lastOutput).to.have.property('field-a', 'value-override-a');
76+
});
4577

78+
it('writes a log with an overridden child logger custom field', function () {
79+
expect(lastOutput).to.have.property('field-b', 'value-override-b');
80+
});
81+
82+
it('returns the original global custom field', function () {
83+
expect([ ...log.getCustomFields().keys() ]).to.have.members(['field-a']);
84+
expect(log.getCustomFields().get('field-a')).to.equal('value-a');
85+
});
86+
87+
it('returns the overridden global and the child logger custom fields', function () {
88+
expect([ ...childLogger.getCustomFields().keys() ]).to.have.members(['field-a', 'field-b']);
89+
expect(childLogger.getCustomFields().get('field-a')).to.equal('value-override-a');
90+
expect(childLogger.getCustomFields().get('field-b')).to.equal('value-b');
91+
});
92+
});
93+
94+
describe('Test writing custom fields using JS Maps', function () {
95+
beforeEach(function () {
96+
log.setCustomFields(new Map().set('field-a', 'value-a'));
97+
childLogger = log.createLogger(new Map().set('field-b', 'value-b'));
98+
childLogger.logMessage('info', 'test-message', new Map().set('field-c', 'value-c'));
99+
});
100+
101+
it('writes a log with a global custom field', function () {
102+
expect(lastOutput).to.have.property('field-a', 'value-a');
103+
});
104+
105+
it('writes a log with a child logger custom field', function () {
106+
expect(lastOutput).to.have.property('field-b', 'value-b');
107+
});
108+
109+
it('writes a log with a custom field from logMessage call', function () {
110+
expect(lastOutput).to.have.property('field-c', 'value-c');
111+
});
112+
113+
it('returns the global custom field', function () {
114+
expect([ ...log.getCustomFields().keys() ]).to.have.members(['field-a']);
115+
expect(log.getCustomFields().get('field-a')).to.equal('value-a');
116+
});
117+
118+
it('returns the global and the child logger custom fields', function () {
119+
expect([ ...childLogger.getCustomFields().keys() ]).to.have.members(['field-a', 'field-b']);
120+
expect(childLogger.getCustomFields().get('field-a')).to.equal('value-a');
121+
expect(childLogger.getCustomFields().get('field-b')).to.equal('value-b');
122+
});
123+
});
124+
125+
describe('Test custom field format', function () {
126+
describe('"cloud-logging" format', function () {
46127
beforeEach(function () {
47128
var obj = {
48129
"cloud-logs": {}

0 commit comments

Comments
 (0)